Skip to content

feat(isolation): add cloud.gov acq kit - #408

Open
mogul wants to merge 6 commits into
mainfrom
feat/cloud-gov-acq-kit
Open

mogul wants to merge 6 commits into
mainfrom
feat/cloud-gov-acq-kit

Conversation

@mogul

@mogul mogul commented Sep 8, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Adds a new cloud.gov acq mixin kit for working with Cloud Foundry on cloud.gov.

The kit:

  • installs Cloud Foundry CLI v8.19.0 from pinned upstream release tarballs with committed SHA-256 checks;
  • allow-lists the cloud.gov apex and all cloud.gov subdomains, plus Cloud Foundry docs and pinned CF CLI release hosts;
  • configures CF CLI auth using an acq-injected placeholder from a host-side cf oauth-token, without storing the real token in the kit or workspace;
  • documents troubleshooting, verification, and the design decision.

Context

This supports cloud.gov application work from isolated acq sandboxes while keeping Cloud Foundry authorization in acq's secret mechanism.

msb support is now unblocked by merged quickstart PR GSA-TTS/agentic-coding-quickstart#451, which translates per-kit **.cloud.gov into msb's suffix rule form. I verified the cloud-gov kit spec against quickstart origin/main: **.cloud.gov:443 emits allow@*.cloud.gov for msb.

Verification

  • PATH="$PWD/.venv/bin:$PATH" make validate - passed
  • PATH="$PWD/.venv/bin:$PATH" make validate-kits - passed
  • PATH="$PWD/.venv/bin:$PATH" integrations/isolation/acq-kits/cloud-gov/scripts/verify - passed offline checks: 4 passed, 0 failed; live sandbox check skipped because RUN_ACQ=1 was not set
  • python3 scripts/scan_unsafe_shell.py integrations/isolation/acq-kits/cloud-gov - passed, 0 errors / 0 warnings
  • bash -n on all new shell scripts - passed
  • npx --prefix .github/linters markdownlint-cli2 "**/*.md" "#node_modules" "#.github/linters/node_modules" "#templates" "#CHANGELOG.md" - passed
  • Direct msb adapter check using quickstart origin/main emitted allow@*.cloud.gov and not allow@**.cloud.gov
  • Adversarial sub-agent review run three times; blocking findings addressed or scoped, final focused confirmation found no remaining blocking issue for the auth-header path

Security Impact

  • Adds egress to cloud.gov, **.cloud.gov, docs.cloudfoundry.org, cli.cloudfoundry.org, github.com, and objects.githubusercontent.com for this kit.
  • **.cloud.gov is intentionally broad because the requested working surface is all cloud.gov subdomains; custom domains outside cloud.gov remain project-specific.
  • CF CLI install is pinned by version and SHA-256 for Linux x86_64 and arm64 tarballs.
  • The real Cloud Foundry token is not committed, logged, or written to the workspace. Runtime config writes only the backend placeholder and rejects JWT-looking values.

Rollback

Revert this PR to remove the cloud-gov kit, registry entry, and Markdown Lint toolchain update.

AI Assistance

This PR was generated with assistance from OpenCode, an AI coding agent. Human review is required before merge.

@mogul
mogul requested a review from a team as a code owner September 8, 2026 22:20
@mogul
mogul marked this pull request as draft September 9, 2026 05:28
@mogul

mogul commented Sep 9, 2026

Copy link
Copy Markdown
Contributor Author

Converted to draft while I ensure this works for msb as well.

@mogul
mogul marked this pull request as ready for review September 10, 2026 16:14
@mogul
mogul requested a review from wz-gsa September 10, 2026 16:16
@mogul
mogul force-pushed the feat/cloud-gov-acq-kit branch 3 times, most recently from f7938b9 to 89a09d3 Compare September 11, 2026 19:16
@wz-gsa

wz-gsa commented Sep 22, 2026

Copy link
Copy Markdown
Contributor

AI-assisted review (OpenCode), advisory — needs a human to confirm before it drives a change. Reviewed at head 89a09d3.

The CF CLI install is done well: exact 8.19.0 pin, both arch sha256 committed, mismatch → exit 1 (fail-closed, and the stricter precedent among the kits I looked at). I re-verified both digests against the cloudfoundry/cli release API. Two items.

1. **.cloud.gov:443 is broader than the kit needs, and the kit itself shows by how much (integrations/isolation/acq-kits/cloud-gov/spec.yaml:34).

That one entry covers every cloud.gov subdomain — control plane, UAA/login, logs, and every deployed tenant application route on a shared *.app.cloud.gov name, i.e. other teams' running apps, not just this operator's. The PR body flags the wildcard as intentional, and it is not TLD-level, so the repo's own check_balanced_allowlist_additions.py won't error on it.

But the concrete surface is enumerable from the kit's own files:

  • scripts/verify:111-113 — api.fr.cloud.gov, cloud.gov, docs.cloud.gov
  • files/home/cloud-gov-configure-cf-auth.sh:34-35 — login.fr.cloud.gov, uaa.fr.cloud.gov

