fix(acq): use sandbox workspace for github scoping

[mogul]

Context

acq github-scope SANDBOX previously defaulted the workspace path to the caller's current directory. That could suggest repositories unrelated to the sandbox when the command was run from a parent directory. The flow also printed one PAT URL per GitHub owner but accepted only one token, even though fine-grained PATs are single-owner and acq stores one sandbox-scoped GitHub token today.

Changes

• Default pathless acq github-scope SANDBOX to the sandbox's recorded workspace instead of $PWD.
• Persist the primary workspace path for new SBX and MSB sandboxes.
• Add fallback workspace discovery for existing sandboxes:
  • SBX reads ACQ_WORKSPACE from the guest.
  • MSB reads the existing /var/lib/acq/workspace marker.
• Refuse multi-owner GitHub workspaces with a clear explanation instead of asking for one token after showing several owner URLs.
• Update help text, how-to docs, and ADR-0013 to document the one-owner constraint.
• Add regression tests for multi-owner refusal, one-owner multi-repo support, and recorded-workspace lookup.
• Isolate host git identity in an existing SBX heal test so its assertion remains focused on the playbook stale-probe behavior.

Verification

• git diff --check passed.
• npm run lint:md passed with 0 issues.
• printf '%s\n' acq acq.backends/*.sh scripts/test-acq-lib.sh scripts/test-acq-bats test/bats/*.bats test/bats/helper.bash | xargs -r -n8 shellcheck --severity=warning passed.
• ./scripts/test-acq-bats passed: 477/477.

Rollback

Revert commit c4a66c9 to restore the prior github-scope behavior.

Security Impact

Improves least-privilege GitHub token guidance by preventing accidental scoping to unrelated repositories and failing closed for unsupported multi-owner PAT flows. No secret values are logged or placed on argv. AI-assisted.

[wz-gsa]

AI-assisted review (OpenCode), advisory — needs a human to confirm before it drives a change. Reviewed at head c5c2672; also read #504 at a699af38.

The defect this fixes is real: scoping a token to whatever repos happen to sit in the caller's $PWD is wrong, and the multi-owner refusal is a good addition. Three things about the new fallback, one of which is a collision with #504 rather than a defect here.

1. It collides with #504, and the resolution matters for more than merge order.

#504 now carries ADR-0030 and its implementation, and commit 3 migrates the workspace marker off the guest:

-  ws=$({ _acq_msb_cli exec "$name" -u 0 -- sh -c 'cat /var/lib/acq/workspace …
+  ws=$(acq_host_config_read msb "$name" workspace | tr -d '\r\n')

Both PRs modify _acq_msb_workspace_for, so they will conflict. Worth flagging explicitly because the msb half of this PR's fallback chain reads a value ADR-0030 has just classified as guest-tamperable — whichever lands second should not reintroduce the guest read.

2. The sbx half is not covered by that migration (acq.backends/sbx.sh:572-574).

acq_backend_workspace_for() {
  sbx exec "$1" -- sh -c 'printf "%s" "${ACQ_WORKSPACE:-}"' </dev/null 2>/dev/null || true
}

I grepped #504's diff for acq_backend_workspace_for and ACQ_WORKSPACE — no hits in sbx.sh. So this read survives ADR-0030's migration. It takes a string from inside the sandbox and uses it as a host path: acq:1414 assigns it, acq:1425 passes it to github_scope_sandbox → detect_workspace_repos, which walks the path and enumerates remote.origin.url for every repo it finds, then drives the user through minting a fine-grained PAT for them.

Nothing validates it in between — no absolute-path check, no -d, and notably not preflight_workspace_path, which this file already applies on create (acq:901, acq:962) but not here. Given ADR-0030's own premise (the agent has passwordless sudo, so guest root-ownership is not a trust boundary), a prompt-injected agent can point this at any single-owner tree on the host and have the operator scope a token to repos the sandbox has no relationship to. The multi-owner refusal doesn't help — it only needs to pick a single-owner path.

Quoting is fine throughout; I checked every expansion. This is about scope, not injection.

3. The guest value is persisted as though it were host provenance (acq:1415).

gs_ws=$(acq_backend_workspace_for "$gs_sandbox")
[ -z "$gs_ws" ] || acq_workspace_record_write "${ACQ_RESOLVED_BACKEND}" "$gs_sandbox" "$gs_ws" || true

After one fallback, every later run takes the "recorded" branch and reports using recorded workspace for '<x>': <path> — wording that implies provision-time host provenance. There is no way to tell a host-derived record (sbx.sh:497, msb.sh:3201, both from validated host positionals) from a laundered guest one. This part is covered by neither #503's marker list nor #504.

Fail-closed would be to drop the guest fallback entirely: a sandbox with no record can simply require the explicit PATH argument the error at acq:1417-1422 already asks for. If it's kept, require absolute + -d + preflight_workspace_path, don't persist it, and say in the message that the value came from the guest unverified.

Smaller items: acq_workspace_record_write (common.sh:1330-1333) rewrites the record with awk -F= '$1 != "workspace"' then appends — a newline in the value injects an extra line that the filter then preserves permanently; and when the record does not yet exist the result is a file containing only workspace=…, with no schema=1/backend=. Also sbx exec auto-starts a stopped sandbox, so a pathless github-scope can boot a sandbox as a side effect of what reads like a lookup (inferred from the comment at sbx.sh:620-625; UNVERIFIED — I did not run sbx).

Method: static analysis at head c5c2672 plus reading #504's diff. No PR code executed.

[wz-gsa]

AI-assisted adversarial review (nexus-agents pr_review), advisory — needs a human to confirm before it drives a change. Note on panel coverage: 4 of 5 voters hit a tool-output parsing error this run (infra issue, not a finding), so only scope_steward returned a substantive vote. That vote is high-confidence (0.72) with a named, traced repro, so flagging it rather than discarding — but this is 1/5 coverage, not the usual 5/5.

High: msb records the guest workspace path where a host path is required (acq.backends/msb.sh:3261).

acq_workspace_record_write is documented as storing "the primary host workspace" mounted into a sandbox, and its consumer github_scope_sandbox → detect_workspace_repos "$ws" walks the host filesystem for git remotes. The two backends disagree on what they hand it:

• sbx.sh passes $_primary_ws — a host path (the new dispatch test in this PR asserts exactly that, assert_output --partial "$pic" against a host tmp dir).
• msb.sh:3261 passes $ACQ_MSB_GUEST_WORKSPACE — a guest-side path. This is confirmed by the sibling acq_backend_workspace_for in the same file, which filters that same value against /home/agent — i.e. it's already known to be a guest path elsewhere in this diff.

On msb — the default backend — acq github-scope SANDBOX (no path arg) will echo a guest path that doesn't exist on the host, detect_workspace_repos finds no git remotes there, and the command silently falls through to "no GitHub repos detected," skipping scoping entirely. That's the exact wrong/no-scope failure mode this PR sets out to fix — just moved from "reads $PWD" to "reads an unusable recorded path," and only on sbx does the new test actually exercise the fixed behavior (the new 30-dispatch-routing.bats case doesn't have an msb equivalent).

Suggested fix: either map guest→host before recording on msb (acq already knows the host mount path at provision time — that's what should be persisted), or have msb's provisioning path capture the host-side value the same way sbx does, rather than the guest env var.

Secondary, non-blocking (raised as conditions if the above is fixed, not separate blockers):

• The sbx acq_backend_workspace_for fallback execs into the guest, reads $ACQ_WORKSPACE there, and persists that guest-reported value into host provenance — a guest-influenceable value shaping which host repos a PAT gets scoped to. Mitigated by the human-in-the-loop step (path is echoed, user picks repos in the GitHub UI), so medium-at-worst, but worth a comment noting the trust direction, and ideally a check that the resolved path is one acq itself mounted.
• Owner-casing: mixed-case duplicates of the same real owner would inflate owner_count and cause a false multi-owner refusal. Fails closed, so not a security issue, just a usability rough edge — worth a tr to a canonical case before the dedup check.
• No command-injection exposure found in the reviewed portion — paths/names only reach quoted arguments or printf, no eval in this diff's own code.
• The 90-sbx-startup-kit.bats git-identity-isolation change in this PR is unrelated to this PR's stated subject (github-scope) and isn't explained in the PR description — probably an incidental fix bundled in, but worth a one-line callout so a future bisect doesn't have to guess why it's here.

No command-injection, no scope-creep, no over-engineering findings — the added helpers reuse the existing provenance-file/awk/mv machinery rather than introducing a new store. The core defect is specifically the msb/sbx path-domain mismatch above.

[mogul]

Addressed the review feedback and pushed commit 48a7aa0.

Changes since the review:

• Removed the pathless github-scope guest fallback entirely; it now uses only host-side provenance and otherwise fails closed with the explicit WORKSPACE_PATH instruction.
• Removed the now-unused backend acq_backend_workspace_for hooks so future paths do not accidentally reintroduce guest-derived host provenance.
• Fixed MSB provisioning to record host-side workspace provenance, including the MSYS host path form, while leaving the guest /var/lib/acq/workspace marker for attach/run only.
• Added workspace_source=host and made acq_workspace_record_read ignore unmarked legacy workspace records, so previously guest-derived records are not trusted.
• Hardened workspace record writes against newline/CR injection and non-directory paths.
• Added regressions for MSB host path recording, MSB --clone recording the original checkout rather than scratch, MSYS host-form provenance, legacy unmarked record rejection, and mixed-case owner handling.

Verification:

• git diff --check
• printf '%s\n' acq acq.backends/*.sh scripts/test-acq-lib.sh scripts/test-acq-bats test/bats/*.bats test/bats/helper.bash | xargs -r -n8 shellcheck --severity=warning
• npm run lint:md
• ./test/vendor/bats-core/bin/bats test/bats/116-clone-option.bats
• ./test/vendor/bats-core/bin/bats test/bats/117-msys-path-forms.bats
• ./test/vendor/bats-core/bin/bats test/bats/125-github-scope.bats
• ./test/vendor/bats-core/bin/bats test/bats/30-dispatch-routing.bats
• ./scripts/test-acq-bats passed: 522/522.

Please re-review when you have a chance.

[basiliskus]

Reviewed at 48a7aa0.

I reviewed the current head with the change-review procedure, focusing on the credential-scoping path and the prior advisory findings. The risky guest-to-host fallback appears removed from the github-scope path: pathless acq github-scope now reads only host-marked provenance and fails closed when no trusted record exists.

1. Minor: PR description is stale (PR body, no file line)

   The body still says this PR adds fallback workspace discovery where SBX reads ACQ_WORKSPACE from the guest and MSB reads /var/lib/acq/workspace. Current code no longer does that: acq github-scope reads acq_workspace_record_read and errors if no host-marked record exists, and acq_backend_workspace_for is gone. Please update the PR body before merge so the review/merge record does not describe the unsafe fallback that was removed.

Below the bar: the test/bats/90-sbx-startup-kit.bats git-identity isolation change is not directly part of github-scope, but it is small and test-isolation-only, so I would not block on it.

Validation note: I confirmed the working tree is at 48a7aa02064eb3a8c9ce3c35aacec1b111489aef, reviewed the diff from 907f4043d14d65e1b70fb62eef6b1d4d8dac4606, checked the prior advisory comments, ran git diff --check and ShellCheck locally with no output, and inspected gh pr checks 476, which shows all checks passing at this head. I did not locally run ./scripts/test-acq-bats or npm run lint:md in this read-only review session because they can create temp/cache artifacts; I relied on the passing GitHub checks for those.

Verdict: approve/comment at 48a7aa02064eb3a8c9ce3c35aacec1b111489aef.

Assisted-by: OpenCode [usai/gpt_5_5_default_v2] change-review