Skip to content

fix(images/code): run gopls install as sandbox user - #1905

Merged
rh-hemartin merged 1 commit into
mainfrom
fix/sandbox-go-ownership
Jun 8, 2026
Merged

fix(images/code): run gopls install as sandbox user#1905
rh-hemartin merged 1 commit into
mainfrom
fix/sandbox-go-ownership

Conversation

@rh-hemartin

Copy link
Copy Markdown
Member

Summary

  • Moves go install gopls after USER sandbox in the code agent Containerfile
  • go install now creates /sandbox/go owned by sandbox instead of root
  • Removes the explicit GOPATH/GOMODCACHE env vars set before the user switch — Go's defaults ($HOME/go) work correctly once the user is right

Test plan

Fixes #1904

@github-actions

github-actions Bot commented Jun 4, 2026

Copy link
Copy Markdown

Site preview

Preview: https://744abf30-site.fullsend-ai.workers.dev

Commit: ab61e7fe976046721db6763dc77cd9cd493bffac

@fullsend-ai-review

fullsend-ai-review Bot commented Jun 4, 2026

Copy link
Copy Markdown

Review

Findings

Medium

  • [content-sandboxing] images/code/Containerfile:117 — gopls is now installed as the sandbox user, placing the binary under /sandbox/go/bin which is writable at runtime (read_write: [/sandbox] in policies/code.yaml). Previously, gopls was installed as root into /usr/local/go/bin with GOBIN=/usr/local/go/bin, which is protected by the read_only: [/usr] policy. A compromised or misbehaving agent could overwrite the gopls binary with an arbitrary executable that would then be invoked by Claude Code's LSP integration. Consider a two-stage approach: install gopls as root with GOBIN=/usr/local/go/bin (as before), then separately fix /sandbox/go ownership by creating the directory tree as root and chowning it to sandbox before the USER sandbox directive (e.g., RUN mkdir -p /sandbox/go/pkg/mod && chown -R sandbox:sandbox /sandbox/go). This preserves gopls in the tamper-proof location while enabling Go auto-download at runtime.

Low

  • [implicit-contract] images/code/Containerfile:117 — The ENV PATH hardcodes /sandbox/go/bin while go install defaults GOBIN to $HOME/go/bin. These match only if the sandbox user's HOME is /sandbox (set by the OpenShell base image, uid 998). The build is self-validating (gopls version fails if the paths diverge), so this is not a silent bug, but the coupling is implicit. Consider retaining explicit GOPATH="/sandbox/go" alongside the PATH declaration, or adding a comment documenting the HOME assumption. See also: [content-sandboxing] finding at this location.

  • [content-sandboxing] internal/cli/run.go:875 — PATH now unconditionally includes /usr/local/go/bin and $HOME/go/bin for all sandbox types, not just when Go plugins are configured. The /usr/local/go/bin addition is benign (read-only mount), but $HOME/go/bin is writable and extends the PATH attack surface. This is necessary for finding gopls in its new location but worth noting.

Info

  • [documentation-pattern] images/code/Containerfile:115 — The gopls section is now the only tool installed after USER sandbox, but the comment block doesn't explain why. A brief note (e.g., "Installed as sandbox user so /sandbox/go is owned by sandbox, enabling Go toolchain auto-downloads at runtime — see fix/code agent: /sandbox/go owned by root blocks Go toolchain auto-download #1904") would help future maintainers understand the exception to the root-install pattern used for all other tools.
Previous run

Review

Findings

Medium

  • [content-sandboxing] images/code/Containerfile:117 — gopls binary moved from read-only /usr/local/go/bin (protected by the sandbox read-only mount policy on /usr, documented at line 119–120 of this Containerfile) to writable /sandbox/go/bin. The old location prevented runtime tampering; the new location is under /sandbox, which is writable by the sandbox user. A compromised or misbehaving agent could replace the gopls binary with an arbitrary executable, which would then be invoked by Claude Code's LSP integration with the agent's full permissions. Consider copying gopls to a read-only path after installation, or adding /sandbox/go/bin to the sandbox policy's read-only mount list.

Low

  • [implicit-contract] images/code/Containerfile:117 — The PR removes the explicit GOPATH and GOMODCACHE environment variables, relying on Go's default of $HOME/go. This creates an implicit dependency on the sandbox user's HOME being /sandbox (confirmed in the OpenShell base image, uid 998). The old explicit ENV vars were resilient to base image changes. Consider retaining GOPATH="/sandbox/go" and GOMODCACHE="/sandbox/go/pkg/mod" alongside the PATH declaration, or adding an explicit ENV HOME=/sandbox to lock the assumption. See also: [content-sandboxing] finding at this location.

  • [documentation-clarity] images/code/Containerfile:115 — The gopls comment block was moved after USER sandbox but doesn't explain why this tool is installed as the sandbox user, unlike every other tool in the file. Since this is the only post-USER sandbox installation and the whole point of the PR (fix/code agent: /sandbox/go owned by root blocks Go toolchain auto-download #1904), a brief comment (e.g., "Installed as sandbox user so /sandbox/go is owned by sandbox, allowing Go toolchain auto-download at runtime") would help future maintainers understand the exception.

Previous run

Review

Findings