That's ~5-6 exact hosts plus a logs endpoint, versus one wildcard. In a file whose entire job is the egress boundary for untrusted agent code, enumerating them is what least privilege means here. If the wildcard is genuinely required for app routes the operator deploys, it would help to say which and why in the spec comment, rather than leaving the breadth asserted.

2. The token guard is shape-based, so the fail-closed claim is narrower than the PR states (files/home/cloud-gov-configure-cf-auth.sh:23-27).

The guard refuses *eyJ*.*.* — a base64url JWT header, which is the common shape — then writes the value into ~/.cf/config.json as AccessToken with chmod 0600. The PR's Security Impact says it "rejects JWT-looking values," which is accurate. But the conclusion — that the real token never lands in the workspace — depends on a real token never arriving in another form. A bearer <jwt> prefix, or any opaque non-eyJ token, passes the guard and is written unredacted.

Assert the positive instead: validate the value is the expected acq placeholder shape and refuse everything else. That inverts it from blocklisting one token shape to allowlisting the one you expect.

Nit, unrelated to the kit: the markdownlint-cli2 0.22.1→0.23.2 bump in .github/linters/package.json:9 also removes 75 "license" fields from package-lock.json with no replacement — the lockfile was regenerated by a different npm major than produced it. This repo ships a dependency-analysis skill that would want that metadata. Either regenerate with the pinned npm, or split the toolchain bump out.

Process question: @mogul's only comment here (2026-09-09) says "Converted to draft while I ensure this works for msb as well," but the PR currently reports as not-draft with no follow-up. Is the msb verification done, or did it come out of draft incidentally?

Verified clean, recording affirmatively: no listening port, no server process, no serviceGateways, no host path mounted, no 0.0.0.0/0, no metadata endpoint; root scoped to the install phase, startup as uid 1000; the agentContext is protective ("Do not ask the user to paste the token into chat or write it to the workspace").

UNVERIFIED: the actual shape of acq's injected CF_OAUTH_TOKEN placeholder — whether the JWT guard suffices depends on it, and it's defined in the quickstart repo, not this diff.

Method: static analysis at head 89a09d3; CF CLI digests checked against the vendor's release API. No kit executed. bin/injection-scan.sh flagged two hits, both inspected and benign — npm lockfile integrity hashes, and prose describing the protective design.

@wz-gsa

wz-gsa commented Sep 23, 2026

Copy link
Copy Markdown
Contributor

AI-assisted adversarial review (nexus-agents pr_review, 5-role panel), advisory — needs a human to confirm before it drives a change. 5-0 request_changes, confidence 0.78–0.82, one finding rated critical.

Critical: the auth mechanism has no working input (all 5 voters, independently derived)

cloud-gov-configure-cf-auth.sh:24's guard is a POSIX case pattern *eyJ*.*.* — in case, . is literal, * is glob — so it matches exactly the real cf oauth-token output the README instructs users to store (bearer eyJ....eyJ....sig), and rejects it with exit 1 inside a startup-phase command, aborting sandbox bring-up.

The complementary path is also broken: if a non-JWT placeholder does reach AccessToken, stock CF CLI v8 doesn't treat that field as an opaque proxy-rewritable string — it base64-decodes it as a JWT locally to render Getting orgs as <user>... and to evaluate token expiry. A placeholder that isn't JWT-shaped fails client-side before any request reaches the proxy that's supposed to swap it.

So: real token → startup fails closed. Placeholder → CLI can't parse it. No value threads both. This is the kit's entire value proposition, and nothing in the default scripts/verify path would catch it — offline verify only checks bash -n on the install script, never cloud-gov-configure-cf-auth.sh (the file with both the guard and the auth logic), and the live check that would exercise cf orgs is gated behind RUN_ACQ=1 (needs a real cloud.gov token + sandbox host), so it's never run in CI.

High: **.cloud.gov:443 allow-lists every tenant's self-service app route (all 5 voters)

cloud.gov is multi-tenant — *.app.cloud.gov routes are self-service, so this wildcard grants the sandbox egress to endpoints any cloud.gov account holder (including an attacker) can stand up. None of the kit's documented functions (cf CLI against api/login/uaa, log-stream, dashboard, docs) need tenant app routes. For a prompt-injected agent this is a working exfiltration channel the kit's own allow-list authorizes. The kit's own TROUBLESHOOTING.md already tells users to add specific hosts per-project for non-cloud.gov domains — the same discipline should apply here: enumerate api./login./uaa./deploy./logs./dashboard.fr.cloud.gov + cloud.gov/docs.cloud.gov by name (~5 lines) and leave app routes opt-in.

High: msb support declared but conceded-unmet (scope_steward, architect)

kits.yaml declares backends: [sbx, msb], but the kit's own ADR states msb requires acq to translate **. wildcards to suffix-rule form, which isn't implemented — so on msb (acq's default backend), every cloud.gov host in this kit is unreachable, not just degraded. The parity field exists to record verified coverage; this should be [sbx] until the translation lands and is live-checked, or cite the acq change that makes it true.

Medium (multiple voters, not individually blocking but compounding)

  • json_escape() only escapes \ and " — a raw newline or control char in the token value (plausible if cf oauth-token's trailing newline isn't stripped upstream) produces invalid JSON; cf then fails with an opaque parse error with no TROUBLESHOOTING entry.
  • chmod 0600 runs after cat > creates ~/.cf/config.json — a brief umask-width window; ~/.cf itself is never chmod 0700.
  • scripts/verify's mktemp -d "$KIT_DIR/verify-workspace.XXXXXX" writes scratch state inside the versioned kit directory with no .gitignore entry — routine verification dirties the repo.
  • Release-asset host risk: allow-list has objects.githubusercontent.com but GitHub release downloads may now redirect through release-assets.githubusercontent.com — worth confirming live before merge.
  • CF CLI v8.19.0 + both SHA-256 pins are unverifiable from the diff (catfish flagged v8 releases they're aware of cluster in the 8.7–8.9 range) — please confirm the version/hash pair against the actual upstream release before merge.
  • spec.yaml has no trailing newline (will trip the repo's EOF/whitespace pre-commit hooks).

No scope-creep findings — scope_steward confirmed this is legitimate build-vs-buy (installing upstream cf rather than reimplementing a CF client) and squarely on-mission for acq-kits/. The concept and structure are sound; the specific implementation of the auth handshake needs to actually work before merge, and the egress scope needs narrowing regardless.

@mogul

mogul commented Sep 24, 2026

Copy link
Copy Markdown
Contributor Author

Addressed the review feedback and pushed commit 2e3062a for re-review.\n\nChanges made:\n- Switched the cloud.gov wildcard from **.cloud.gov to *.cloud.gov so current msb accepts the rule directly instead of receiving invalid allow@**.cloud.gov.\n- Kept the broad cloud.gov wildcard, but documented the tradeoff: Cloud Foundry app routes are dynamically assigned, so a reusable static kit cannot know which *.app.cloud.gov routes belong to the current operator. Custom domains remain project-specific.\n- Hardened cloud-gov-configure-cf-auth.sh to validate CF_OAUTH_TOKEN before invoking external commands, clear it for the cf api targeting call, accept only recognized backend placeholder shapes, and reject raw token/JWT-like material.\n- Stopped writing the placeholder into ~/.cf/config.json; docs now describe direct CF API calls as the authenticated placeholder-backed path until an authenticated stock cf path is live-tested.\n- Expanded offline verify coverage for the auth script and added stale-token guidance for 401/403 auth probe failures.\n- Moved verify scratch dirs out of the repo and removed the unrelated markdownlint dependency bump from this PR.\n\nVerification:\n- PATH="/Users/bretamogilefsky/Documents/Code/gsa/agentic-coding/.paseo-worktrees/1x1krnuy/wrathful-mouse/.venv/bin:/home/agent/.npm-global/bin:/.msb/scripts:/home/agent/.local/bin:/usr/local/share/npm-global/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin" make validate passed (existing advisory warnings only).\n- PATH="/Users/bretamogilefsky/Documents/Code/gsa/agentic-coding/.paseo-worktrees/1x1krnuy/wrathful-mouse/.venv/bin:/home/agent/.npm-global/bin:/.msb/scripts:/home/agent/.local/bin:/usr/local/share/npm-global/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin" make validate-kits passed.\n- PATH="/Users/bretamogilefsky/Documents/Code/gsa/agentic-coding/.paseo-worktrees/1x1krnuy/wrathful-mouse/.venv/bin:/home/agent/.npm-global/bin:/.msb/scripts:/home/agent/.local/bin:/usr/local/share/npm-global/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin" integrations/isolation/acq-kits/cloud-gov/scripts/verify passed: 8 passed, 0 failed; live sandbox check skipped locally because RUN_ACQ=1 was not set.\n- python3 scripts/scan_unsafe_shell.py integrations/isolation/acq-kits/cloud-gov passed: 0 errors / 0 warnings.\n- bash -n on the cloud-gov shell scripts passed.\n- npx --prefix .github/linters markdownlint-cli2 "**/*.md" "#node_modules" "#.github/linters/node_modules" "#templates" "#CHANGELOG.md" passed with 0 errors.\n- git diff --check passed.\n- Live reporter verification after refreshing the short-lived CF token passed: RUN_ACQ=1 scripts/verify reported 17 passed, 0 failed, including authenticated /v3/organizations -> 200.\n\nPlease re-review when you have a chance.

@wz-gsa

wz-gsa commented Sep 30, 2026

Copy link
Copy Markdown
Contributor

AI-assisted re-review (OpenCode), advisory — needs a human to confirm before it drives a change. Reviewed fix commit 2e3062a, live-tested against real msb 0.7.4 and sbx binaries rather than reading the diff alone — the previous review's critical finding turned out to still hold under a different guise, so I wanted first-hand reproduction before repeating a verdict.

1. The critical defect (no value threads both the guard and real auth) is still open — the fix moved to a different placeholder shape, not a working one

cloud-gov-configure-cf-auth.sh's guard now accepts acq_placeholder_?* or sbx-cs-?*, plus a [A-Za-z0-9_-] charset check, and exit 1s on anything else inside a startup-phase command.

I live-tested msb's actual placeholder shape rather than assuming it: created a real sandbox with msb create --secret TESTVAR@example.com alpine, then forced real shell expansion inside the guest:

$ msb exec pr408check2 -- sh -c 'echo "TESTVAR is: [$TESTVAR]"'
TESTVAR is: [$MSB_TESTVAR]

msb's real placeholder is the literal, unexpanded string $MSB_<ENV> (confirmed against docs/BACKEND_GUIDE.md:729 in quickstart, which documents this explicitly: "msb puts a placeholder ($MSB_<env>) in the guest"). That string contains $, which:

  • doesn't match acq_placeholder_?* or sbx-cs-?* (I checked both patterns directly against it), and
  • fails the charset guard too ($ isn't in [A-Za-z0-9_-]).

So on msb — the kit's own declared backend — the script exits 1 during startup and sandbox bring-up aborts. This is the same "auth mechanism has no working input" finding from the original panel review; the specific never-matching pattern changed, but the underlying gap (the guard was never checked against what the target backend actually produces) did not.

2. The wildcard narrowing (**.cloud.gov → *.cloud.gov) made no practical difference on msb

I read _acq_msb_balanced_target in quickstart's acq.backends/msb.sh — it only special-cases a literal **. prefix; any other wildcard form, including *.cloud.gov, passes through unchanged into msb create --net-rule allow@*.cloud.gov:tcp:443. msb's own CLI docs (msb create --help) define *.example.com as a domain-suffix rule with no depth limit — the same semantics as **.. I confirmed live: a real msb sandbox with exactly this rule reached api.fr.cloud.gov, login.fr.cloud.gov, and arbitrary other cloud.gov subdomains — unbounded-depth coverage, identical blast radius to before. spec.yaml's own comment still self-describes this as "a deliberate broad egress exception." The original HIGH finding (multi-tenant exfiltration surface — any cloud.gov account holder's self-service app route is reachable) is unchanged on the default backend.

3. New regression: the same edit breaks the kit's own required hosts on sbx

sbx's wildcard semantics are genuinely single-label for *.host (confirmed live via sbx policy check network): *.cloud.gov allows www.cloud.gov but denies api.fr.cloud.gov, login.fr.cloud.gov, and uaa.fr.cloud.gov — the exact control-plane hosts the kit's own auth flow requires (spec.yaml's documented cf oauth-token | acq secret set -g cloud-gov --host api.fr.cloud.gov step). The README's Backend Parity section states "sbx accepts [*.cloud.gov] as a cloud.gov subdomain wildcard" including "API, login/UAA" — I tested this directly and it's false. This makes the kit non-functional for its stated purpose on sbx, which is not a defect that existed before this fix (the prior **.cloud.gov form, per sbx's own multi-label semantics, would have covered these 2-label hosts correctly).

Net assessment

The single-character wildcard edit substituted one wrong tradeoff for another rather than resolving the underlying tension: msb's suffix-matching can't be narrowed by wildcard syntax alone (it needs explicit per-host enumeration to actually shrink the exposure), while sbx's **./*. distinction is real and needs the multi-label form to reach the kit's own required hosts. One shared wildcard string can't correctly serve both backends' different semantics — this likely needs a backend-conditional egress list, or explicit per-host enumeration (the original review's suggested ~5-6 lines: api./login./uaa./logs./dashboard.fr.cloud.gov + cloud.gov/docs.cloud.gov) rather than a wildcard on either backend.

Recommend: still request_changes. Happy to help draft the enumerated-host version if useful — happy to also independently verify a next pass the same way (live sandbox creation on both backends) rather than by diff inspection alone, since that's specifically what caught both the original and this regression.

@mogul

mogul commented Oct 1, 2026 •

Copy link
Copy Markdown
Contributor Author

Re-review requested for the cloud.gov acq kit updates.

Summary:

  • Tightened auth startup handling to accept backend placeholders, reject raw bearer/JWT-like token material, and avoid writing token material into CF CLI config.
  • Replaced wildcard cloud.gov egress with exact allow-listed hosts for cloud.gov control-plane/docs and pinned CF CLI downloads.
  • Updated README, troubleshooting, and decision docs for backend parity and stock CF/direct API limitations.
  • Added offline verification for synthetic placeholder handling and exact egress allow-list assumptions.

Verification:

  • integrations/isolation/acq-kits/cloud-gov/scripts/verify — PASS, 23 passed, 0 failed; live sandbox check skipped because RUN_ACQ=1 was not set.
  • shellcheck on cloud.gov scripts — PASS.
  • uv run make validate-kits — PASS.
  • uv run make validate — PASS.
  • uv run make scan-shell — PASS.
  • uv run --extra dev make test — PASS, 401 passed.

Blocked locally:

  • Live acq/cloud.gov backend verification requires live credentials and RUN_ACQ=1.
  • Markdown lint was not available locally without adding unrequested Node tooling.

AI assistance: OpenCode Agent assisted with this update; human review required before merge.

mogul and others added 4 commits October 1, 2026 07:13
Add an sbx cloud.gov acq mixin kit that installs the pinned Cloud Foundry CLI, allow-lists cloud.gov and Cloud Foundry docs egress, and configures CF auth through acq secret placeholders.

Tracks deferred msb wildcard parity in #407.

Co-authored-by: OpenCode Agent <agent@gsa.gov>
Update the cloud.gov kit registry and docs after verifying that the quickstart msb wildcard fix translates per-kit **.cloud.gov to allow@*.cloud.gov. This depends on GSA-TTS/agentic-coding-quickstart#451 for msb runtime support.

Co-authored-by: OpenCode Agent <agent@gsa.gov>
Tighten CF token placeholder handling, use an msb-compatible cloud.gov wildcard, and update verification/docs to reflect the supported auth path.

Co-authored-by: OpenCode Agent <agent@gsa.gov>
Accept synthetic msb-style backend placeholders while rejecting raw bearer/JWT-like token material before writing CF CLI auth state. Replace wildcard cloud.gov egress with exact control-plane and documentation hosts, and extend offline verification for placeholder and allow-list assumptions.

Co-authored-by: OpenCode Agent <bret.mogilefsky@gsa.gov>
@mogul
mogul force-pushed the feat/cloud-gov-acq-kit branch from c2b09db to d16c1a8 Compare October 1, 2026 07:16
@wz-gsa

wz-gsa commented Oct 6, 2026

Copy link
Copy Markdown
Contributor

AI-assisted re-review (OpenCode), advisory — needs a human to confirm before it drives a change. Reviewed head 48b71a0, with a live msb sandbox rather than diff inspection, because this is the third round on the same finding and I did not want to repeat a verdict from a six-day-old measurement.

Two of three prior findings are resolved. The critical one is still open, and I can now show it end-to-end on current software.


Resolved — the egress findings (my items 2 and 3 from the 2026-09-30 round)

The wildcard is gone, replaced with exact hosts:

# Exact cloud.gov control-plane and documentation hosts. Avoid wildcard
# semantics so sbx and msb translate the same least-privilege egress shape.
- api.fr.cloud.gov:443
- uaa.fr.cloud.gov:443
- login.fr.cloud.gov:443
- doppler.fr.cloud.gov:443
- log-cache.fr.cloud.gov:443
- cloud.gov:443
- www.cloud.gov:443
- docs.cloud.gov:443
…13 hosts, zero wildcards

This resolves both prior findings at once rather than trading one for the other, which is what the single-character **. → *. edit did:

  • msb blast radius. No wildcard means no suffix match, so the unbounded-depth *.cloud.gov coverage I reproduced live is gone. I checked the translation path: _acq_msb_balanced_target's documented behaviour for a non-wildcard token is "the host unchanged (exact domain)", and kit_spec_net_allow parses host[:port] correctly, so all 13 pass through 1:1.
  • sbx missing control-plane hosts. api., uaa. and login.fr.cloud.gov — the three the *.cloud.gov form denied under sbx's single-label semantics, and which the kit's own auth flow requires — are now explicit.

The README's Backend Parity section also no longer carries the claim I measured as false ("sbx accepts it as a cloud.gov subdomain wildcard … including API, login/UAA"). The replacement text is accurate: "Both sbx and msb receive the same exact host list."

One limit I want to be straight about: I could not re-verify the sbx side this round. sbx policy check network is wedged on my host with error: docker login service unavailable: a prior ambiguous refresh attempt for this credential is still cooling down. Exact hosts have no wildcard semantics to get wrong, so I expect them to be fine — but that is reasoning, not measurement, and live sbx testing is exactly what caught the regression last round. Treat the sbx half as UNVERIFIED by me.


Still open (critical) — the auth guard rejects the placeholder msb actually injects

The guard now accepts two more shapes:

acq_placeholder_?*|sbx-cs-?*|MSB_PLACEHOLDER?*|\<MSB_PLACEHOLDER*\>)

msb does not emit either of them. Measured on msb 0.7.7 (my prior round was 0.7.4, so I re-measured rather than citing it):

$ msb exec pr408check3 -- sh -c 'echo "CF_OAUTH_TOKEN is: [$CF_OAUTH_TOKEN]"'
CF_OAUTH_TOKEN is: [$MSB_CF_OAUTH_TOKEN]

Unchanged from 0.7.4, and still what quickstart documents — docs/BACKEND_GUIDE.md:729: "msb puts a placeholder ($MSB_<env>) in the guest".

Then I ran this PR's own case statement, inside the guest, against that real value:

$ msb exec pr408check3 -- sh -c 'case "$CF_OAUTH_TOKEN" in
    acq_placeholder_?*|sbx-cs-?*|MSB_PLACEHOLDER?*|\<MSB_PLACEHOLDER*\>) echo "ACCEPT";;
    *.*.*) echo "REFUSE token-like";;
    *) echo "REFUSE unrecognized -> exit 1 -> startup aborts";;
  esac'
