Define dynamic repository enclave contract - #8196
Conversation
Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
lpcox
left a comment
lpcox
left a comment
There was a problem hiding this comment.
The dynamic enclave contract has four blocking gaps: dynamic script access is not implementable with the documented topology, the one-repository guarantee contradicts current static GitHub identity scope, runtime identity delegation lacks an implementable trust/control-plane contract, and dynamic tool policy is not restricted to repository-scopable read-only operations.
|
|
||
| Dynamic invocation flow: | ||
|
|
||
| 1. The primary agent calls `enclave_run_script` or `enclave_run_agent` with one |
There was a problem hiding this comment.
Dynamic mode cannot currently support enclave_run_script: script enclaves are explicitly no-network, and this mode provides no immutable seed. The script would have no path to repository contents. Please either restrict dynamic admission to agent entries or define a secure repository-access topology for dynamic script execution.
There was a problem hiding this comment.
Addressed in 1f65008: dynamic admission now rejects enclave_run_script and is limited to enclave_run_agent unless a future ADR defines a secure dynamic script repository-access topology.
| or a dynamic repository policy. Caller-controlled repository names are never | ||
| interpreted as policy. They cannot alter sensitivity, tools, credentials, | ||
| runtime, model, image, network, filesystem, resource limits, timeouts, or | ||
| quotas. Cross-repository aggregation occurs only in the primary agent by |
There was a problem hiding this comment.
This one-repository guarantee does not hold for existing static GitHub-enabled agent enclaves. Their job-lifetime mcpg identity covers the union of configured repositories, so an invocation assigned one seed can still read sibling repositories through GitHub MCP. Please scope this guarantee to dynamic mode or require per-invocation repository-scoped identities for static GitHub access too.
There was a problem hiding this comment.
Addressed in 1f65008: the one-repository guarantee is now scoped to dynamic admissions and static seed exposure, with an explicit caveat for existing static GitHub-enabled agent enclaves using a union-scoped job-lifetime mcpg identity.
| 2. AWF serializes admission through the shared enclave lane, validates the | ||
| selector against the compiler envelope, and debits the shared repository | ||
| ledger before execution. | ||
| 3. The compiler-owned GitHub MCP path creates or confirms an invocation-scoped |
There was a problem hiding this comment.
This does not yet define an implementable trust contract. The compiler is not a runtime service, and mcpg currently receives fixed identities and policies at startup. Please identify the component responsible for delegation and specify the authenticated control channel, capability ownership, atomic create/confirm/revoke semantics, restart recovery, and required component version gates.
There was a problem hiding this comment.
Addressed in 1f65008: the docs now define mcpg’s dynamic-delegation controller, the private authenticated control channel, AWF-owned control capability, atomic create/confirm/revoke semantics, restart recovery, and required version gates.
|
|
||
| - allowed owners or exact owner/repository patterns; | ||
| - sensitivity classification and disclosure bucket; | ||
| - permitted executor types and GitHub MCP tools; |
There was a problem hiding this comment.
The contract should require a versioned, closed allowlist of repository-scopable read-only GitHub tools. Merely requiring tools to be policy-approved could permit write operations or tools whose arguments escape the admitted repository scope. Those operations should fail closed until their repository confinement can be enforced.
There was a problem hiding this comment.
Addressed in 1f65008: dynamic mode now requires github-repository-read-v1, a closed versioned allowlist of repository-scopable read-only tools, and fails closed for writes, unscoped search, repository discovery, or tools that cannot be mechanically confined.
|
@copilot address the review feedback |
Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
Addressed in |
There was a problem hiding this comment.
🟡 Changes recommended
Core wire, delegation, and script-isolation boundaries remain undefined, and planned behavior is presented as currently supported.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Documents the planned security contract for dynamic repository admission alongside static seed-backed enclaves.
Changes:
- Adds an ADR covering policy, identity, quotas, errors, and threats.
- Extends the architecture guide with dynamic admission flow and controls.
File summaries
| File | Description |
|---|---|
docs/adr/0001-agent-enclaves.md |
Defines the proposed dynamic enclave contract. |
docs/enclaves-architecture.md |
Adds dynamic admission architecture and threat model. |
Review details
Suppressed comments (3)
docs/adr/0001-agent-enclaves.md:53
- The “closed” envelope is only a conceptual field list: it has no wire field names, types, units, required/optional rules, version, transport, or matching semantics. “Includes at least” also conflicts with rejecting every unknown field. Independent compiler and AWF implementations therefore cannot serialize and validate the same stable contract; define a versioned normative schema and its delivery mechanism.
- maximum admitted repository count, invocation count, or concurrency; or
- envelope expiry time.
docs/adr/0001-agent-enclaves.md:75
- The credential flow skips the authenticated delegation protocol between AWF and the compiler-owned mcpg path. Binding claims describe the resulting identity, but not who calls which operation, how the request is authorized and bound to the admitted invocation, what artifact is returned, or how revocation is acknowledged. Without those trust-boundary details, the confused-deputy and replay guarantees are not an implementable end-to-end contract.
new versioned policy proves repository confinement.
AWF rejects any envelope field it does not understand and fails closed when the
docs/adr/0001-agent-enclaves.md:85
- Dynamic mode includes
enclave_run_script, but this mount/access model does not preserve the documented no-network script boundary. If the identity is visible to caller-supplied Python, read-only access still lets the script return it, violating line 71; if only a trusted broker sees it, the contract must instead define trusted repository materialization before launching the no-network script. Clarify that separation, or restrict dynamic admission to an executor that can safely use MCP.
The primary agent never receives GitHub credentials, repository seeds, dynamic
policy contents beyond the public tool schema, mcpg identity material, or a
direct transport to the enclave backend. The compiler is not a runtime service:
- Files reviewed: 2/2 changed files
- Comments generated: 2
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| - **Dynamic GitHub-MCP-backed mode**: the compiler supplies a policy envelope | ||
| instead of a seed catalog. Each invocation supplies only a canonical | ||
| `owner/repo` selector, a bounded prompt or script, and a finite response | ||
| schema. AWF asks the compiler-owned GitHub MCP path to admit one matching |
| Repository admission has two modes, both served by this same MCP backend: | ||
|
|
||
| - **Static seed-backed mode** — the existing `repos` lists in enclave entries | ||
| form the trusted repository catalog. AWF stages immutable seeds for that | ||
| catalog before primary-agent work begins. Each invocation selects one catalog | ||
| entry and exposes only that repository to the single-use executor. | ||
| - **Dynamic GitHub-MCP-backed mode** — the compiler provides a closed policy |
Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
|
@copilot address review feedback |
The outstanding review findings are addressed by 83dc208: it marks dynamic admission as proposed/unimplemented until version-gated support exists, aligns the config specification with the required static seed list, and defines the selector grammar and exact canonical bytes. |
|
✅ Copilot review passed with no inline comments. @copilot Add the |
The two words are used throughout and mean different things, but nothing said
so. An executor is broker-side machinery and a config kind — the code under
containers/enclave/{script,agent}-executor/ ships into the enclave-mcp-server
image and holds the Docker socket. An enclave is the ephemeral container an
executor launches per invocation, with its own image, name, and entrypoint.
One executor launches many enclaves; the executor is trusted and the enclave
is not, so conflating them inverts the trust boundary.
Adds a terminology note rather than renaming anything. "single-use executor"
for the sandbox is pre-existing prose from #8196 and appears on main, so a
partial rename would leave the docs inconsistent with the contract language,
and executor_bearer is mcpg's wire field name. The note explains the overload
instead. The mermaid node, which this PR introduced, is relabelled to name the
container an enclave.
Refs #8195
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
…entities (#8276) * feat: run dynamic agent enclaves with repository-scoped identities Implements the remaining AWF runtime path for dynamic, GitHub-MCP-backed agent enclaves (ADR 0001, #8195). A dynamic entry now selects one canonical repository at runtime, receives one short-lived `github-repository-read-v1` identity minted through mcpg's private delegation control channel, and reads that repository through GitHub MCP without cloning or mounting a seed. Startup and private handoff: - Validate the compiler-issued control endpoint strictly: literal loopback hosts only (`localhost` is rejected as resolver-dependent), the exact mcpg control path, no credentials/query/fragment, and normalized default-port semantics so `:80` and an omitted port both resolve to 80. - Take custody of the endpoint and capability before any inherited environment is assembled, stage them into the 0700 private root with exclusive 0600 files, and never mount either into the broker, the executor, the model sidecar, the general MCP route, or the delegated data plane. - A dynamic-only entry needs no GH_TOKEN/GITHUB_TOKEN, clones nothing, writes no seed catalog, and mounts neither /awf/seed nor a seed map. Static and separate static+dynamic behaviour is unchanged. Control plane: - Add a strict mcpg v0.4.17 client for create-or-confirm, status, reconcile, revoke, and revoke-by-labels. Operation paths are siblings of the controller name, `requested_ttl` is an integer number of Go nanoseconds, bodies are bounded, and every response is validated against the request before it is trusted. - Recovery calls status, revokes stale labelled identities, then reconciles; admissions stay blocked until that sequence succeeds. Broker integration: - Construct one DynamicRepositoryRegistry per dynamic entry and route enclave_run_agent through canonical admission before content exposure. Because mcpg's control listener is published on host loopback only, the control client runs in the AWF host process and the broker reaches it over an AWF-private request/response directory bind-mounted only into the broker. - Reserve and settle repository, invocation, output-byte, and execution-second quotas on every terminal path; register admissions in the same live per-repository disclosure ledger; revoke on success, agent failure, schema failure, timeout, cancellation, broker error, and shutdown. An unresolved revocation blocks further admissions and downgrades the invocation to the canonical error. Executor: - Generate an invocation-private, bearer-only GitHub MCP configuration limited to list_issues and issue_read for the one admitted repository, with no job token, control capability, handle, envelope, state, generation, or control endpoint, and no route to the control listener. - Update the dynamic agent instructions away from /awf/seed and prohibit cloning, arbitrary URLs, the GitHub CLI, writes, unscoped search, organization/global discovery, and sibling-repository access. The optional admitted SHA is omitted rather than obtained through a broader token or tool, so reads are audited as live. Refs #8195 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * fix: settle dynamic enclave identities on unexpected pipeline errors An internal error escaping the executor pipeline after canonical admission could leave a delegated identity live until the shutdown label sweep. Settle and revoke from a finally block keyed by the invocations that actually reached admission, so every terminal path — including an unexpected throw — releases the reservation and revokes the identity exactly once. Also stop the admission channel loop in the test-only reset helper so a leftover interval cannot outlive a suite. Refs #8195 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * fix: make the dynamic enclave path reachable end to end Three defects each made a dynamic run non-functional, and each was invisible to the suite because the tests entered the pipeline below the broken seam. - runMainWorkflow validated the enclave configuration before prepareEnclaves had taken custody of the delegation handoff, so every dynamic run aborted at startup with the missing-handoff error. Taking custody deletes both environment variables, so exactly one caller may read them; the early gate now validates structure only and defers the handoff check to that single custodian, which still runs before any container is created. - The per-invocation delegation binding was passed to the runner in a config object that runEnclaveContainer never read, so every executor launched with AWF_ENCLAVE_AGENT_DYNAMIC_REPO=undefined and died with a configuration error. The binding now flows through the spec builder, which revalidates the admitted repository and read mode; only those two scalars may vary per invocation, and the reconciliation paths skip the launch-only checks. - The broker's per-launch network-isolation proof only admitted the shared gateway when the static GitHub profile was enabled, so it rejected the very topology a dynamic run creates. It now admits the gateway for either shape, and the broker receives the real gateway container name for a dynamic entry. Adds regression coverage for all three at the layer that was bypassed: runMainWorkflow with a dynamic entry, createAgentRunner through the real runner, and the dynamic steady-state topology. Refs #8195 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * test: cover the delegation channel mount under a split DinD filesystem The AWF-private admission channel is a bind mount, so on an ARC/DinD runner the Docker daemon must see it at the same prefixed path the runner wrote it to. If that ever diverged, the broker would poll an empty directory and every dynamic admission would time out into the canonical denial — a fail-closed outcome, but an opaque one. The translation already happens (the channel is part of the server volume list that applyHostPathPrefixToVolumes rewrites); this pins it, alongside the work mount, so the channel carries no special-case risk relative to the paths that already work under DinD. Refs #8195 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * docs: correct the delegation control-plane reachability claim The prose asserted that neither the primary agent, the broker, the executor, nor the model sidecar "can route to" mcpg's delegation control listener. That claim was inherited from gh-aw's own source comment and describes the host publication, not container-to-container reachability. Docker's -p 127.0.0.1 publication only bounds who can reach the port *through the host*. Under network isolation gh-aw binds the in-container listener to 0.0.0.0 — it has to, because Docker NATs a published port to the container's bridge IP and a container-local 127.0.0.1 bind would be unreachable. A peer that shares a Docker network with mcpg addresses the container IP directly and never traverses the published port, so co-attachment rather than publication scope decides reachability. The single-use executor meets mcpg at 172.31.0.40 on the enclave agent network, so it is not held off the control plane by publication. Nothing about the implementation changes: the control plane was always protected by the AWF-only capability, which is never placed in any container's environment or mount and which mcpg checks on every request. The correction is to state that authentication is the enforced control, and to stop implying a routing guarantee this repository does not establish. Also documents the network the primary agent actually uses to reach mcpg (awf-net, via topologyAttach). Refs #8195 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * docs: diagram the dynamic enclave network homing The prose said the broker is confined to one network and mcpg is co-attached with every peer it serves, but nothing showed it. That asymmetry is the whole reason the control plane relies on capability authentication rather than unreachability, and it is the detail an earlier revision of this branch got wrong, so it is worth making visible rather than leaving in a paragraph. Adds a mermaid diagram to the dynamic runtime topology section showing each component and the networks it is homed on, including mcpg's four planes and its image versus container name. Mermaid matches the existing convention in docs/gvisor-integration.md and docs/sbx-integration.md and stays diffable; the syntax was validated by parsing it with the mermaid package rather than assumed. Refs #8195 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * docs: define executor versus enclave The two words are used throughout and mean different things, but nothing said so. An executor is broker-side machinery and a config kind — the code under containers/enclave/{script,agent}-executor/ ships into the enclave-mcp-server image and holds the Docker socket. An enclave is the ephemeral container an executor launches per invocation, with its own image, name, and entrypoint. One executor launches many enclaves; the executor is trusted and the enclave is not, so conflating them inverts the trust boundary. Adds a terminology note rather than renaming anything. "single-use executor" for the sandbox is pre-existing prose from #8196 and appears on main, so a partial rename would leave the docs inconsistent with the contract language, and executor_bearer is mcpg's wire field name. The note explains the overload instead. The mermaid node, which this PR introduced, is relabelled to name the container an enclave. Refs #8195 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Dynamic repository enclaves need a stable security contract for repositories selected at invocation time, without letting caller-controlled names influence trusted policy. This documents how dynamic GitHub-MCP-backed admission coexists with existing static seed-backed enclaves.
ADR
docs/adr/0001-agent-enclaves.md.Policy and identity contract
Runtime and threat model
Architecture doc update
docs/enclaves-architecture.mdwith the dynamic enclave flow and threat model.