Skip to content

fix/batch-changes: validate files target paths in changeset hooks and executor#1352

Open
cbrnrd wants to merge 1 commit into
mainfrom
carterbrainerd-vuln-104-changesethooks-files-validation-bypass-can-reach-executor
Open

fix/batch-changes: validate files target paths in changeset hooks and executor#1352
cbrnrd wants to merge 1 commit into
mainfrom
carterbrainerd-vuln-104-changesethooks-files-validation-bypass-can-reach-executor

Conversation

@cbrnrd

@cbrnrd cbrnrd commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Problem

src-cli rejects commas in top-level steps.files target paths because a comma breaks out of Docker --mount syntax and lets an attacker inject arbitrary source=/target= pairs (e.g. mounting /var/run/docker.sock). That same validation was missing for changesetHooks.*.steps[].files, so a malicious v3 batch spec could smuggle a dangerous files target path through a hook and reach the executor-side Docker mount sink.

The executor (src batch exec) makes this worse: it loads already-parsed Step structs from JSON and never re-runs ParseBatchSpec/validateHooks, so parse-time validation alone is a coordinator-only guard.

Solution

  • Add the missing comma check for files target paths in validateHooks (covers both onCIFailure and onMergeConflict).
  • Add defense-in-depth validation at the executor sink in createFilesToMount, so the check holds regardless of how a step reaches the executor.

Verification Evidence

$ go test ./internal/batches/service/ -run ParseBatchSpec -v
--- PASS: TestService_ParseBatchSpec/hook_files_target_path_with_comma

$ go test ./internal/batches/executor/ -run CreateFilesToMount -v
--- PASS: TestCreateFilesToMount_RejectsCommaInTargetPath

$ go test ./internal/batches/...
ok  	github.com/sourcegraph/src-cli/internal/batches/executor	2.402s
ok  	github.com/sourcegraph/src-cli/internal/batches/service	0.626s
(all batch packages pass)

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.

1 participant