REFUSE unrecognized -> exit 1 -> startup aborts

$MSB_CF_OAUTH_TOKEN falls to the *) arm, the script exit 1s in a startup-phase command, and sandbox bring-up aborts on msb — the kit's own declared backend.

For completeness, the full pattern set against the real value and the shapes that do match:

$MSB_CF_OAUTH_TOKEN    -> REFUSE (unrecognized)    <- what msb emits
MSB_PLACEHOLDER_CF     -> ACCEPT                   <- what the fix expects
<MSB_PLACEHOLDER>      -> ACCEPT
acq_placeholder_x      -> ACCEPT
sbx-cs-abc             -> ACCEPT
eyJhbG.payload.sig     -> REFUSE (token-like)      <- correct, and worth keeping

The pattern across three rounds

Each round has fixed the specific string and left the method unchanged — the guard is written against an assumed placeholder shape and not checked against what the backend produces. That is now three different never-matching patterns (*eyJ*.*.*, then acq_placeholder_/sbx-cs-, now MSB_PLACEHOLDER).

Two ways out, and I'd suggest the second:

  1. Match the real shape. Add \$MSB_?* to the accept arm. Note the $ must survive the preceding *[![:print:]]*|*[[:space:]]* guard — it does, $ is printable and not whitespace, so that arm is fine as written.
  2. Stop hardcoding backend placeholder shapes in the kit. Three rounds suggests this is the wrong layer to know it. The kit's actual requirement is "this value is a backend placeholder, not raw token material" — the negative half (reject bearer*, reject *.*.* JWT shapes) is backend-independent and already correct. Inverting to "refuse known-token-material, otherwise proceed" would be robust to every backend's placeholder format, including ones that do not exist yet. The positive allow-list is what keeps breaking.

Either way the fix wants a test that asserts the guard against a measured placeholder rather than a literal in the test file, or round four is likely.


Verification method, and one trap worth knowing

Live msb sandbox (msb 0.7.7, alpine, dummy non-secret value — not real credentials), --secret CF_OAUTH_TOKEN@api.fr.cloud.gov, then forced shell expansion in-guest.

A trap I hit: my first msb create failed with error: invalid config: secret CF_OAUTH_TOKEN: host environment variable CF_OAUTH_TOKEN is not set — but left a stopped sandbox behind with no secret bound. Measuring against that sandbox returns an empty value, which would have led me to the opposite conclusion. I removed and recreated before measuring. Worth knowing if anyone reproduces this: check msb list shows the sandbox running and that the create actually bound the secret.

Sandbox removed after the measurement.


Net

The egress work is done and done correctly — the enumerated-host approach is what both prior rounds were asking for, and the README is accurate now.

The auth guard is still non-functional on msb, and it is the kit's central mechanism. I'd recommend request_changes on that single item, with the caveat that my sbx-side egress check is unverified this round for host reasons unrelated to the PR.

Happy to verify a next pass the same way — live sandbox, in-guest, both backends once sbx policy check is usable again.

@basiliskus basiliskus left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Verdict: request changes — 48b71a03582c43f3d0884b6b0764bf9810983a13

The prior auth-placeholder finding remains open. Current quickstart documents and implements the msb guest placeholder as literal $MSB_<env>, so this kit receives $MSB_CF_OAUTH_TOKEN. The startup guard at integrations/isolation/acq-kits/cloud-gov/files/home/cloud-gov-configure-cf-auth.sh:27 accepts MSB_PLACEHOLDER... forms instead, and the real msb shape falls through to the refusal at line 35. I reproduced that exact value against this script: it printed "not a recognized backend placeholder" and exited 1. Because this is a startup command and the kit declares msb support, a correctly provisioned msb sandbox cannot start with the documented secret binding.

Please accept the actual $MSB_... placeholder shape and cover $MSB_CF_OAUTH_TOKEN in the offline guard test, or move placeholder recognition to an authoritative backend-aware interface.

Minor: integrations/isolation/acq-kits/README.md:23 says the exact-host kit allows app-route domains, but the kit deliberately excludes app routes. Please make the catalog description match the implemented boundary.

Assisted-by: OpenCode [usai/gpt-5.6-sol-latest-guardrails-defaultv2] skill:change-review

Accept the documented msb guest-secret placeholder in the cloud.gov auth guard and cover it in offline verification. Align kit documentation with the implemented domain boundary.

Co-authored-by: OpenCode Agent <user@gsa.gov>
@mogul
mogul requested a review from basiliskus October 14, 2026 03:20
@wz-gsa

wz-gsa commented Oct 14, 2026

Copy link
Copy Markdown
Contributor

AI-assisted re-review (OpenCode), advisory — needs a human to confirm before it drives a change. Reviewed head e2a3091b0, measured live in an msb guest rather than by reading the diff, because this is the fourth round on the same finding and I did not want to repeat a verdict from inference.