Medium

  • [content-sandboxing] images/code/Containerfile:117 — gopls binary moved from read-only /usr/local/go/bin (protected by the sandbox read-only mount policy on /usr, documented at line 119–120 of this Containerfile) to writable /sandbox/go/bin. The old location prevented runtime tampering; the new location is under /sandbox, which is writable by the sandbox user. A compromised or misbehaving agent could replace the gopls binary with an arbitrary executable, which would then be invoked by Claude Code's LSP integration with the agent's full permissions. Consider copying gopls to a read-only path after installation, or adding /sandbox/go/bin to the sandbox policy's read-only mount list.

Low

  • [implicit-contract] images/code/Containerfile:117 — The PR removes the explicit GOPATH and GOMODCACHE environment variables, relying on Go's default of $HOME/go. This creates an implicit dependency on the sandbox user's HOME being /sandbox (confirmed in the OpenShell base image, uid 998). The old explicit ENV vars were resilient to base image changes. Consider retaining GOPATH="/sandbox/go" and GOMODCACHE="/sandbox/go/pkg/mod" alongside the PATH declaration, or adding an explicit ENV HOME=/sandbox to lock the assumption. See also: [content-sandboxing] finding at this location.

  • [documentation-clarity] images/code/Containerfile:115 — The gopls comment block was moved after USER sandbox but doesn't explain why this tool is installed as the sandbox user, unlike every other tool in the file. Since this is the only post-USER sandbox installation and the whole point of the PR (fix/code agent: /sandbox/go owned by root blocks Go toolchain auto-download #1904), a brief comment (e.g., "Installed as sandbox user so /sandbox/go is owned by sandbox, allowing Go toolchain auto-download at runtime") would help future maintainers understand the exception.

@fullsend-ai-review fullsend-ai-review Bot added the requires-manual-review Review requires human judgment label Jun 4, 2026
Move gopls install after USER sandbox so go install creates
/sandbox/go owned by sandbox, not root. Fixes permission denied
when Go auto-downloads a newer toolchain from go.mod constraints.

Fixes #1904

Signed-off-by: Hector Martinez <hemartin@redhat.com>
@rh-hemartin
rh-hemartin force-pushed the fix/sandbox-go-ownership branch from d9611eb to ab61e7f Compare June 5, 2026 06:21
@rh-hemartin
rh-hemartin marked this pull request as ready for review June 5, 2026 06:22
@fullsend-ai-review fullsend-ai-review Bot added requires-manual-review Review requires human judgment and removed requires-manual-review Review requires human judgment labels Jun 5, 2026
@rh-hemartin
rh-hemartin added this pull request to the merge queue Jun 8, 2026
@rh-hemartin rh-hemartin self-assigned this Jun 8, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Jun 8, 2026
@rh-hemartin
rh-hemartin added this pull request to the merge queue Jun 8, 2026
Merged via the queue into main with commit 12e6c21 Jun 8, 2026
13 of 14 checks passed
@rh-hemartin
rh-hemartin deleted the fix/sandbox-go-ownership branch June 8, 2026 09:36
@fullsend-ai-retro

Copy link
Copy Markdown

Retro: PR #1905fix(images/code): run gopls install as sandbox user

Workflow went well overall. The review agent demonstrated strong quality in its challenger pass, correctly discarding a factually incorrect high-severity finding from its style sub-agent and appropriately downgrading over-reported findings. The medium security finding (gopls binary moving from read-only /usr/local/go/bin to writable /sandbox/go/bin) was legitimate, specific, and included a concrete mitigation.

Timeline

  1. 2026-06-04 15:49 — Human (rh-hemartin) opened PR fix(images/code): run gopls install as sandbox user #1905 fixing issue fix/code agent: /sandbox/go owned by root blocks Go toolchain auto-download #1904 (gopls ownership problem)
  2. 2026-06-04 15:57 — Review agent posted findings: 1 medium (content-sandboxing), 2 low (implicit HOME contract, missing comment). Action: comment (non-blocking)
  3. 2026-06-05 06:22 — Second review triggered on new SHA; found same core issues plus one additional finding in run.go. One of two simultaneous dispatches was properly cancelled
  4. 2026-06-05 18:28 — Human (ralphbean) approved the PR without addressing the medium security finding
  5. 2026-06-08 09:36 — PR merged

What went well

  • Challenger pass quality: The orchestrator correctly identified and discarded a factually wrong claim that Docker ENV behaves differently after USER switch. It also consolidated 3 independent sub-agent findings about the removed GOPATH/GOMODCACHE vars into a single low-severity finding after verifying the HOME=/sandbox assumption in the base image.
  • Second review was justified: Different SHA, additional finding discovered, refined recommendations.
  • Duplicate dispatch handling: Concurrent pull_request_target events were properly deduplicated (one cancelled).

Known gaps (already tracked)

  • Medium findings don't gate merge — The security concern was posted as comment (non-blocking) and the human approved without addressing it. Covered by #870 (medium findings should trigger fix agent or block merge) and #1956 (auto-file tracking issues for unresolved medium+ findings).
  • Duplicate review dispatches — Two events fired 4 seconds apart. Covered by #1452 (deduplicate review dispatches for same HEAD SHA).

No new proposals filed — all identified improvement opportunities are already covered by existing open issues.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

requires-manual-review Review requires human judgment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix/code agent: /sandbox/go owned by root blocks Go toolchain auto-download

2 participants