Skip to content

fix/batches: prevent workspace path injection - #1372

Merged
cbrnrd merged 4 commits into
mainfrom
carterbrainerd-vuln-143-src-batch-volume-mode-a-server-supplied-workspacepath-is
Sep 9, 2026
Merged

fix/batches: prevent workspace path injection#1372
cbrnrd merged 4 commits into
mainfrom
carterbrainerd-vuln-143-src-batch-volume-mode-a-server-supplied-workspacepath-is

Conversation

@cbrnrd

@cbrnrd cbrnrd commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Problem

In volume workspace mode, src batch put a workspace file name directly into a Docker mount specification and a shell command. Workspace paths can come from directory names in repository content. A crafted directory name could therefore change the mount specification and run shell commands during workspace setup.

The same server-provided data was also trusted in two related places:

  • In bind workspace mode, a path containing .. could copy .gitignore or .gitattributes outside the workspace and change permissions on a host directory.
  • Docker bind mount source paths were joined into mount specifications without validation. A comma in server-provided repository metadata could add or replace mount fields.

This fixes VULN-143 and the related issues identified while verifying the fix.

Solution

Use fixed, generated paths for temporary Docker mounts and pass workspace destination names as shell arguments instead of adding them to the shell command.

Reject additional workspace file paths that resolve outside the workspace in both bind and volume modes.

Move Docker bind mount construction into a shared helper that validates every source and target before building the mount specification. Use that helper for all bind mounts in the batches code.

Verification Evidence

  • The reporter verified the original fix before and after, including an end-to-end run against a stock Sourcegraph instance and the original proof-of-concept payloads.
  • Added regression coverage for Docker mount injection and shell injection through workspace file names.
  • Added a bind-mode traversal test that confirms no file is written outside the workspace and existing directory permissions are unchanged.
  • Added tests that reject unsafe archive paths, additional-file paths, and Docker mount sources and targets.
  • Ran go test ./internal/batches/... successfully.

@cbrnrd
cbrnrd marked this pull request as ready for review September 8, 2026 15:20
@cbrnrd
cbrnrd requested a review from a team September 8, 2026 15:20
@DaedalusG

Copy link
Copy Markdown
Contributor

I believe #1373 fixes the same issue but its a better fix
https://sourcegraph.sourcegraph.com/deepsearch/a3f99b21-7c63-43a4-858f-465124bf57a4

@cbrnrd
cbrnrd requested a review from DaedalusG September 9, 2026 13:21
@cbrnrd

cbrnrd commented Sep 9, 2026

Copy link
Copy Markdown
Contributor Author

@DaedalusG there were some other places that needed a fix, #1373 does fix the original issue but now there's more that PR doesn't cover.

@cbrnrd
cbrnrd enabled auto-merge (squash) September 9, 2026 18:23
@cbrnrd
cbrnrd merged commit bc8db51 into main Sep 9, 2026
8 checks passed
@cbrnrd
cbrnrd deleted the carterbrainerd-vuln-143-src-batch-volume-mode-a-server-supplied-workspacepath-is branch September 9, 2026 18:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants