Skip to content

fix(lint): remove nolint:gosec directives from templates and e2e tests #225

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
skevetter merged 7 commits into from
May 5, 2026
Merged

fix(lint): remove nolint:gosec directives from templates and e2e tests #225

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
2e50d06
fix(lint): remove nolint:gosec directives from templates and e2e tests
skevetter May 5, 2026
635e78c
fix(e2e): increase timeouts for container-pull-heavy E2E tests
skevetter May 5, 2026
75f7563
fix(e2e): increase timeout for shared-image feature test
skevetter May 5, 2026
b22a127
fix(e2e): increase timeout for feature http/cache download tests
skevetter May 5, 2026
cf318df
fix(e2e): blanket timeout upgrade for all up-features specs
skevetter May 5, 2026
3a0e4d0
fix(e2e): blanket timeout upgrade for all up-docker-compose specs
skevetter May 5, 2026
f811098
fix(ci): increase E2E test runner timeout from 900s to 1500s
skevetter May 5, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .github/workflows/pr-ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -454,7 +454,7 @@ jobs:
KUBECONFIG="${KUBECONFIG:-$HOME/.kube/config}" \
PATH="${PATH}" \
GOROOT="${GOROOT}" \
go test -v -ginkgo.v -timeout 900s --ginkgo.label-filter="${{ matrix.label }}"
go test -v -ginkgo.v -timeout 1500s --ginkgo.label-filter="${{ matrix.label }}"
else
GH_USERNAME="${GH_USERNAME}" \
GH_ACCESS_TOKEN="${GH_ACCESS_TOKEN}" \
Expand All @@ -463,7 +463,7 @@ jobs:
PATH="${PATH}" \
GOROOT="${GOROOT}" \
DOCKER_HOST="npipe:////./pipe/podman-machine-default" \
go test -v -ginkgo.v -timeout 900s --ginkgo.label-filter="${{ matrix.label }}"
go test -v -ginkgo.v -timeout 1500s --ginkgo.label-filter="${{ matrix.label }}"
fi

- name: verify docker is installed
Expand Down
45 changes: 33 additions & 12 deletions cmd/templates/apply.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,12 @@ func parseTemplateArgs(args []string, metadata *TemplateMetadata) map[string]str
}

func applyTemplateSubstitution(templateDir string, vars map[string]string) error {
root, err := os.OpenRoot(templateDir)
if err != nil {
return fmt.Errorf("open template root: %w", err)
}
defer func() { _ = root.Close() }()

return filepath.Walk(templateDir, func(path string, info os.FileInfo, err error) error {
if err != nil {
return err
Expand All @@ -230,12 +236,19 @@ func applyTemplateSubstitution(templateDir string, vars map[string]string) error
return nil
}

return substituteFileVars(path, info, vars)
relPath, err := filepath.Rel(templateDir, path)
if err != nil {
return err
}

return substituteFileVars(root, relPath, info, vars)
})
}