The blocker is fixed. I can show it on the real value this time.


Resolved — the guard now accepts what msb actually injects

msb 0.7.8 (my prior round was 0.7.7, so I re-measured rather than citing it), live sandbox, dummy non-secret value bound via --secret CF_OAUTH_TOKEN@api.fr.cloud.gov:

$ msb exec pr408r4 -- sh -c 'echo "CF_OAUTH_TOKEN is: [$CF_OAUTH_TOKEN]"'
CF_OAUTH_TOKEN is: [$MSB_CF_OAUTH_TOKEN]

Then I ran this PR's actual auth script, unmodified, inside that guest against that real value:

$ msb exec pr408r4 -- sh -c '... base64 -d > /tmp/auth.sh; HOME=/tmp/h sh /tmp/auth.sh; echo "EXIT=$?"'
cloud-gov: validated CF_OAUTH_TOKEN as a backend placeholder
EXIT=0

Startup no longer aborts on msb. msb_prefix='$'MSB_ plus "$msb_prefix"?* is the right shape, and quoting the variable in the case arm is what makes it a literal $MSB_ prefix rather than an expansion — that detail is easy to get wrong and it is correct here.

The negative half still holds, measured in the same guest:

bearer eyJ.a.b           REFUSE
eyJhbG.payload.sig       REFUSE
MSB_PLACEHOLDER_TOKEN    REFUSE
random-junk              REFUSE

Removing the two tests that asserted MSB_PLACEHOLDER / <MSB_PLACEHOLDER> was correct — those shapes never existed, so they were asserting the bug.

Offline verifier, in a network-off container at this head:

23 passed, 0 failed    (exit 0)

validate-kits.py needs jsonschema+pyyaml; with those installed it passes. Without them the script reports SKIP, not PASS — correct degraded-path behavior, worth keeping.


Nit (non-blocking): the allow-list arm admits shell metacharacters

"$msb_prefix"?* accepts anything after the prefix, including:

$MSB_x;id        ACCEPT
$MSB_$(id)       ACCEPT
$MSB_`id`        ACCEPT

I chased this to a conclusion rather than flagging it on appearance, and it is not exploitable as written. The value is only ever compared in case, never eval'd, never interpolated into a command, and never written to ~/.cf/config.json — the one downstream consumer explicitly blanks it:

env CF_OAUTH_TOKEN= cf api https://api.fr.cloud.gov

I grepped the whole kit; no other code path consumes the variable. So this is defense-in-depth, not a defect. If you want it tighter, "$msb_prefix"[A-Za-z0-9_]* would match msb's actual env-name alphabet and cost nothing.

Nit: the new test asserts a shape msb does not emit

MSB_PLACEHOLDER='$'MSB_SECRET_PLACEHOLDER

msb emits $MSB_<ENVNAME>, i.e. $MSB_CF_OAUTH_TOKEN. Both match the $MSB_ prefix, so the test passes for the right reason — but the literal is still a guess about the suffix, in a test named "accepts msb guest secret placeholder". '$'MSB_CF_OAUTH_TOKEN would make the fixture the thing the kit will actually see.

This is the same method that produced the first three rounds: the guard's positive arm is written against an assumed shape and the test encodes that assumption as a literal. It now matches reality, so it is no longer a blocker — but the structural point from my last round stands, and if a backend ever changes its placeholder format this breaks again silently.


UNVERIFIED

  • The sbx half. sbx policy check network is still wedged on my host with error: docker login service unavailable: a prior ambiguous refresh attempt for this credential is still cooling down — same failure as my last round, unrelated to this PR. Exact hosts have no wildcard semantics to get wrong, so I expect it is fine, but that is reasoning, not measurement.
  • RUN_ACQ=1 live cloud.gov verification. Needs a real cloud.gov token; I did not run it and would not with real credentials in a review sandbox.

Sandbox removed after measurement (msb remove --force pr408r4, confirmed gone).


Net

The finding I raised three times is genuinely closed, verified against the real injected value rather than a literal. The egress work was already correct. All 7 checks pass at this head and the PR is mergeable.

No blocking findings remain from me. The two nits above are optional and neither needs a new round.

@basiliskus basiliskus left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Verdict: approve — e2a3091b081eff1609f803f36d0bcb9b29611870

The prior startup blocker is resolved at integrations/isolation/acq-kits/cloud-gov/files/home/cloud-gov-configure-cf-auth.sh:28: the quoted case pattern matches the documented literal $MSB_<env> prefix. The replacement test at integrations/isolation/acq-kits/cloud-gov/scripts/verify:85 now exercises that real shape, while the bearer and JWT-like negative cases remain. Although the suffix accepts shell metacharacters, I found no evaluation or config-write path: the startup script only validates the value, and the documented/verification consumers pass its expansion inside a quoted HTTP-header argument.

The prior catalog issue is also resolved at integrations/isolation/acq-kits/README.md:23, which no longer advertises app-route access.

