Skip to content

Commit a2a3f58

Browse files
sebastianrathsebastianrath
committed
Address security issues and update e2e tests
1 parent b4d5207 commit a2a3f58

29 files changed

Lines changed: 255 additions & 97 deletions

‎core/docker.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ func (d *DockerClient) PullImage(ctx context.Context, imageRef string) error {
100100
verbose := !IsTestE2eRunning()
101101

102102
if verbose {
103-
utils.LogOut.Infof("%sPulling image '%s'\n", utils.LogGhStartGroup, imageRef)
103+
utils.LogOut.Infof("%sPulling image '%s'\n", utils.LogGhStartGroup, utils.SanitizeImageRef(imageRef))
104104
defer utils.LogOut.Infof(utils.LogGhEndGroup)
105105
}
106106

@@ -138,7 +138,7 @@ func (d *DockerClient) BuildImage(ctx context.Context, dockerfilePath, contextPa
138138
verbose := !IsTestE2eRunning()
139139

140140
if verbose {
141-
utils.LogOut.Infof("%sBuilding image '%s' from %s\n", utils.LogGhStartGroup, tag, dockerfilePath)
141+
utils.LogOut.Infof("%sBuilding image '%s' from %s\n", utils.LogGhStartGroup, utils.SanitizeImageRef(tag), utils.SanitizeImageRef(dockerfilePath))
142142
defer utils.LogOut.Infof(utils.LogGhEndGroup)
143143
}
144144

‎core/github.go‎

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -213,9 +213,12 @@ func LoadGitHubContext(env map[string]string, inputs map[string]any, secrets map
213213
// Support for github.event.pull_request, github.event.commits, etc.
214214
eventData := make(map[string]any)
215215
if eventPath, ok := env["GITHUB_EVENT_PATH"]; ok && eventPath != "" {
216-
fileContent, err := os.ReadFile(eventPath)
216+
cleanPath, err := utils.ValidatePath(eventPath)
217217
if err == nil {
218-
_ = json.Unmarshal(fileContent, &eventData)
218+
fileContent, err := os.ReadFile(cleanPath)
219+
if err == nil {
220+
_ = json.Unmarshal(fileContent, &eventData)
221+
}
219222
}
220223
}
221224

@@ -439,9 +442,13 @@ func SetupGitHubActionsEnv(finalEnv map[string]string) error {
439442

440443
for _, filePath := range fileCommandFiles {
441444
if filePath != "" {
442-
f, err := os.Create(filePath)
445+
cleanPath, err := utils.ValidatePath(filePath)
446+
if err != nil {
447+
return CreateErr(nil, err, "invalid file command path %s", filePath)
448+
}
449+
f, err := os.Create(cleanPath)
443450
if err != nil {
444-
return CreateErr(nil, err, "failed to create file command file %s", filePath).
451+
return CreateErr(nil, err, "failed to create file command file %s", cleanPath).
445452
SetHint("Check that you have write permissions to the runner temp directory.")
446453
}
447454
f.Close()

‎core/github_evaluator.go‎

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import (
1616
"strconv"
1717
"strings"
1818

19+
"github.com/actionforge/actrun-cli/utils"
1920
"github.com/rhysd/actionlint"
2021
)
2122

@@ -318,9 +319,13 @@ func (e *Evaluator) hashFiles(patterns ...string) (string, error) {
318319
}
319320
var uniqueFiles []string
320321
for f := range fileSet {
321-
info, err := os.Stat(f)
322+
cleanPath, err := utils.ValidatePath(f)
323+
if err != nil {
324+
continue
325+
}
326+
info, err := os.Stat(cleanPath)
322327
if err == nil && info.Mode().IsRegular() {
323-
uniqueFiles = append(uniqueFiles, f)
328+
uniqueFiles = append(uniqueFiles, cleanPath)
324329
}
325330
}
326331
sort.Strings(uniqueFiles)
@@ -331,7 +336,11 @@ func (e *Evaluator) hashFiles(patterns ...string) (string, error) {
331336

332337
hasher := sha256.New()
333338
for _, filePath := range uniqueFiles {
334-
file, err := os.Open(filePath)
339+
cleanPath, err := utils.ValidatePath(filePath)
340+
if err != nil {
341+
continue
342+
}
343+
file, err := os.Open(cleanPath)
335344
if err != nil {
336345
continue
337346
}

‎core/graph.go‎

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -325,13 +325,17 @@ func RunGraph(ctx context.Context, graphName string, graphContent []byte, opts R
325325

326326
// Priority 1 (Lowest): Config file
327327
if opts.ConfigFile != "" {
328-
if _, err := os.Stat(opts.ConfigFile); err == nil {
329-
localConfig, err := utils.LoadConfig(opts.ConfigFile)
328+
cleanConfigPath, err := utils.ValidatePath(opts.ConfigFile)
329+
if err != nil {
330+
return CreateErr(nil, err, "invalid config file path")
331+
}
332+
if _, err := os.Stat(cleanConfigPath); err == nil {
333+
localConfig, err := utils.LoadConfig(cleanConfigPath)
330334
if err != nil {
331335
return CreateErr(nil, err, "failed to load config file")
332336
}
333337

334-
configName := filepath.Base(opts.ConfigFile)
338+
configName := filepath.Base(cleanConfigPath)
335339
envTracker.set(localConfig.Env, configName, true, false)
336340
inputTracker.set(localConfig.Inputs, configName, true, false)
337341
secretTracker.set(localConfig.Secrets, configName, true, true)
@@ -463,11 +467,15 @@ func RunGraph(ctx context.Context, graphName string, graphContent []byte, opts R
463467
}
464468

465469
if newCwd != "" {
470+
cleanCwd, err := utils.ValidatePath(newCwd)
471+
if err != nil {
472+
return CreateErr(nil, err, "invalid working directory path")
473+
}
466474
originalCwd, err := os.Getwd()
467475
if err != nil {
468476
return CreateErr(nil, err, "failed to get current working directory")
469477
}
470-
if err := os.Chdir(newCwd); err != nil {
478+
if err := os.Chdir(cleanCwd); err != nil {
471479
return CreateErr(nil, err, "failed to change working directory to ACT_CWD/GITHUB_WORKSPACE")
472480
}
473481
defer func() {
@@ -1102,10 +1110,14 @@ func RunGraphFromString(ctx context.Context, graphName string, graphContent stri
11021110
}
11031111

11041112
func RunGraphFromFile(ctx context.Context, graphFile string, opts RunOpts, debugCb DebugCallback) error {
1105-
graphContent, err := os.ReadFile(graphFile)
1113+
cleanPath, err := utils.ValidatePath(graphFile)
1114+
if err != nil {
1115+
return CreateErr(nil, err, "invalid graph file path")
1116+
}
1117+
graphContent, err := os.ReadFile(cleanPath)
11061118
if err != nil {
11071119
if os.IsNotExist(err) {
1108-
err = fmt.Errorf("open %s: no such file or directory", graphFile)
1120+
err = fmt.Errorf("open %s: no such file or directory", cleanPath)
11091121
}
11101122

11111123
return CreateErr(nil, err, "failed loading graph")

‎nodes/credentials-ssh@v1.go‎

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,12 @@
11
package nodes
22

33
import (
4-
"github.com/actionforge/actrun-cli/core"
5-
64
_ "embed"
75
"os"
86

7+
"github.com/actionforge/actrun-cli/core"
98
ni "github.com/actionforge/actrun-cli/node_interfaces"
9+
"github.com/actionforge/actrun-cli/utils"
1010
)
1111

1212
//go:embed credentials-ssh@v1.yml
@@ -48,19 +48,22 @@ func (n *SshCredentialNode) OutputValueById(c *core.ExecutionState, outputId cor
4848
}
4949
expandedPath = homeDir + expandedPath[1:]
5050
}
51-
_, err = os.Stat(expandedPath)
52-
privateKeyInput = expandedPath
51+
cleanPath, pathErr := utils.ValidatePath(expandedPath)
52+
if pathErr != nil {
53+
return nil, core.CreateErr(c, pathErr, "invalid private key path")
54+
}
55+
_, err = os.Stat(cleanPath)
5356
if err == nil {
54-
keyBytes, readErr := os.ReadFile(privateKeyInput)
57+
keyBytes, readErr := os.ReadFile(cleanPath)
5558
if readErr != nil {
56-
return nil, core.CreateErr(c, readErr, "failed to read private key from path '%s'", privateKeyInput)
59+
return nil, core.CreateErr(c, readErr, "failed to read private key from path '%s'", cleanPath)
5760
}
5861
keyContent = string(keyBytes)
5962
if keyContent == "" {
6063
return nil, core.CreateErr(c, nil, "private key content is empty")
6164
}
6265
} else {
63-
return nil, core.CreateErr(c, err, "error checking path for private key '%s'", privateKeyInput)
66+
return nil, core.CreateErr(c, err, "error checking path for private key '%s'", cleanPath)
6467
}
6568

6669
credential := SshCredentials{

‎nodes/dir-create@v1.go‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66

77
"github.com/actionforge/actrun-cli/core"
88
ni "github.com/actionforge/actrun-cli/node_interfaces"
9+
"github.com/actionforge/actrun-cli/utils"
910
)
1011

1112
//go:embed dir-create@v1.yml
@@ -29,11 +30,16 @@ func (n *DirCreateNode) ExecuteImpl(c *core.ExecutionState, inputId core.InputId
2930
return err
3031
}
3132

33+
cleanPath, pathErr := utils.ValidatePath(path)
34+
if pathErr != nil {
35+
return core.CreateErr(c, pathErr, "invalid directory path")
36+
}
37+
3238
var mkdirErr error
3339
if mkdirAll {
34-
mkdirErr = os.MkdirAll(path, 0755)
40+
mkdirErr = os.MkdirAll(cleanPath, 0755)
3541
} else {
36-
mkdirErr = os.Mkdir(path, 0755)
42+
mkdirErr = os.Mkdir(cleanPath, 0755)
3743
}
3844

3945
if mkdirErr != nil {

‎nodes/dir-walk@v1.go‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99

1010
"github.com/actionforge/actrun-cli/core"
1111
ni "github.com/actionforge/actrun-cli/node_interfaces"
12+
"github.com/actionforge/actrun-cli/utils"
1213

1314
"golang.org/x/exp/maps"
1415
)
@@ -118,6 +119,11 @@ func walk(root string, opts walkOpts, pattern []string, items map[string]os.File
118119
}
119120
}
120121

122+
root, err = utils.ValidatePath(root)
123+
if err != nil {
124+
return "", core.CreateErr(nil, err, "invalid path")
125+
}
126+
121127
root, err = filepath.Abs(root)
122128
if err != nil {
123129
return "", core.CreateErr(nil, err, "failed to get absolute path")

‎nodes/docker-run@v1.go‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111

1212
"github.com/actionforge/actrun-cli/core"
1313
ni "github.com/actionforge/actrun-cli/node_interfaces"
14+
"github.com/actionforge/actrun-cli/utils"
1415
"github.com/google/uuid"
1516
)
1617

@@ -130,13 +131,19 @@ func (n *DockerNode) ExecuteImpl(c *core.ExecutionState, inputId core.InputId, p
130131
dockerfilePath = filepath.Join(cwd, dockerfilePath)
131132
}
132133

133-
if _, err := os.Stat(dockerfilePath); os.IsNotExist(err) {
134+
cleanPath, pathErr := utils.ValidatePath(dockerfilePath)
135+
if pathErr != nil {
136+
return core.CreateErr(c, pathErr, "invalid Dockerfile path")
137+
}
138+
139+
if _, err := os.Stat(cleanPath); os.IsNotExist(err) {
134140
// check if this looks like an image reference (user forgot docker:// prefix)
135141
if looksLikeImageReference(imageRef) {
136-
return core.CreateErr(c, nil, "Dockerfile not found: %s. Did you mean 'docker://%s' to pull from a registry?", dockerfilePath, imageRef)
142+
return core.CreateErr(c, nil, "Dockerfile not found: %s. Did you mean 'docker://%s' to pull from a registry?", cleanPath, imageRef)
137143
}
138-
return core.CreateErr(c, nil, "Dockerfile not found: %s", dockerfilePath)
144+
return core.CreateErr(c, nil, "Dockerfile not found: %s", cleanPath)
139145
}
146+
dockerfilePath = cleanPath
140147

141148
var containerIdSuffix string
142149
if core.IsTestE2eRunning() {

‎nodes/file-compress@v1.go‎

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import (
1616

1717
"github.com/actionforge/actrun-cli/core"
1818
ni "github.com/actionforge/actrun-cli/node_interfaces"
19+
"github.com/actionforge/actrun-cli/utils"
1920

2021
"golang.org/x/exp/maps"
2122
)
@@ -126,25 +127,29 @@ func createArchiveStreamFromPaths(c *core.ExecutionState, itemPaths []string, co
126127
dirSet := make(map[string]struct{})
127128

128129
for _, path := range itemPaths {
129-
stats, err := os.Lstat(path)
130+
cleanPath, pathErr := utils.ValidatePath(path)
131+
if pathErr != nil {
132+
return nil, core.CreateErr(c, pathErr, "invalid path: '%s'", path)
133+
}
134+
stats, err := os.Lstat(cleanPath)
130135
if err != nil {
131-
return nil, core.CreateErr(c, err, "failed to stat file: '%s'", path)
136+
return nil, core.CreateErr(c, err, "failed to stat file: '%s'", cleanPath)
132137
}
133138

134139
if stats.IsDir() {
135140
// ignore walking a directory that has already been walked
136-
if _, ok := dirSet[path]; ok {
141+
if _, ok := dirSet[cleanPath]; ok {
137142
continue
138143
}
139144

140145
tmpItemSet := make(map[string]os.FileInfo)
141-
path, err = walk(path, walkOpts{
146+
walkPath, err := walk(cleanPath, walkOpts{
142147
recursive: true,
143148
files: true,
144149
dirs: false,
145150
}, nil, tmpItemSet)
146151
if err != nil {
147-
return nil, core.CreateErr(c, err, "failed to walk directory: '%s'", path)
152+
return nil, core.CreateErr(c, err, "failed to walk directory: '%s'", walkPath)
148153
}
149154

150155
for k, v := range tmpItemSet {
@@ -158,7 +163,7 @@ func createArchiveStreamFromPaths(c *core.ExecutionState, itemPaths []string, co
158163
}
159164

160165
} else if stats.Mode().IsRegular() {
161-
itemSet[path] = stats
166+
itemSet[cleanPath] = stats
162167
}
163168
}
164169

‎nodes/file-read@v1.go‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77

88
"github.com/actionforge/actrun-cli/core"
99
ni "github.com/actionforge/actrun-cli/node_interfaces"
10+
"github.com/actionforge/actrun-cli/utils"
1011
)
1112

1213
//go:embed file-read@v1.yml
@@ -25,13 +26,18 @@ func (n *FileReadNode) ExecuteImpl(c *core.ExecutionState, inputId core.InputId,
2526
return err
2627
}
2728

28-
fp, err := os.Open(path)
29+
cleanPath, pathErr := utils.ValidatePath(path)
30+
if pathErr != nil {
31+
return core.CreateErr(c, pathErr, "invalid file path")
32+
}
33+
34+
fp, err := os.Open(cleanPath)
2935
if err != nil {
3036
return core.CreateErr(c, err)
3137
}
3238

3339
dsf := core.DataStreamFactory{
34-
SourcePath: path,
40+
SourcePath: cleanPath,
3541
Reader: fp,
3642
Length: core.GetReaderLength(fp),
3743
}
@@ -42,10 +48,10 @@ func (n *FileReadNode) ExecuteImpl(c *core.ExecutionState, inputId core.InputId,
4248
return err
4349
}
4450

45-
st, statErr := os.Stat(path)
51+
st, statErr := os.Stat(cleanPath)
4652
if statErr == nil {
4753
if st.IsDir() {
48-
statErr = core.CreateErr(c, nil, "stat %s: is a directory", path)
54+
statErr = core.CreateErr(c, nil, "stat %s: is a directory", cleanPath)
4955
}
5056
}
5157

0 commit comments

Comments
 (0)