func substituteFileVars(path string, info os.FileInfo, vars map[string]string) error {
data, err := os.ReadFile(filepath.Clean(path))
func substituteFileVars(
root *os.Root, relPath string, info os.FileInfo, vars map[string]string,
) error {
data, err := root.ReadFile(relPath)
if err != nil {
return err
}
Expand All @@ -252,8 +265,7 @@ func substituteFileVars(path string, info os.FileInfo, vars map[string]string) e
}

if modified {
//nolint:gosec // path is from filepath.Walk within temp dir
return os.WriteFile(path, []byte(content), info.Mode())
return root.WriteFile(relPath, []byte(content), info.Mode())
}

return nil
Expand All @@ -264,7 +276,13 @@ func copyTemplateFiles(srcDir, destDir string, omitPaths []string) error {
return fmt.Errorf("create workspace folder: %w", err)
}

copier := &templateCopier{destDir: destDir, omitPaths: omitPaths}
destRoot, err := os.OpenRoot(destDir)
if err != nil {
return fmt.Errorf("open dest root: %w", err)
}
defer func() { _ = destRoot.Close() }()

copier := &templateCopier{destDir: destDir, destRoot: destRoot, omitPaths: omitPaths}

return filepath.Walk(srcDir, func(path string, info os.FileInfo, err error) error {
if err != nil {
Expand All @@ -286,6 +304,7 @@ func copyTemplateFiles(srcDir, destDir string, omitPaths []string) error {

type templateCopier struct {
destDir string
destRoot *os.Root
omitPaths []string
}

Expand All @@ -304,21 +323,23 @@ func (c *templateCopier) copyEntry(path, relPath string, info os.FileInfo) error
return os.MkdirAll(destPath, 0o750)
}

return copyFile(path, destPath, info.Mode())
return copyFile(path, c.destRoot, relPath, info.Mode())
}

func copyFile(srcPath, destPath string, mode os.FileMode) error {
func copyFile(srcPath string, destRoot *os.Root, destRelPath string, mode os.FileMode) error {
data, err := os.ReadFile(filepath.Clean(srcPath))
if err != nil {
return err
}

if err := os.MkdirAll(filepath.Dir(destPath), 0o750); err != nil {
return err
destDir := filepath.Dir(destRelPath)
if destDir != "." {
if err := destRoot.Mkdir(destDir, 0o750); err != nil && !os.IsExist(err) {
return err
}
}

//nolint:gosec // mode from source template, path constructed from destDir+relPath
return os.WriteFile(destPath, data, mode)
return destRoot.WriteFile(destRelPath, data, mode&0o666)
}
Comment on lines +329 to 343

@coderabbitai coderabbitai Bot May 5, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Look for executable scripts in any in-repo template fixtures, and any lifecycle
# hooks that invoke scripts directly (./...) rather than via an interpreter.
rg -nP --type=json -C2 '"(postCreate|postStart|onCreate|initialize)Command"\s*:\s*"\s*\./'
fd -t f -e sh -x sh -c 'printf "%s %s\n" "$(stat -c %a "$1" 2>/dev/null || stat -f %A "$1")" "$1"' _ {}

Repository: devsy-org/devsy

Length of output: 3898

🏁 Script executed:

# Search for devcontainer.json or template config files
fd -e json | head -20

Repository: devsy-org/devsy

Length of output: 1021

🏁 Script executed:

# Check the context of copyFile - where is it called from?
rg -n "copyFile" cmd/templates/apply.go

Repository: devsy-org/devsy

Length of output: 219

🏁 Script executed:

# Look for any devcontainer.json files that might have lifecycle hooks
fd -name "devcontainer.json" -o -name "*.devcontainer" -type f

Repository: devsy-org/devsy

Length of output: 290

🏁 Script executed:

# Search for postCreateCommand, postStartCommand patterns in all files
rg -i "postCreateCommand|postStartCommand|onCreateCommand" --type=json -A2

Repository: devsy-org/devsy

Length of output: 8657

🏁 Script executed:

# Look at the context around copyFile - what is it part of?
sed -n '220,350p' cmd/templates/apply.go

Repository: devsy-org/devsy

Length of output: 3005

🏁 Script executed:

# Check if there are features or template fixture scripts that expect to be executable
fd -path "*/features/*/install.sh" | head -5 | xargs -I {} sh -c 'echo "File: {}"; head -5 "{}"'

Repository: devsy-org/devsy

Length of output: 228

🏁 Script executed:

# Search for any features that might reference scripts in lifecycle hooks
fd -name "devcontainer-feature.json" | xargs grep -l "postCreateCommand\|postStartCommand\|onCreateCommand" 2>/dev/null

Repository: devsy-org/devsy

Length of output: 292

🏁 Script executed:

# Check devcontainer-feature.json files for lifecycle hooks or script references
fd -name "devcontainer-feature.json" -type f | head -10 | xargs cat 2>/dev/null | grep -A5 -B5 "postCreateCommand\|postStartCommand\|onCreateCommand"

Repository: devsy-org/devsy

Length of output: 290

🏁 Script executed:

# Check if any feature's install.sh invokes other scripts directly
fd -path "*/features/*/install.sh" -type f | head -3 | xargs cat 2>/dev/null

Repository: devsy-org/devsy

Length of output: 228

🏁 Script executed:

# Look at apply.go more carefully - what calls copyTemplateFiles and where do templates come from?
rg -n "copyTemplateFiles" cmd/templates/

Repository: devsy-org/devsy

Length of output: 270

🏁 Script executed:

# Find what templates are used - search for template references
rg -i "template" cmd/templates/apply.go | head -20

Repository: devsy-org/devsy

Length of output: 1059

🏁 Script executed:

# Look at the apply function and understand the overall context
sed -n '1,120p' cmd/templates/apply.go

Repository: devsy-org/devsy

Length of output: 3137

🏁 Script executed:

# Check if there are any example templates or fixtures with scripts
fd -path "*example*" -name "*.sh" -o -path "*fixture*" -name "*.sh" -o -path "*template*" -name "*.sh" | head -10

Repository: devsy-org/devsy

Length of output: 228

🏁 Script executed:

# Check for devcontainer-feature.json content to understand feature install scripts
fd "devcontainer-feature.json" | xargs cat 2>/dev/null | head -50

Repository: devsy-org/devsy

Length of output: 1275

🏁 Script executed:

# Look for git history or comments around the mode masking in copyFile
rg -B5 -A5 "0o666" cmd/templates/apply.go

Repository: devsy-org/devsy

Length of output: 368

🏁 Script executed:

# Check if there's any security or permission-related documentation
fd -name "README*" -o -name "SECURITY*" | xargs cat 2>/dev/null | grep -i "permission\|execute\|mode" | head -20

Repository: devsy-org/devsy

Length of output: 290

🏁 Script executed:

# Look for any tests or examples that verify executable scripts are preserved or stripped
rg -i "executable\|0o755\|0o644" cmd/ --type go

Repository: devsy-org/devsy

Length of output: 41

🏁 Script executed:

# Check if there are any comments or docs about template security model
rg -i "template.*security\|security.*template" --type=markdown --type=go

Repository: devsy-org/devsy

Length of output: 41

Executable bits are stripped by mode & 0o666—potential regression for template scripts.

copyFile masks the file mode to 0o666, which strips all execute bits. This affects any devcontainer template that ships executable scripts from the OCI layer. While the test fixtures in this repository avoid direct script invocation (using sh -c wrappers instead), external templates pulled from OCI registries could include executable scripts referenced in lifecycle hooks or configuration. Those scripts will land in the workspace without execute permissions, breaking invocations like ./script.sh.

Two considerations:

  1. If preserving execute is acceptable: Use mode & 0o755 to retain owner execute while capping group/other write.
  2. Inconsistency: 0o666 allows group + other write (permissive) while stripping execute (restrictive). If the intent is security hardening, consider 0o644 or (mode & 0o777) &^ 0o022 for consistent file-level restrictions.
Suggested fix 
-	return destRoot.WriteFile(destRelPath, data, mode&0o666)
+	// Preserve owner exec for scripts; cap group/other to read-only.
+	return destRoot.WriteFile(destRelPath, data, mode&0o755)
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@cmd/templates/apply.go` around lines 329 - 343, The copyFile function
currently strips execute bits by masking mode with 0o666; update the final write
permission calculation (used in destRoot.WriteFile call) to preserve execute
bits — e.g., replace the mask with mode & 0o755 so owner execute is retained
while capping group/other writes; if stricter non-executable defaults are
desired instead, consider using 0o644 or computing (mode & 0o777) &^ 0o022 to
apply a consistent umask-like restriction before calling
copyFile/destRoot.WriteFile.


func shouldOmit(relPath string, omitPaths []string) bool {
Expand Down
16 changes: 13 additions & 3 deletions cmd/templates/generate_docs.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,13 @@ type discoveredTemplate struct {
func discoverTemplates(projectFolder string) ([]discoveredTemplate, error) {
var templates []discoveredTemplate

err := filepath.Walk(projectFolder, func(path string, info os.FileInfo, err error) error {
root, err := os.OpenRoot(projectFolder)
if err != nil {
return nil, fmt.Errorf("open project root: %w", err)
}
defer func() { _ = root.Close() }()

err = filepath.Walk(projectFolder, func(path string, info os.FileInfo, err error) error {
if err != nil {
return err
}
Expand All @@ -96,8 +102,12 @@ func discoverTemplates(projectFolder string) ([]discoveredTemplate, error) {
return nil
}

//nolint:gosec // path from filepath.Walk within project folder
data, err := os.ReadFile(filepath.Clean(path))
relPath, err := filepath.Rel(projectFolder, path)
if err != nil {
return err
}

data, err := root.ReadFile(relPath)
if err != nil {
return err
}
Expand Down
4 changes: 3 additions & 1 deletion e2e/tests/readconfiguration/readconfiguration.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -319,7 +319,9 @@ var _ = ginkgo.Describe("read-configuration command", ginkgo.Label("read-configu
framework.ExpectNoError(err)
containerID := strings.TrimSpace(string(out))
ginkgo.DeferCleanup(func() {
_ = exec.Command("docker", "rm", "-f", containerID).Run() //nolint:gosec // G204
args := []string{"rm", "-f", containerID}
cmd := exec.Command("docker", args...) // #nosec G204
_ = cmd.Run()
})

stdout, _, err := f.ExecCommandCapture(ctx, []string{
Expand Down
4 changes: 2 additions & 2 deletions e2e/tests/templates/templates.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ var _ = ginkgo.Describe("templates command", ginkgo.Label("templates"), func() {
gomega.Expect(err).NotTo(gomega.HaveOccurred(),
"devcontainer.json should exist after apply")

data, err := os.ReadFile(devcontainerPath) //nolint:gosec // test path from MkdirTemp
data, err := os.ReadFile(filepath.Clean(devcontainerPath))
framework.ExpectNoError(err)

var config map[string]any
Expand All @@ -70,7 +70,7 @@ var _ = ginkgo.Describe("templates command", ginkgo.Label("templates"), func() {
framework.ExpectNoError(err)

devcontainerPath := filepath.Join(tempDir, ".devcontainer", "devcontainer.json")
data, err := os.ReadFile(devcontainerPath) //nolint:gosec // test path from MkdirTemp
data, err := os.ReadFile(filepath.Clean(devcontainerPath))
framework.ExpectNoError(err)

var config map[string]any
Expand Down
34 changes: 17 additions & 17 deletions e2e/tests/up-docker-compose/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ var _ = ginkgo.Describe(
)
framework.ExpectNoError(err)
framework.ExpectNoError(tc.verifyWorkspaceMount(ctx, workspace, tempDir))
}, ginkgo.SpecTimeout(framework.TimeoutModerate()))
}, ginkgo.SpecTimeout(framework.TimeoutLong()))

ginkgo.It("sub-folder", func(ctx context.Context) {
tempDir, workspace, err := tc.setupAndStartWorkspace(
Expand All @@ -53,7 +53,7 @@ var _ = ginkgo.Describe(
)
framework.ExpectNoError(err)
framework.ExpectNoError(tc.verifyWorkspaceMount(ctx, workspace, tempDir))
}, ginkgo.SpecTimeout(framework.TimeoutModerate()))
}, ginkgo.SpecTimeout(framework.TimeoutLong()))

ginkgo.It("overrides", func(ctx context.Context) {
tempDir, workspace, err := tc.setupAndStartWorkspace(
Expand All @@ -62,7 +62,7 @@ var _ = ginkgo.Describe(
)
framework.ExpectNoError(err)
framework.ExpectNoError(tc.verifyWorkspaceMount(ctx, workspace, tempDir))
}, ginkgo.SpecTimeout(framework.TimeoutModerate()))
}, ginkgo.SpecTimeout(framework.TimeoutLong()))

ginkgo.It("env-file", func(ctx context.Context) {
tempDir, err := setupWorkspace(
Expand Down Expand Up @@ -92,7 +92,7 @@ var _ = ginkgo.Describe(
gomega.Expect(ids).To(gomega.HaveLen(1), "1 compose container to be created")
gomega.Expect(devsyUpOutput).
NotTo(gomega.ContainSubstring("Defaulting to a blank string."))
}, ginkgo.SpecTimeout(framework.TimeoutModerate()))
}, ginkgo.SpecTimeout(framework.TimeoutLong()))

ginkgo.It("restart", func(ctx context.Context) {
tempDir, err := setupWorkspace(
Expand Down Expand Up @@ -133,7 +133,7 @@ var _ = ginkgo.Describe(
)
framework.ExpectNoError(err)
gomega.Expect(restartIds).To(gomega.HaveLen(1), "1 compose container after restart")
}, ginkgo.SpecTimeout(framework.TimeoutModerate()))
}, ginkgo.SpecTimeout(framework.TimeoutLong()))

ginkgo.It("environment variables", func(ctx context.Context) {
_, workspace, err := tc.setupAndStartWorkspace(
Expand All @@ -160,7 +160,7 @@ var _ = ginkgo.Describe(
[]string{"ssh", "--command", "echo $FOO", workspace.ID},
)
framework.ExpectNoError(err)
}, ginkgo.SpecTimeout(framework.TimeoutModerate()))
}, ginkgo.SpecTimeout(framework.TimeoutLong()))

ginkgo.It("user", func(ctx context.Context) {
_, workspace, err := tc.setupAndStartWorkspace(
Expand All @@ -187,7 +187,7 @@ var _ = ginkgo.Describe(
[]string{"ssh", "--command", "ps u -p 1", workspace.ID},
)
framework.ExpectNoError(err)
}, ginkgo.SpecTimeout(framework.TimeoutModerate()))
}, ginkgo.SpecTimeout(framework.TimeoutLong()))

ginkgo.It("override command", func(ctx context.Context) {
_, workspace, err := tc.setupAndStartWorkspace(
Expand All @@ -201,7 +201,7 @@ var _ = ginkgo.Describe(
gomega.Expect(detail.Config.Entrypoint).
NotTo(gomega.ContainElement("bash"), "overrides container entry point")
gomega.Expect(detail.Config.Cmd).To(gomega.BeEmpty(), "overrides container command")
}, ginkgo.SpecTimeout(framework.TimeoutModerate()))
}, ginkgo.SpecTimeout(framework.TimeoutLong()))

ginkgo.It(
"implements updateRemoteUserUID with root container user",
Expand Down Expand Up @@ -265,7 +265,7 @@ var _ = ginkgo.Describe(
verifyHostFileAccess(hostFile, expectedContent)
verifyHostFileOwnership(hostFile, testUID, testGID, testUID == 0)
},
ginkgo.SpecTimeout(framework.TimeoutModerate()),
ginkgo.SpecTimeout(framework.TimeoutVeryLong()),
)

ginkgo.It(
Expand Down Expand Up @@ -323,7 +323,7 @@ var _ = ginkgo.Describe(
verifyHostFileAccess(hostFile, expectedContent)
verifyHostFileOwnership(hostFile, testUID, testGID, testUID == 0)
},
ginkgo.SpecTimeout(framework.TimeoutModerate()),
ginkgo.SpecTimeout(framework.TimeoutVeryLong()),
)

ginkgo.It("privileged", func(ctx context.Context) {
Expand All @@ -337,7 +337,7 @@ var _ = ginkgo.Describe(
framework.ExpectNoError(err)
gomega.Expect(detail.HostConfig.Privileged).
To(gomega.BeTrue(), "container run with privileged true")
}, ginkgo.SpecTimeout(framework.TimeoutModerate()))
}, ginkgo.SpecTimeout(framework.TimeoutLong()))

ginkgo.It("capabilities", func(ctx context.Context) {
_, workspace, err := tc.setupAndStartWorkspace(
Expand All @@ -354,7 +354,7 @@ var _ = ginkgo.Describe(
gomega.Expect(detail.HostConfig.CapAdd).
To(gomega.Or(gomega.ContainElement("NET_ADMIN"), gomega.ContainElement("CAP_NET_ADMIN")),
"devcontainer configuration can add capabilities")
}, ginkgo.SpecTimeout(framework.TimeoutModerate()))
}, ginkgo.SpecTimeout(framework.TimeoutLong()))

ginkgo.It("security options", func(ctx context.Context) {
_, workspace, err := tc.setupAndStartWorkspace(
Expand All @@ -369,7 +369,7 @@ var _ = ginkgo.Describe(
To(gomega.ContainElement("seccomp=unconfined"), "securityOpts contain seccomp=unconfined")
gomega.Expect(detail.HostConfig.SecurityOpt).
To(gomega.ContainElement("apparmor=unconfined"), "securityOpts contain apparmor=unconfined")
}, ginkgo.SpecTimeout(framework.TimeoutModerate()))
}, ginkgo.SpecTimeout(framework.TimeoutLong()))

ginkgo.It("remote env", func(ctx context.Context) {
_, workspace, err := tc.setupAndStartWorkspace(
Expand Down Expand Up @@ -405,7 +405,7 @@ var _ = ginkgo.Describe(
[]string{"ssh", "--command", "cat $HOME/remote-env.out", workspace.ID},
)
framework.ExpectNoError(err)
}, ginkgo.SpecTimeout(framework.TimeoutModerate()))
}, ginkgo.SpecTimeout(framework.TimeoutLong()))

ginkgo.It("remote env null unsets variable", func(ctx context.Context) {
_, workspace, err := tc.setupAndStartWorkspace(
Expand Down Expand Up @@ -454,7 +454,7 @@ var _ = ginkgo.Describe(
},
)
framework.ExpectNoError(err)
}, ginkgo.SpecTimeout(framework.TimeoutModerate()))
}, ginkgo.SpecTimeout(framework.TimeoutLong()))

ginkgo.It("remote user", func(ctx context.Context) {
_, workspace, err := tc.setupAndStartWorkspace(
Expand All @@ -481,7 +481,7 @@ var _ = ginkgo.Describe(
[]string{"ssh", "--command", "cat $HOME/remote-user.out", workspace.ID},
)
framework.ExpectNoError(err)
}, ginkgo.SpecTimeout(framework.TimeoutModerate()))
}, ginkgo.SpecTimeout(framework.TimeoutLong()))

ginkgo.It("variables substitution", func(ctx context.Context) {
tempDir, workspace, err := tc.setupAndStartWorkspace(
Expand Down Expand Up @@ -544,6 +544,6 @@ var _ = ginkgo.Describe(
)
framework.ExpectNoError(err)
gomega.Expect(containerWorkspaceFolderBasename).To(gomega.Equal("workspaces"))
}, ginkgo.SpecTimeout(framework.TimeoutModerate()))
}, ginkgo.SpecTimeout(framework.TimeoutLong()))
},
)
Loading
Loading