Repository navigation
feat(kits): add goose-server chat-harness acq mixin kit - #415
mark-aronson wants to merge 1 commit into
Conversation
8e5fb24 to
d755007
Compare
Add the first acq chat-harness kit: run the goose ACP server (goose serve, "goosed") inside the sandbox and publish its port so a native goose Desktop app on the host drives it as an external server (goose's documented remote-server pattern). All agent work — the interactive loop, tool execution, MCP extensions, file/shell access, LLM egress — runs under acq's isolation; Desktop is a thin UI. - Neutral hybrid/v1 mixin: create-time install of a pinned, sha256-verified, arch-selected goose CLI; startup supervises `goose serve` on 0.0.0.0:3284, published to a host-loopback port (0.0.0.0 in-guest so msb create-time publish reaches it). - Provider: ships a goose custom provider (custom_usai) matching what a GSA goose Desktop uses; key injected as CUSTOM_USAI_API_KEY (never stored in the kit). The provider's model list is GENERATED from the single USAi catalog by a new emitter (integrations/providers/usai/emitters/goose.mjs) with a byte-exact lockstep test, so it stays in sync with OpenCode/prime-agent instead of drifting as a hand-maintained snapshot. Default model: claude-sonnet-5. - Auth: shared secret-key handshake (GOOSE_SERVER__SECRET_KEY) — goose Desktop 1.50.0's ACP client always sends a secret, so unauthenticated is rejected. - Docs: README, TROUBLESHOOTING, ADRs (split, transport, provider, and an alignment note recording expected migration to devenv process supervision + declarative install + kit-inference once those directions land). - Verified live end-to-end on msb (install, server, config merge, provider stage, published port, 0.0.0.0 bind, secret-key handshake, self-heal) and connected a real goose Desktop with a working USAi model turn. Registers the kit in kits.yaml and the kits README. Co-authored-by: OpenCode Agent <agent@gsa.gov>
d755007 to
60a8486
Compare
|
AI-assisted review (OpenCode), advisory — needs a human to confirm before it drives a change. Reviewed at head The supply-chain hygiene here is better than most: exact version pin, both arch sha256 digests committed, verify-before-extract, no 1. A sha256 mismatch fails open ( if [ "$got" != "$sha" ]; then
warn "goose archive sha256 mismatch for $rarch/$variant:"
warn " expected $sha"
warn " got $got"
warn "Refusing to install a mismatched binary. (non-fatal)"
exit 0
fiEvery failure path in this script exits 0 — thirteen of them — which the header states as the design ("any failure warns to stderr and exits 0 so sandbox provisioning still succeeds"). That is the right call for most of them: no curl, unsupported arch, extraction failure, no sha256 tool. It is the wrong call for a digest mismatch, because that is the one signal that specifically means the artifact is not what was pinned. The consequence is that provisioning cannot distinguish "goose is absent because the download failed" from "goose is absent because the bytes were substituted". Both produce the same warning in the same stream and the same exit 0. The sibling installer in #408 ( 2. Drop the unused The comment calls it "belt-and-suspenders" for a latest-tag lookup, but the installer pins a version and fetches Scope suggestion, not a blocker: Verified clean, recording affirmatively: no wildcard egress, no UNVERIFIED: that the host side of Method: static analysis at head |
|
AI-assisted review (OpenCode), advisory — needs a human to confirm before it drives a change. Reviewed at head Correction on my own process, up front: an earlier pass at this PR used a diff I had manually (and mistakenly) trimmed before dispatch — the trim cut the 3/5 request_changes, 2/5 approve (architect, security, catfish request changes; devex, scope_steward approve). Blocking: hardcoded shared secret in
|
Corrections found by a 5-role pr_review panel plus independent verification (acq internals, cross-repo PR status), then confirmed via a 7-role higher_order consensus vote (85.7% approve) on the fix approach itself: - Consequences/Positive claimed "one shared config-merge/bounded-fetch library replaces the per-kit copies" — false against the ADR's own Decision. Layer 3 mandates each harness kit render its own native config; only the bounded-fetch helper is actually shared. Corrected the claim and stated explicitly that no shared config-merge library exists. - Added a security invariant, in three places (Layer 2 output description, Layer 3 read-list, Security model bullet list): the guest-writable aggregate catalog.json carries only model metadata, never routing/credential fields (host/baseUrl/modelsUrl/keyEnv). Those fields are read exclusively from the Layer 1 read-only facts mount. Without this, a sudo-capable in-guest agent could rewrite the catalog to pair one provider's keyEnv with another provider's allowlisted host, bypassing the Layer 1 env-var-ownership and SSRF checks entirely within caps.network.allow. - "quickstart ADR-0030" and "goose-server ADR 0004" were cited as settled sibling ADRs. Both are, as of this commit, open and unmerged (GSA-TTS/agentic-coding-quickstart#504, #415 — freshly reverified via `gh pr view` before this commit). Added "(pending — org/repo#N)" qualifiers per the repo's fully-qualified cross-repo reference convention. Consensus panel also flagged the Layer 1 artifact-location schema gap (no field locates a provider's normalizer or vendored snapshot) as a design change, not a textual fix, and recommended landing it in its own commit for separate review. That follows in the next commit. markdownlint: 0 issues. Co-authored-by: OpenCode Agent <agent@gsa.gov>
Addresses mogul's 3 unresolved inline comments from the 2026-10-04 review: 1. Fix the broken Mermaid diagram. The parse error was a literal semicolon in a sequence-message label (`credential; timeout + size cap...`) — Mermaid's sequence-diagram grammar treats `;` as a statement separator inside message text, not literal punctuation. Verified both the broken and fixed form directly against @mermaid-js/mermaid-cli (not just visually): the original text reproduces the exact parse error from the review comment; replacing the semicolon with a comma renders clean. 2. Answer "is the containment contract ever enforced, or just trusted?" with a real answer, not more assertion. Dispatched a research subagent against the actual acq.backends/*.sh codebase (quickstart repo) to check, property by property, whether any of the four containment properties (no network, no credential in env, bounded time, bounded output) has an existing reusable enforcement primitive. Confirmed definitively: none do. What exists are two narrow, hand-copied timeout(1) wrappers that FAIL OPEN (run unbounded) when timeout is absent from the guest, and a sibling repo's JS-level fetch()-body size cap that bounds an HTTP response, not a subprocess's stdout. Added this finding directly to the ADR's Negative/risks section, consulted a security-expert review on the wording (verdict: explicit written acknowledgment of an unenforced trust boundary is the correct, honest thing to put in a design record, not an invitation to exploit it, provided it also states the resulting trust decision and a fail-closed requirement — both added), and added a corresponding "must not decide unilaterally" entry so shipping Layer 2 with the contract merely documented, not enforced, requires the same human/CODEOWNERS review as relaxing it outright. 3. Re-verified all cross-repo links before merge, as asked. Checked every GSA-TTS/agentic-coding-* PR/issue reference in the document against its live current state: #504, #435, #415 all still open and pending as cited. Quickstart issue #506 is now CLOSED (fixed by quickstart#512) — updated its one citation (References section) from a forward-looking "tracking... the upstream fix this ADR depends on" to a past-tense "closed, fixed by #512," consistent with the durable-reference convention already used elsewhere in this document for settled dependencies. Also rebased onto main (7 commits: the markdownlint-cli migration, 2 CVE overrides, the opencode kit, 2 releases, 2 dependency bumps) — none touch this file; this resolves the Markdown Lint CI failure that was purely a result of the branch predating main's markdownlint-cli2 CVE fix. Verification: - node .github/linters/run-markdownlint.mjs: clean, 191 files. - Both Mermaid diagrams verified directly against mermaid-cli (not just read): the trust-boundary flow diagram renders without error. - python3 scripts/validate_repo.py: all validators pass. Co-authored-by: OpenCode Agent <agent@gsa.gov>
wz-gsa
left a comment
wz-gsa
left a comment
There was a problem hiding this comment.
AI-assisted review (OpenCode), advisory — needs a human to confirm before it drives a change.
Process correction on my part first: I reviewed this on Sep 22 and Sep 23 (the second a 5-role panel that returned 3/5 request_changes), but posted both as plain PR comments rather than as reviews. So this PR has been sitting at reviewDecision: REVIEW_REQUIRED with zero recorded reviews — from any dashboard or notification view it looks unreviewed, which isn't fair to you and isn't accurate. Converting to a formal review now so the state is visible. The detail is in those two comments; this is a consolidation, not a re-litigation.
Head is still 60a8486 (Sep 14) with no commits since, so I re-verified each finding is still present rather than assuming:
Blocking: hardcoded shared secret, described as a security control
spec.yaml:247 — GOOSE_SERVER__SECRET_KEY: "goose-server-kit-local-loopback" is a literal in a committed kit file, and kits.yaml's parity note advertises the kit as running "with a SHARED SECRET-KEY handshake." A fixed string in a public repo is identical for every install and authenticates nobody — anyone who reads the PR holds it. All three request_changes voters landed on this independently.
The protected surface isn't trivial: goosed executes shell/tool calls in the sandbox, has egress to api.gsa.usai.gov with the injected USAi credential, and runs --enable-scheduler (persistent job execution). docs/decisions/0002-plaintext-http-loopback.md does explain why an unauthenticated server doesn't work against Desktop 1.50.0 (its ACP client always sends a secret) — that part is sound. What it doesn't address is that a public constant satisfies Desktop's non-empty-secret requirement while providing zero authentication value, and the kit format's stated invariant is that real secret material arrives only via acq secret set and never touches kit files.
Not asking you to solve Desktop's protocol requirement — asking that the value not be a committed constant and not be described as a handshake/control in kits.yaml. Generating it per-sandbox at startup would satisfy both.
Should-fix: a sha256 mismatch fails open
files/home/goose-install.sh:157-163 — on digest mismatch the script warns and exit 0, same as its other twelve failure paths. Non-fatal is the right call for most of them (no curl, unsupported arch, extraction failure, no sha256 tool). It's the wrong call for a mismatch, because that's the one signal meaning the artifact is not what was pinned — and provisioning currently can't distinguish "goose is absent because the download failed" from "goose is absent because the bytes were substituted."
The sibling installer in #408 (cloud-gov-install-cf-cli.sh:66-69) exits 1 on exactly this condition, so the stricter precedent already exists in-repo. Either make the mismatch fatal, or write a machine-readable marker the caller can gate on so the degraded state is observable rather than only greppable.
Nit: unused api.github.com egress grant
spec.yaml:104-108 — the comment calls it belt-and-suspenders for a latest-tag lookup, but the installer pins a version and fetches releases/download/${ver}/… directly, so it never calls the API. In the file whose entire job is the egress boundary, an allowlist entry with no consumer is scope without purpose.
What holds (unchanged from Sep 22)
Supply-chain hygiene here is better than most: exact version pin, both arch sha256 digests committed, verify-before-extract, no curl | sh, no latest. I independently re-verified v1.50.0 and both digests against the aaif-goose/goose release API — assets exist at that tag and the pins match.
Also mechanical
mergeStateStatus: DIRTY — real conflicts against current main, which has moved substantially since Sep 14 (markdownlint-cli migration, CVE overrides, several new kits). Worth a rebase whenever you pick this back up.
Standing scope suggestion (not blocking)
integrations/providers/usai/{README.md, emitters/goose.mjs, tests/*} is ~450 lines of provider-emitter work that stands on its own and isn't the kit. Splitting it would make both halves easier to review and let the emitter land independently of the secret-key question above.
2 participants
Summary
Adds goose-server, the first acq chat-harness kit: it runs the goose ACP server (goose serve, "goosed") inside the sandbox and publishes its port so a native goose Desktop app on the host drives it as an external server (goose's documented remote-server pattern). All agent work runs under acq's isolation while goose Desktop stays a thin host UI — extending the platform beyond coding-agent CLIs without virtualizing a GUI. Opt-in mixin, neutral hybrid/v1, no acq core changes, works on sbx and msb.
Eligibility & Accountability
• [x] I am a federal employee or contractor (PRs merge with @GSA-TTS/agentic-coding-team code-owner approval; eligibility is attested here, not authenticated). Anyone may open issues without this.
• [x] I understand this change and can explain it in my own words — I am the author of record and accountable for it.
• [x] By opening this PR, I attest I have the right to release this work under the repository's CC0-1.0 dedication.
AI Assistance Disclosure
•[x] AI tools were used in this contribution. If checked, note which and for what: OpenCode — researched goose's architecture, authored the kit (spec, install/startup/config scripts, custom_usai.json, verify script, README/TROUBLESHOOTING, ADRs), and ran the offline + live verification loop.
•[x] I verified all AI-generated content: no fabricated/hallucinated APIs, dependencies, citations, or placeholder content; any AI-suggested dependency was confirmed to exist. (goose version + sha256 digests pulled from the real GitHub release API; provider/model ids from the USAi catalog and the operator's own goose config; ACP behavior confirmed against a live goose Desktop 1.50.0.)
•[x] Any inherited/reused third-party material retains its original license and is identified (not represented as public domain). (custom_usai.json reproduces the shape of a goose custom-provider definition; no third-party code copied.)
Type of Change
•[ ] New skill pattern
•[ ] New prompt pattern
•[ ] New workflow pattern
•[ ] New agent instructions
•[ ] New lesson learned
•[x] New tool/editor integration (integrations/)
•[ ] Documentation update
•[ ] Bug fix
•[ ] Enhancement to existing pattern
•[ ] Infrastructure/tooling change
Pattern Information
Pattern ID: goose-server (acq isolation kit)
Pattern Type: integration (acq-kit, hybrid/v1 mixin)
Status: experimental (opt-in mixin; not a default GSA kit)
Owners: @GSA-TTS/agentic-coding-team
Motivation
The acq sandbox platform supported coding-agent CLI harnesses (OpenCode, Pi, Prime) but not chat clients. goose Desktop natively supports pointing at an external goose serve server, so we can run the server (which holds all the blast radius — loop, tools, MCP, files, egress) inside the sandbox under acq's controls, and keep the native Desktop UI on the host. This gets goose's capabilities under isolation without virtualizing a desktop GUI, and gives the kit layer its first non-TUI harness shape to reason about.
Testing
•[x] Ran make validate locally (all checks pass) — see Validation Results
•[x] Ran make test (if applicable) — validate-kits --strict, validate_repo.py, scan_unsafe_shell.py, and node --check all clean
•[x] Tested with at least one real scenario — live end-to-end on the msb backend (aarch64) via RUN_ACQ=1 scripts/verify, and a manual connect from real goose Desktop 1.50.0 with a working USAi model turn against the in-sandbox server
•[x] Examples are concrete and realistic
•[x] Commit messages follow Conventional Commits
format (see CONTRIBUTING.md)
Safety Checklist
•[x] No secrets, API keys, tokens, or passwords included (the USAi key is injected at runtime by acq; only a non-secret loopback handshake token ships in the kit env)
•[x] No PII (names, emails, SSNs, addresses)
•[x] No CUI (Controlled Unclassified Information)
•[x] No internal URLs, hostnames, or system paths (only the public USAi gateway host and public GitHub release hosts, all deny-default allow-listed)
•[x] No customer data or proprietary information
•[x] Pattern defines prohibited_content in output contract (if applicable) — handled at the kit/schema level
Quality Checklist
•[x] Frontmatter includes all required fields (spec.yaml
passes the hybrid/v1 schema)
•[x] Plain language used (Grade 10 or below preferred)
•[x] Examples included (Quick start, Desktop connect walkthrough, TROUBLESHOOTING)
•[x] Prerequisites clearly stated
•[x] Output contract defined (backend-parity note, deny-default egress, secret handling)
•[x] Status set to experimental for new patterns (opt-in mixin)
Validation Results
============================================================ Running: Frontmatter schema validation ============================================================ ✓ .agents/skills/agentic-actions-auditor/SKILL.md ✓ .agents/skills/backdoor-review/SKILL.md ✓ .agents/skills/communications/agentic-value-analyst/SKILL.md ✓ .agents/skills/communications/artifact-brief/SKILL.md ✓ .agents/skills/communications/artifact-qa/SKILL.md ✓ .agents/skills/communications/narrative-architect/SKILL.md ✓ .agents/skills/communications/one-pager/SKILL.md ✓ .agents/skills/communications/slide-deck/SKILL.md ✓ .agents/skills/communications/software-delivery-explainer/SKILL.md ✓ .agents/skills/communications/technical-concept-translator/SKILL.md ✓ .agents/skills/communications/visual-direction/SKILL.md ✓ .agents/skills/compliance-claim-checker/SKILL.md ✓ .agents/skills/dependency-analysis/SKILL.md ✓ .agents/skills/documentation-review/SKILL.md ✓ .agents/skills/frontend/accessibility-review/SKILL.md ✓ .agents/skills/frontend/federal-service-blueprint/SKILL.md ✓ .agents/skills/frontend/plain-language-review/SKILL.md ✓ .agents/skills/frontend/uswds-form-flow/SKILL.md ✓ .agents/skills/frontend/uswds-landing-page/SKILL.md ✓ .agents/skills/frontend/uswds-prototype/SKILL.md ✓ .agents/skills/incident-evidence-review/SKILL.md ✓ .agents/skills/least-privilege-review/SKILL.md ✓ .agents/skills/meta/pattern-router/SKILL.md ✓ .agents/skills/outreach/explainer-gif/SKILL.md ✓ .agents/skills/outreach/explainer-video/SKILL.md ✓ .agents/skills/over-engineering-review/SKILL.md ✓ .agents/skills/safe-shell-script-author/SKILL.md ✓ .agents/skills/secure-code-review/SKILL.md ✓ .agents/skills/test-generation/SKILL.md ✓ .agents/skills/untrusted-input-boundary-review/SKILL.md ✓ AGENTS.md ✓ agents/documentation/AGENTS.md ✓ agents/general/AGENTS.md ✓ agents/security-review/AGENTS.md ✓ lessons-learned/example-agentic-session/SKILL.md ✓ prompts/planning/implementation-plan/SKILL.md ✓ prompts/review/qa-round/SKILL.md ✓ prompts/security/safe-code-review/SKILL.md ✓ templates/agent-template/AGENTS.md ✓ templates/lesson-template/SKILL.md ✓ templates/prompt-template/SKILL.md ✓ templates/skill-template/SKILL.md ✓ templates/workflow-template/SKILL.md ✓ workflows/design-artifact/SKILL.md ✓ workflows/issue-to-merge-request/SKILL.md ✓ workflows/qa-round/SKILL.md ✓ workflows/security-scan-review/SKILL.md 47/47 files passed validation ============================================================ Running: Sensitive terms scan ============================================================ ⚠ CODE_OF_CONDUCT.md (warnings - review recommended) Line 39: Controlled Unclassified Information marker - **No sensitive data, anywhere.** Do not post secrets, credentials, PII, CUI, Line 39: Personally Identifiable Information marker - **No sensitive data, anywhere.** Do not post secrets, credentials, PII, CUI, ⚠ examples/frontend/README.md (warnings - review recommended) Line 66: Personally Identifiable Information marker 5. **Use placeholder content** — No real PII, secrets, or internal URLs ⚠ .agents/skills/incident-evidence-review/SKILL.md (warnings - review recommended) Line 197: Personally Identifiable Information marker Flag any **real** PII, IOC (IP, domain, hash), hostname, or victim identifier Line 251: Personally Identifiable Information marker - The output contains **no** real incident data, PII, IOCs, or hostnames — every ⚠ .agents/skills/fixtures/README.md (warnings - review recommended) Line 5: Controlled Unclassified Information marker secrets, PII, CUI, working exploit payloads, live IOCs, or internal hostnames. Line 5: Personally Identifiable Information marker secrets, PII, CUI, working exploit payloads, live IOCs, or internal hostnames. ⚠ .agents/skills/communications/technical-concept-translator/reference/concept-library.md (warnings - review recommended) Line 261: Controlled Unclassified Information marker controls: Sandbox is FIPS-Low, local-dev, no PII/CUI; the agent MUST NEVER access or modify producti Line 261: Personally Identifiable Information marker controls: Sandbox is FIPS-Low, local-dev, no PII/CUI; the agent MUST NEVER access or modify producti ⚠ .agents/skills/outreach/explainer-video/SKILL.md (warnings - review recommended) Line 117: Controlled Unclassified Information marker - The content contains secrets, real PII, or real CUI (never send to a renderer Line 117: Personally Identifiable Information marker - The content contains secrets, real PII, or real CUI (never send to a renderer ============================================================ SUMMARY ============================================================ ⚠ 11 Tier 2 warning(s) - review recommended (CUI/PII/data terms) These may be legitimate in security documentation. Use --strict to treat as errors, or verify they're safe. NOTE: This is a lightweight hygiene check. Use gitleaks or truffleHog for production secret scanning. ============================================================ Running: Cross-reference & delegation-cycle validation ============================================================ ✓ 41 patterns: all cross-references resolve, no delegation cycles ============================================================ Running: Network-tier baseline allowlist validation ============================================================ ✓ network-tier baseline valid: 148 core + 43 extended host(s) ============================================================ Running: Balanced-allowlist additions guard (deterministic risk lint) ============================================================ WARNING: core: 'docker-images-prod.6aa30f8b08e16409b46e0173d6de2f56.r2.cloudflarestorage.com' has deep subdomain nesting (5 labels). WARNING: core: 'isrg.trustid.ocsp.identrust.com:80' has deep subdomain nesting (5 labels). WARNING: extended: 'vscode.download.prss.microsoft.com' has deep subdomain nesting (5 labels). ⚠ balanced-allowlist additions guard: 3 warning(s) for reviewer attention (advisory — defer to CODEOWNERS + the per-host `why`). ============================================================ SUMMARY ============================================================ ✓ All validators passed $ python3 scripts/scan_unsafe_shell.py integrations/isolation/acq-kits/goose-server unsafe-shell scan: 0 error(s), 0 warning(s) Live end-to-end (msb backend, aarch64): RUN_ACQ=1 scripts/verify PASS goose CLI installed (create-time) | 1.50.0 PASS goose global config selects the custom_usai (USAi) provider PASS custom_usai provider definition staged in ~/.config/goose/custom_providers/ PASS CUSTOM_USAI_API_KEY present in the guest env PASS goosed answers /status on 127.0.0.1:3284 PASS /status 200 with kit secret; /acp?token=… → 406 (handshake accepted) PASS goosed LISTEN bound to 0.0.0.0 in-guest PASS respawn supervisor running (exactly one) PASS goosed re-launched after being killed (self-heal) All checks passed.Related Issues
Relates to GSA-TTS/agentic-coding-quickstart#466 (first-class acq run goose via kit-inference — follow-up)
Additional Context
•Design decisions are recorded in goose-server/docs/decisions/: (0001) server-in-sandbox / Desktop-on-host split; (0002) plaintext HTTP + secret-key handshake over host-loopback; (0003) USAi via a custom_usai custom provider; (0004) alignment with the in-flight devenv / harness-adapter direction.
•Three findings from live testing shaped the final kit and are documented in the ADRs: (1) use the
.tar.gz
release asset because the shell-docker template has no bzip2; (2) the server must define the same named provider Desktop uses (custom_usai), or agent init fails; (3) goose Desktop 1.50.0's ACP client always sends a secret, so the server runs with GOOSE_SERVER__SECRET_KEY (not unauthenticated).
•Forward-looking: ADR-0004 records expected migration to devenv process supervision, a declarative install, and acq kit-inference once those directions land — so the hand-rolled supervisor loop and imperative installer are tracked debt, not silent debt.
•Scope hygiene: this branch/commit contains only goose-server changes; unrelated mcp-gateway/* edits in the working tree were deliberately excluded.
•Commit signing: the commit is currently unsigned (no SSH signing key in the authoring environment); amend with -S before merge if required.
Reviewer Notes
Reviewers should verify:
•[ ] spec.yaml frontmatter is valid and complete (hybrid/v1)
•[ ] Safety checklist items confirmed (no secrets; egress is deny-default, no leading-* hosts)
•[ ] Pattern is clear and actionable (Quick start + Desktop connect walkthrough)
•[ ] Examples are realistic (placeholders for / )
•[ ] No sensitive information included
•[ ] Pattern follows repository conventions (self-contained kit; backend-parity note; ADRs; durable references)
•[ ] make validate passes in CI
•[ ] The auth posture (cleartext loopback handshake token) and its rationale (sandbox is the boundary; TLS is a tracked follow-up) are acceptable — see ADR-0002