Assisted-by: OpenCode [usai/gpt-5.6-sol-latest-guardrails-defaultv2] skill:change-review

@basiliskus
basiliskus self-requested a review October 14, 2026 20:25
@wz-gsa

wz-gsa commented Oct 14, 2026

Copy link
Copy Markdown
Contributor

AI-assisted follow-up (OpenCode), advisory. New Blocker found after my previous comment cleared the auth guard. I had marked the sbx half UNVERIFIED for host reasons; that is now fixed on my side, and running the real path immediately surfaced something diff inspection did not.

acq create with this kit fails on any Zscaler-intercepted host. The CF CLI install can never verify TLS, because it runs a phase before the CA that signs the connection is installed.


Measured

$ acq --backend sbx create --name cgorder408 --kit .../acq-kits/cloud-gov opencode /tmp/cgws
  $ chmod 0644 '/home/agent/zscaler-ca.crt' ... (kit=zscaler-ca-certificate, user=0)
  ✓ chmod 0644 '/home/agent/zscaler-ca.crt' ... (kit=zscaler-ca-certificate, user=0, 8ms)
  $ sh /home/agent/cloud-gov-install-cf-cli.sh (kit=cloud-gov, user=0)
  ✗ sh /home/agent/cloud-gov-install-cf-cli.sh (kit=cloud-gov, user=0, exit 60)
  ── captured output ──
  curl: (60) SSL certificate OpenSSL verify result: unable to get local issuer certificate (20)
  ──────────────────────
  registering 7 startup commands, run on every container start
    ○ sh -c set -eu install -m 0644 /home/agent/zscaler-ca.crt \ ... (kit=zscaler-ca-certificate, user=0)
error: failed to apply kit to sandbox

Read the ordering in that log: the zscaler cert is staged at create time, the cloud-gov install runs and fails, and the zscaler step that actually installs the CA into the trust store is only registered as a startup command — it has not run yet.

Cause

The two kits disagree about phase:

cloud-gov/spec.yaml:57     - phase: install     <- curl to github.com releases
zscaler-ca-certificate/spec.yaml:65  - phase: startup   <- update-ca-certificates

cloud-gov-install-cf-cli.sh:64 does an unconditional TLS fetch:

url="https://github.com/cloudfoundry/cli/releases/download/v${CF_CLI_VERSION}/${asset}"
curl -fsSL "$url" -o "$work/cf-cli.tgz"

On an intercepted network that chain terminates at the corporate root, which is not in the container trust store until the zscaler kit's startup step runs. So curl exits 60 and — because the script is set -eu — the kit application aborts. sbx create fails outright; the sandbox is unusable.

This is the same trap the zscaler kit documents for itself, in its own README §"Why initFiles + startup, not files + install":

copies the cert in a commands.install step, which on the opencode-docker base runs before sandbox setup lays down files/ ... so at install time neither the source cert nor the destination dir exists, and sbx create fails.

That kit solved its own ordering problem and recorded the reasoning in an ADR. cloud-gov then put a TLS fetch in the phase that runs first.

Scope

I checked every kit with an install phase for a direct TLS fetch in its payload:

cloud-gov      files/home/cloud-gov-install-cf-cli.sh:64   curl -fsSL "$url"
git-ssh-sign   (none)
oci-engine     (none — uses apt-get/dnf/apk, not a direct curl)
prime-agent    (none)

cloud-gov is the only one. That is also why this has not been hit before.

Why my offline verification missed it

scripts/verify without RUN_ACQ=1 never creates a sandbox, so all 23 offline checks pass at this head — I reported exactly that last round. The failure only appears on the live path, on an intercepted host. Two conditions, neither of which the default verifier exercises.

Suggested fixes

  1. Move the install to startup, ordered after the CA. Matches what the zscaler kit concluded for itself. Costs an idempotency check, which the script already has (command -v cf → exit 0 at line 23).
  2. Keep install but make it fail soft, like oci-engine does — log and exit 0 so the sandbox still comes up without cf, rather than failing create. Weaker: silently no cf, and the kit's whole purpose is cf.
  3. Declare the dependency if the schema can express "run after kit X's startup."

I'd suggest 1. Option 2 trades a hard failure for a silent capability gap, which is the degraded-path shape worth avoiding.


Unaffected by this

The auth-guard fix I verified in my previous comment still stands — that was measured in an msb guest with no TLS fetch involved. The egress host list is also fine. This is specifically a create-time ordering defect on intercepted networks.

Note on my own environment, for reproducibility

My host also hit a Go 1.27 CA issue that is not related to this PR: golang/go#77865 made crypto/x509 honor SSL_CERT_FILE/SSL_CERT_DIR on darwin in 1.27, so a shell exporting an incomplete bundle breaks every Go tool against intercepted hosts. That was wedging sbx for me across two earlier rounds. It affects the reviewer's host, not the kit — mentioning it only because anyone reproducing this on macOS with Go 1.27+ tooling may hit it first and mistake it for the same bug.

Sandboxes created for this measurement were removed (sbx rm --force).

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