Conversation
Records the design that emerged from three rounds of nexus-agents 7-role consensus review this session (7-0 reject on live-poll-with- no-discovery; 4-3 approve/reject split on host-side third-party code execution next to secrets; 5-2 approve on in-guest polling that reuses the harness's own already-standing network+credential trust boundary), plus follow-up research into the actual duplication this solves: - acq's secret store dedupes by service-name string only, never by host: usai-provider registers USAi's credential as service `usai`; the in-flight goose-server kit independently registers the SAME credential, same host, as service `goose-usai`. Confirmed via acq.backends/secret-store.sh's _acq_secret_key. - The two kits' copy-on-first-boot/merge-without-clobbering config scripts are ~230-260 lines each, ~150-190 of which per kit are the same algorithm, hand-copied (goose's own header admits it mirrors usai-provider's). - The bounded-fetch-with-timeout-and-size-cap helper has already been copy-pasted once and has already drifted (the copy gained 401/403 handling the original lacks). Decision: three-layer design — (1) provider kits self-register via a well-known guest-path facts file (mirrors the already-shipped zscaler-ca-certificate -> openchamber/paseo pattern) and ship their OWN normalizer + vendored fallback snapshot; (2) a new, vendor- agnostic models orchestrator kit (pure orchestration, zero vendor knowledge, core-repo promotion candidate) discovers providers, refreshes a host-mounted-volume cache on a TTL, falls back to last-good-cache then to the provider's own snapshot on any failure, and aggregates into one neutral catalog; (3) each harness kit reads that aggregate + config-kit defaults and renders its OWN native config, owning its full configuration lifecycle end-to-end. N provider normalizers + M harness renderers, not NxM per-pair emitters. Incorporates both dissenting votes from the final round as binding conditions: an env-var-ownership check (a provider kit cannot name a credential env var it doesn't itself declare) and SSRF hardening (fetched host must equal the facts file's own declared host, itself on the sandbox's already-effective network allow-list). No hybrid/v1 schema change required — composes entirely from already-shipped primitives (caps.network.allow union, sequential kit-startup ordering, well-known-guest-path self-registration, the neutral volumes[] vocabulary for the cache mount). Co-authored-by: OpenCode Agent <agent@gsa.gov>
|
CI-green, mergeable, no other blockers I can see. I can't self-approve (author). Could a maintainer review/merge this ADR when convenient? I have follow-up implementation work (#424) that cites this ADR's path — low risk either way since it's docs-only, but merging unblocks a clean citation rather than a provisional one. |
There was a problem hiding this comment.
See note; agents have passwordless sudo, so root-ownership is not the barrier implied. It's possible to mount a volume :ro, but we can't do that if we want the kit startup to be how we drop a file there. Other ideas? Maybe acq is responsible for grabbing the file from the kit and putting it there, and it's :ro?
|
Also: This clarity of this ADR would be vastly improved by the addition of Mermaid sequence diagrams. |
…ed cache Corrects ADR 0003 after PR #423 review (mogul): the Layer 1 provider-facts mechanism relied on a root-owned guest path being un-writable, but acq grants the in-sandbox agent passwordless sudo, so guest root-ownership is not a trust boundary against a prompt-injected agent. It could sudo-write a bogus provider facts file that the models orchestrator would then trust. Changes: - Layer 1: provider kits SHIP a static facts file in their (pinned) kit dir; acq reads it on the host, validates it host-side (env-var-ownership + SSRF + schema), and mounts it read-only into the guest. No provider-kit code runs on the host (static data read only); the normalizer still runs in-guest. - Layer 2: drop the shared host-side cache + TTL entirely. A cache written by one sandbox and read by another is a cross-sandbox poisoning channel. The catalog is generated per-sandbox at startup, kept guest-local (rw), and never shared; the deepest fallback is the provider's vendored snapshot on the read-only mount. - Add a note on startup integrity across restarts (orchestrator/normalizer code invoked from the read-only mount so it cannot be tampered between restarts). - Add two Mermaid sequence diagrams (GitHub/Host/Guest boxes) contrasting the rejected guest-write path with the chosen host-materialized read-only path. - Reference quickstart ADR-0030 (Host-Authoritative Sandbox Configuration) as the general principle and mount/exec mechanism of record. Co-authored-by: OpenCode [claude_4_8_opus] <bret.mogilefsky@gsa.gov>
|
Pushed The security issue you flagged is removed. The Layer 1 mechanism no longer relies on a root-owned guest path being un-writable. You're right that this isn't a barrier: acq deliberately gives the in-sandbox agent passwordless sudo, so a prompt-injected agent can Layer 1 — host-authoritative, guest read-only. Provider kits now ship a static facts file in their (pinned) kit directory rather than writing one from a Layer 2 — shared host cache dropped. Per your call, the cross-sandbox host cache + TTL is gone entirely: a cache written by one sandbox and read by another is a cross-sandbox poisoning channel. The catalog is now generated per-sandbox at startup, kept guest-local (rw), and never shared. Deepest fallback is the provider's vendored snapshot on the Your restart-tamper concern is addressed too. You noted the models kit startup script could be tampered between restarts. Correct — and it's the general form of the same problem, since acq's restart heal re-runs guest-resident kit code. The fix is the same boundary: the orchestrator/normalizer code is staged into the General principle recorded separately. The mount/exec mechanism and the broader rule ("acq must not trust guest-generated/tamperable state to configure the sandbox; migrate the existing Mermaid diagrams added (your other request): two markdownlint-cli2: 0 issues. |
The ADR had accreted into a change-log of the review (consensus rounds, vote tallies, "corrected from an earlier draft" notes, reviewer names, dueling Option 2/3 correction blocks). Once merged, an ADR is documentation of how the thing works, not a journal of how we arrived at it. Rewrite it to describe the agreed end state directly: - Context states the problem (duplicated credential registration, duplicated merge code, manual release-coupled refresh) and the design goals, without the panel/vote narrative. - Decision describes Layers 1-3 in the present tense as the design, folding the host-authoritative read-only facts/code rationale inline instead of as a post-hoc correction. - A single "Security model" section replaces the scattered "hardening conditions" attributed to reviewer roles. - One "Trust-boundary flow" diagram (the chosen design); the rejected guest-write sequence diagram is dropped (its point now lives as one line in Alternatives + the Layer 1 rationale). - Alternatives Considered keeps WHAT was excluded and WHY, dropping vote counts and session references. No behavioral change; documentation only. Co-authored-by: OpenCode [claude_4_8_opus] <bret.mogilefsky@gsa.gov>
|
Pushed Your point is right: once this merges, the ADR is reference documentation. The prior revision had turned into a review journal — consensus rounds, vote tallies (7-0 / 4-3 / 5-2), reviewer role names, "corrected from an earlier draft" blocks, and two separate "the guest-write approach was wrong" correction notes. A reader arriving after merge has to reconstruct the actual design from the diffs between rejected and accepted states. What changed:
No design change from markdownlint: 0 issues; the remaining Mermaid diagram parses (mermaid v11). (The provenance you'd normally want preserved — the consensus rounds and the review that caught the sudo/root-ownership flaw — lives durably in this PR's history and conversation, which is the right home for it.) |
… prose-only Adds the actual quickstart#506 link to the References section — the prior commit named the issue in prose but didn't link it since #506 wasn't filed yet at write time.
|
I'm taking on GSA-TTS/agentic-coding-quickstart#506 so we can merge this one without caveats about that race-condition. |
|
`integrations/isolation/docs/decisions/0003-` was claimed by two open PRs: #416 (serviceGateways vocabulary, opened 2026-09-15) and this one (opened 2026-09-18). The filenames differ, so git merges both cleanly and leaves two ADR-0003s in the same directory — every later "ADR 0003 (isolation)" reference becomes ambiguous. #416 claimed it first and its number is not cited anywhere else, so this PR moves. 0004 is free: `main` has 0001 and 0002, and no other open PR claims it. Only the file name and the H1 change. The "usai-provider ADR 0003" reference at line 353 is a DIFFERENT, kit-local ADR under acq-kits/usai-provider/docs/decisions/ and is deliberately left alone. AI-assisted (OpenCode). Human review and merge still required.
#423's ADR moved from `0003-neutral-model-provider-discovery.md` to `0004-` to clear a collision with #416, which claimed `integrations/isolation/docs/ decisions/0003-` first. Update the two comment references here so they do not dangle once that lands. Comment-only; no behavior change. `node --check` and a YAML parse both pass. AI-assisted (OpenCode). Human review and merge still required.
wz-gsa
changed the title
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>
…der directory
Design amendment, landed separately from the accuracy/citation fixes
in the prior commit per the consensus panel's recommendation (7-role
higher_order vote, 85.7% approve): several voters flagged this as a
schema change to the Layer 1 contract, not a wording fix, and asked
that it be independently reviewable/revertable from the citation and
claim corrections.
The gap: the v1 facts file has no field locating a provider's
normalizer or vendored snapshot, yet Layer 2 steps 2 and 4 require the
orchestrator to resolve both. Two independent provider-kit authors
implementing v1 as written could place those artifacts at different
conventions, and the orchestrator would find neither — undermining the
ADR's own central interoperability claim ("a third-party provider kit
works with no PR into the orchestrator").
Fix: replace the single per-provider JSON file with a fixed
per-provider directory (facts.json + normalizer + snapshot.json),
materialized read-only as a unit. Updated Layer 2's glob (step 1) and
its normalizer/snapshot resolution (steps 2 and 4) to locate both
artifacts as siblings of the facts.json just read. Layer 3's read-list
(added in the prior commit) is updated to the corrected path.
The mermaid trust-boundary diagram is left unchanged: its RO
participant description and the one path-like string in the diagram
(`providers/evil.json` in the adversary-attempt line) don't cite the
specific single-file convention this commit changes, so nothing there
needed to move.
markdownlint: 0 issues. Content verified against the full draft
produced during review (byte-identical diff, confirmed before commit).
Co-authored-by: OpenCode Agent <agent@gsa.gov>
Independent review + fixes (AI-assisted, OpenCode)Ran a 5-role Before applying any fix, ran the proposed fix set through a 7-role Commit 1 — accuracy + a security invariant (
Commit 2 — Layer 1 artifact restructure (
markdownlint: 0 issues on both commits. Full panel transcripts and the verification subagent's findings available on request. Human review and merge still required — this is advisory, not a self-approval. |
2 participants
Summary
Records the model-provider discovery design from this session's nexus-agents consensus review work (3 rounds: 7-0 reject → 4-3 split → 5-2 approve), plus the concrete duplication findings that motivate it.
Key findings backing this ADR
acq's secret store dedupes by service-name string only, never by host.usai-providerregisters USAi's credential as serviceusai; the in-flightgoose-serverkit independently registers the same credential, same host, asgoose-usai. Confirmed viaacq.backends/secret-store.sh's_acq_secret_key.Decision
Three layers, no
hybrid/v1schema change required:zscaler-ca-certificate→openchamber/paseopattern), each shipping its own normalizer + vendored fallback snapshot.Both dissenting votes from the final consensus round are incorporated as binding security conditions (env-var-ownership check; SSRF host-matching against the already-effective network allow-list).
Verification
markdownlint-cli2: 0 issues.No implementation in this PR — issues to follow, per this repo's own ADR-then-issues convention.