feat(#578): credential broker — the egress-stamping layer (phase 2/4) by Weegy · Pull Request #772 · byte5ai/omadia

Weegy · 2026-08-20T12:35:58Z

Summary

Phase 2/4 of #578 (credential keychain with grants and a broker). The
egress-stamping layer: CredentialBroker, fail-closed, every violation
counted and (when wired) audited.

Base: feat/578-keychain (phase 1/4, PR #769), not main. Stacked
per the phase cut in docs/plans/phase4a-578-keychain-prompt-2026-08-20.md
— this PR should not be merged before #769, and its diff (once #769 is
in) is exactly the 6 new files below.

What this delivers

The broker: an agent names a service credential and describes a request
(host, method, path). CredentialBroker.request() decides whether the
calling principal, right now, may use that credential for exactly that
request — and if so, decrypts the secret and stamps it onto the outbound
call itself. The caller gets back only the response. It never sees the
secret, on either the success or the failure path.

src/credentials/requestMatching.ts: normalizeHost/normalizeMethod,
normalizePathForMatch (traversal-safe — resolves ./.. via
path.posix.normalize, which clamps at the root for an absolute path
rather than escaping above it), matchPath (boundary-safe prefix
matching: a prefix of /v1 matches /v1/anything but not /v1extra,
closing the classic naive-startsWith hole alongside the traversal one).
src/credentials/brokerMetrics.ts: counters built the SAME shape as
securityScreenMetrics.ts (Instrument the security screener: an always-failing screener is indistinguishable from an occasionally-unavailable one #749), per the scoping prompt's explicit
instruction to reuse that pattern — every outcome is counted, not just
logged, with a consecutive-denial streak alert (same threshold and
reasoning as UNSCREENABLE_STREAK_ALERT).
src/credentials/broker.ts: CredentialBroker. Every check is a
fail-closed gate: unknown/revoked credential, wrong kind (personal
credentials are broker-exempt — they're for phase 3's keychain-asks),
no active grant, host/method/path mismatch against the credential's own
declaration, a malformed declaration, or a store outage. Every denial
is counted (recordBrokerOutcome) and, when onAudit is wired, emitted
as a BrokerAuditEvent — fingerprint only, never the secret.

Security-relevant design points (each has its own test)

SSRF prevention: BrokerRequestDescriptor.host is compared against
the credential's OWN declared host before anything is dispatched. A
broker that trusted the declaration alone would let an agent say "use
github-token, but send it to evil.example.com" — this denies with
host-not-allowed and dispatches nothing.
Traversal: /v1/messages/../../admin normalises to /admin (Node's
path.posix.normalize clamps .. at the root) before it is ever
compared against a declared prefix — the exact case named in the
scoping prompt (/api/../admin).
Boundary: prefix matching requires a / boundary, so /v1 does not
match /v1extra — the other classic hole a bare startsWith opens.
once-grant ordering: all non-mutating checks (credential, grant,
host, method, path) run BEFORE a once grant is consumed. Consuming
first would let a request that was always going to be refused burn the
caller's single-use permission for nothing — pinned by a test that
sends a wrong-host request on a once grant, then confirms the
correctly-addressed retry still succeeds.
once-grant race: the atomic markGrantConsumed call is the LAST
gate before dispatch; its own false return (lost a race to a
concurrent use of the same grant) is itself a fail-closed denial
(grant-consumed-concurrently) — pinned with a store wrapper that
simulates a concurrent winner.
Header injection collision: a caller-supplied Authorization
header cannot override or discover the broker's own injected value —
the broker's header always wins.
query-param encoding: both the key and the secret are
encodeURIComponent-ed — pinned with a secret containing & and =.
No secret leakage: BrokerDenialError's message never contains the
secret or the raw underlying store error; audit events carry only the
credential's fingerprint. Both asserted directly (JSON.stringify
the audit event / response and check the secret string is absent).

What's still out of scope (later phases / follow-up)

No wiring into an agent-callable tool. The scoping prompt's binding
surface separation forbids touching agentBuilder.ts or existing skill
routes — CredentialBroker is a fully-tested, standalone library
component. Exposing it as an MCP/skill tool the agent can actually call
is necessarily a follow-up (likely alongside phase 3's keychain-asks,
or its own issue).
Not wired into middleware/src/index.ts's composition root — still no
route, no live call site, same as phase 1.

Blast radius

New files only (6 files, all under src/credentials/ or test/).
Nothing existing was touched.
No changes to src/services/skill*, agentBuilder.ts,
src/routes/admin.ts, or any existing route.
No new migration.

Tests

59 tests total in this PR's own files, all against
InMemoryCredentialStore — no external dependency, no Postgres needed
for this phase.

test/credentialRequestMatching.test.ts — 22 tests.
test/credentialBrokerMetrics.test.ts — 9 tests.
test/credentialBroker.test.ts — 28 tests (allow paths for all 4
injection schemes, every deny reason, the SSRF host check, traversal,
the once-grant ordering + race, secret-leakage checks, audit + metrics).

Full verification run on top of #769's latest commit:

npm run build (full ordered workspace build): clean.
npm run lint: clean.
npm run typecheck (src): clean.
npm run typecheck:test (test/scripts tree ratchet, issue npm run typecheck never typechecks middleware/test/ #573):
"406 known error(s), no regressions" — same baseline as feat(#578): credential keychain data model + store (phase 1/4) #769.
npm run test (the FULL middleware suite, ~7000 tests): 7000 passed,
0 failed, 12 skipped (Postgres suites, no test DB env var set locally).

Mutation-check evidence

#	Mutation	Result
1	`matchPath`: dropped the segment-boundary requirement (bare `startsWith`)	Caught by "does NOT match a sibling path that merely shares a string prefix".
2	`normalizePathForMatch`: skipped `path.posix.normalize`	Caught by 4 tests (3 direct traversal tests + the broker-level traversal-denial test).
3	`broker.ts`: removed the `host-not-allowed` check entirely	Caught by 4 tests (the SSRF test, the once-grant-ordering test, and both audit/metrics tests whose expectations depend on the denial happening).
4	`broker.ts`: moved the `once`-grant consumption BEFORE the host/method/path checks	Caught by exactly the test written to pin this ordering — no other test noticed.
5	`broker.ts`: dropped `encodeURIComponent` on the query-param injection	Caught by the reserved-character query-param test (added specifically because the original happy-path test used a secret with no special characters and would NOT have caught this).
6	`broker.ts`: removed the `recordBrokerOutcome('deny', reason)` call inside `deny()`	Caught by "records metrics for both allow and deny outcomes".

All six mutations reverted; final state (this PR) is the code shown in
the diff, full suite re-verified green afterward.

Open questions for Marcel

Same broker-declaration-schema caveat as feat(#578): credential keychain data model + store (phase 1/4) #769: CredentialInjectionScheme
(bearer | header | basic-password | query-param) is a guess at what's
needed. basic-password currently expects the credential's stored secret
to already be the full user:pass string (documented in the test) rather
than having a separate username field — flagging in case that's not the
intended shape.
Tool/skill wiring for the agent to actually call the broker is
deliberately NOT in this PR (see "What's still out of scope" above) —
confirming that's expected, not an oversight.

^{Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.}

Data model and durable store for the credential keychain (#578), the credential-side counterpart to Privacy Shield: encrypted, fingerprinted credentials owned by a principal, and grants (audience scope, once vs standing, purpose, expiry, revocation) that let a principal use one. No route, no tool yet — phase 1 is data model + storage only, per the phase cut in docs/plans/phase4a-578-keychain-prompt-2026-08-20.md. - packages/harness-channel-sdk/src/credentials.ts (NEW, additive): Credential / CredentialGrant / CredentialStore types, InMemoryCredentialStore, isGrantActive, validateNewGrantInput, fingerprintSecret. Barrel export appended to the END of index.ts to avoid merge conflicts with the parallel #577 session. - src/credentials/crypto.ts: AES-256-GCM seal/unseal, reusing fileVault's resolveMasterKey under a DIFFERENT env var (CREDENTIAL_KEYCHAIN_KEY) and dev-key file — separate trust domain from the provider-secret vault, sharing only the key-resolution code. - src/credentials/postgresCredentialStore.ts: durable CredentialStore, built the same way as PostgresGrantStore / PostgresAttachmentBindingStore (does not own the pool, throws rather than swallows a failure). - src/credentials/credentialStoreFactory.ts: explicit Postgres-vs-in-memory choice, so the vault no-pool case is a stated decision, not an implicit fallback. - migrations/0040_credentials.sql: credentials + credential_grants tables. 0038 is reserved (#746); 0039 is turn_receipts (#757). Why a dedicated store instead of a second GrantStore: a credential grant needs expiry/purpose/once-vs-standing metadata GrantStore's capability-string model cannot express. The coarse layer (does this principal have any right to reach the broker at all) still reuses the existing GrantStore/resolveCapabilities mechanism in phase 2; this table is the fine layer underneath it. Rationale is written out in credentials.ts's module header and the migration's own comments. Tests: 59 (49 unit + 10 against a real Postgres, skips cleanly with no test DB configured). Mutation-tested: isGrantActive's expiry boundary, principal-canonicalisation in activeGrant (an earlier version of this test passed even with canonicalisation removed, because both principals were built via makePrincipal which already canonicalises — fixed to use a raw, non-canonical Principal literal so the store's own canonicalisation is what's under test), the activeGrant active-filter, and the revokeGrant/markGrantConsumed idempotency guards. Every mutant was caught after the fix; dist was rebuilt between channel-sdk mutation runs.

headerName only made sense for the "header" injection scheme. Phase 2 (the broker) also needs a query-parameter NAME for the "query-param" scheme, which had nowhere to go under the old field. Renamed before phase 2 lands on top of this, rather than working around the gap there: "header" uses it as the header name, "query-param" as the parameter name, "bearer"/"basic-password" ignore it (the whole secret IS the value). No behavioural change for "bearer" (the only scheme phase 1's own tests exercise) — this is a rename, not new logic.

`fsp.readdir(dir).catch(() => [])` inferred the catch handler's return as `never[]`, which the test/tsconfig.json project (checked separately from src/ per #573) flagged as a new, previously-unbaselined error — `npm run typecheck` (src-only) never saw it, only `npm run typecheck:test` does, and that is what CI's "Typecheck (test + scripts trees, ratchet)" step runs. Annotated the handler's return type explicitly. CI failed on this before the Test steps even ran (they were skipped, not green) — verified locally with `npm run typecheck:test`, now reporting "406 known error(s), no regressions".

The broker: an agent names a `service` credential and describes a request (host, method, path). The broker decides whether the calling principal, right now, may use that credential for exactly that request, and if so decrypts the secret and stamps it onto the outbound call itself. The caller receives only the response, never the secret, on either the success or the failure path. - src/credentials/requestMatching.ts: normalizeHost/Method, normalizePathForMatch (traversal-safe via path.posix.normalize, which clamps `..` at the root), matchPath (boundary-safe prefix matching, not a bare startsWith). - src/credentials/brokerMetrics.ts: counters built the same shape as securityScreenMetrics.ts (#749) per the scoping prompt's instruction to reuse that pattern — count every outcome, consecutive-denial streak alert. - src/credentials/broker.ts: CredentialBroker. Fail-closed on every check (unknown/revoked credential, wrong kind, no active grant, host/method/path mismatch, malformed declaration, store outage). Every denial is counted and (when onAudit is wired) audited via BrokerAuditEvent — fingerprint only, never the secret. Security-relevant design points, each with its own test: - BrokerRequestDescriptor.host is compared against the credential's OWN declared host before dispatch — the SSRF-prevention check: an agent naming the right credential but the wrong destination denies with host-not-allowed rather than exfiltrating to an attacker host. - Path prefix matching normalises both sides and requires a segment boundary, closing both the traversal hole (/api/../admin -> /admin) and the naive-startsWith hole (/v1 matching /v1extra). - All non-mutating checks run BEFORE a `once` grant is consumed, so a request that was always going to be refused never burns the caller's single-use permission. The atomic markGrantConsumed call is the last gate before dispatch; its own false-return (lost a race to a concurrent use of the same grant) is a fail-closed denial (grant-consumed-concurrently), covered by a test that simulates the race via a store wrapper. - A caller-supplied header cannot override or discover the injected Authorization/header/query-param value. - query-param injection URL-encodes both the key and the secret. Tests: 87 (59 unit covering requestMatching/brokerMetrics/broker in isolation and via InMemoryCredentialStore, no external dependency). Mutation-tested: matchPath's boundary check, normalizePathForMatch's traversal clamp, the host-not-allowed check (SSRF guard), the check ordering that protects a once grant from being burned by a host/path-rejected request, query-param URL-encoding, and the deny-path metrics counter. Every mutant was caught; all reverted.

…#776) * feat(#576): durable per-scope sandbox — P1 interface + Docker backend Introduces @omadia/sandbox (middleware/packages/harness-sandbox): the narrow Sandbox contract from issue #576 (qm competitive analysis) — provision/run/ read/write/list/teardown, optional capabilities (process-sessions/backup/ blob-staging) behind type guards rather than interface fields, and an AgentComputerProfile declaring persistence/egress/process-session posture. v1 backend is plain local Docker (DockerSandboxBackend), built on the same injectable-spawn pattern as src/plugins/builder/buildSandbox.ts's executeBuild seam (see dockerExec.ts's execDocker injection point) so the full backend logic is testable with zero real Docker. Wiring, not just declaration: - profile.egress === false becomes docker run --network none. Proven at two levels: a stub-level argv assertion (always runs) and, behind the opt-in SANDBOX_DOCKER_TEST=1 gate, a real container attempting an outbound wget and observing it fail — not just that the flag was passed. - read/write/list are traversal-hardened against a fixed sandbox root (pathGuard.ts's clampSandboxPathPosix), same discipline as the #772 broker and zipExtractor.ts's zip-slip guard: absolute paths, NUL bytes, and any ../ resolution outside the root are rejected before a single docker exec is issued (asserted directly — traversal tests check the stub recorded zero calls). - Container naming is a deterministic function of the scope key (sha256(scopeKey)[0:24]), so provision() re-attaches to an already-running container across backend-instance restarts without needing a DB-backed registry yet — that scope-durability bookkeeping (last-used timestamps for a reaper, RO-layer content-hash tracking, multi-backend routing) is P3's job, not a blocker for this backend working correctly today. No orchestrator touch (by design — P1 scope per the phase cut). No new runtime dependencies: the backend shells out to the docker CLI via node:child_process, mirroring dev-runner-shim's dockerd.ts and buildSandbox.ts's Node-builtins-only constraint. Tests: middleware/test/sandbox/{pathGuard,agentComputerProfile, dockerSandbox}.test.ts. 30 stub-tier tests (always run, no Docker required) + 2 real-Docker tests gated on SANDBOX_DOCKER_TEST=1 (both verified green locally against an actual daemon, including the egress block). Root npm test / typecheck / build / lint all green with this package included in the workspace chain. Mutation-checked: inverting the egress condition in dockerSandbox.ts and disabling the escape check in pathGuard.ts (with a full package rebuild between runs) both broke the corresponding tests; reverted and confirmed green again before committing. * fix(#576): reword dockerExec.ts comment to stop tripping the #470 core-decoupling ratchet The comment referencing 'dev-runner-shim' by name matched the ratchet's 'dev-runner' literal pattern (scripts/check-core-decoupling.mjs), which counts references to the Dev Platform being extracted in epic #470 — an unrelated coincidence (dev-runner-shim is a real, unrelated package; the ratchet's pattern list is deliberately broad-literal). Reworded to drop the package name while keeping the same intent (Node-builtins-only constraint on this spawn seam). Confirmed the ratchet passes locally after this change: 'Dev Platform references held at 3296' (baseline unchanged, not lowered — this was never a real Dev Platform reference, just a name collision).

…icy gate (#777) * feat(#576): durable per-scope sandbox — P1 interface + Docker backend Introduces @omadia/sandbox (middleware/packages/harness-sandbox): the narrow Sandbox contract from issue #576 (qm competitive analysis) — provision/run/ read/write/list/teardown, optional capabilities (process-sessions/backup/ blob-staging) behind type guards rather than interface fields, and an AgentComputerProfile declaring persistence/egress/process-session posture. v1 backend is plain local Docker (DockerSandboxBackend), built on the same injectable-spawn pattern as src/plugins/builder/buildSandbox.ts's executeBuild seam (see dockerExec.ts's execDocker injection point) so the full backend logic is testable with zero real Docker. Wiring, not just declaration: - profile.egress === false becomes docker run --network none. Proven at two levels: a stub-level argv assertion (always runs) and, behind the opt-in SANDBOX_DOCKER_TEST=1 gate, a real container attempting an outbound wget and observing it fail — not just that the flag was passed. - read/write/list are traversal-hardened against a fixed sandbox root (pathGuard.ts's clampSandboxPathPosix), same discipline as the #772 broker and zipExtractor.ts's zip-slip guard: absolute paths, NUL bytes, and any ../ resolution outside the root are rejected before a single docker exec is issued (asserted directly — traversal tests check the stub recorded zero calls). - Container naming is a deterministic function of the scope key (sha256(scopeKey)[0:24]), so provision() re-attaches to an already-running container across backend-instance restarts without needing a DB-backed registry yet — that scope-durability bookkeeping (last-used timestamps for a reaper, RO-layer content-hash tracking, multi-backend routing) is P3's job, not a blocker for this backend working correctly today. No orchestrator touch (by design — P1 scope per the phase cut). No new runtime dependencies: the backend shells out to the docker CLI via node:child_process, mirroring dev-runner-shim's dockerd.ts and buildSandbox.ts's Node-builtins-only constraint. Tests: middleware/test/sandbox/{pathGuard,agentComputerProfile, dockerSandbox}.test.ts. 30 stub-tier tests (always run, no Docker required) + 2 real-Docker tests gated on SANDBOX_DOCKER_TEST=1 (both verified green locally against an actual daemon, including the egress block). Root npm test / typecheck / build / lint all green with this package included in the workspace chain. Mutation-checked: inverting the egress condition in dockerSandbox.ts and disabling the escape check in pathGuard.ts (with a full package rebuild between runs) both broke the corresponding tests; reverted and confirmed green again before committing. * feat(#576): durable per-scope sandbox — P2 execute tool + command-policy gate Adds the `execute` native tool (packages/harness-orchestrator/src/tools/ executeTool.ts), off by default behind operator config `sandbox_execute_enabled` (honest-inert, same convention as the #575 audience floor and #580 command policy — sharper here because this tool's entire job is running arbitrary commands). ## Security boundary — belt and braces, not a new mechanism orchestrator.ts's dispatchTool already runs every tool call through guardToolCommands (#580) at the ONE existing choke point, keyed on a top-level `command` field — execute's input schema uses exactly that key, so it is automatically gated by the EXISTING seam whenever a deployment installs a commandPolicy provider on the turn context. No deployment does that today (the seam is honest-inert until an operator config UI ships). Relying solely on that opt-in seam would mean execute ships fully open in every deployment that hasn't separately configured a policy — the #748 lesson (fail-open + no evidence = an invisible outage) applied to a much sharper edge than a security screener. So the handler ALSO runs its own command-policy check before ever touching a sandbox: resolveCommandPolicy defaults to defaultCommandPolicy() (the DEFAULT_ORG_FLOOR: recursive rm, force-push, destructive SQL, fork bombs, pipe-to-shell), and a throwing resolver is FAIL-CLOSED — refused, never run. require_approval is surfaced as an explicit refusal naming why, never silently escalated. No parallel policy mechanism: both checks call the same decideCommand/defaultCommandPolicy pure primitives from @omadia/channel-sdk. ## Count, don't just log (#749/#750 pattern) New commandPolicyMetrics.ts (mirrors securityScreenMetrics.ts): in-memory counters for allowed/denied/require_approval/truncated/resolve_failed, plus a per-rule-id tally. Wired into BOTH guardToolCommands (every existing #580 branch now records) and executeTool's own belt-and-braces check, so a broken policy resolver shows up as a resolveFailed streak instead of silence. ## Sandbox wiring execute resolves the calling turn's scope via ScopeId (parseSessionScope/formatSessionScope from @omadia/channel-sdk), falling back to a turn-unique key for an unscoped turn rather than a shared literal (the #445 lesson). Provisions/reuses that scope's sandbox via the injected SandboxBackend (DockerSandboxBackend in plugin.ts's wiring) and runs the command there. Registered in harness-orchestrator's plugin.ts — the SAME composition seam the existing ProcessMemory tools use (OB-76) — not src/index.ts or agentBuilder.ts, neither of which this PR touches. No writeCapabilities annotation: that contract is {dataClass, operation} for structured-write idempotency dedupe; execute's effects are arbitrary and untyped, and re-running the "same" shell command is not safely deduplicable the way replaying a structured write is. Deliberately absent, documented at the call site. ## Tests test/sandbox/executeTool.test.ts (17 tests) — every deny/require_approval/ truncated/resolve_failed path proven to never call SandboxBackend.provision (the security-boundary property), and the permitted path proven to reach provision with the correct scope key + run options. test/ commandPolicyMetrics.test.ts (9 tests) — counter module plus guardToolCommands wiring. All new + existing #580 tests green. ## Mutation-check evidence (dist rebuild between runs, per the phase-4b ## prompt's requirement now that @omadia/orchestrator imports @omadia/sandbox) Short-circuited the deny-decision branch in executeTool.ts (`if (false && decision.decision === 'deny')`), rebuilt @omadia/sandbox and @omadia/orchestrator, reran the suite: both the rm -rf and force-push denial tests failed as expected — the stub sandbox returned exit 0 for a command that should never have reached it. Reverted, rebuilt again, confirmed green. Full workspace build/typecheck/lint/test green after merging latest origin/main (7115 tests, 0 fail, 12 pre-existing skips). * fix(#576): reword dockerExec.ts comment to stop tripping the #470 core-decoupling ratchet The comment referencing 'dev-runner-shim' by name matched the ratchet's 'dev-runner' literal pattern (scripts/check-core-decoupling.mjs), which counts references to the Dev Platform being extracted in epic #470 — an unrelated coincidence (dev-runner-shim is a real, unrelated package; the ratchet's pattern list is deliberately broad-literal). Reworded to drop the package name while keeping the same intent (Node-builtins-only constraint on this spawn seam). Confirmed the ratchet passes locally after this change: 'Dev Platform references held at 3296' (baseline unchanged, not lowered — this was never a real Dev Platform reference, just a name collision). * fix(#576): sync package-lock.json peerDependencies entry for @omadia/orchestrator -> @omadia/sandbox npm install after the P1 merge regenerated this — the peerDependencies entry added to harness-orchestrator/package.json in the original P2 commit had not been reflected into package-lock.json's own copy of that block.

…r content hash + reaper (#779) * feat(#576): durable per-scope sandbox — P1 interface + Docker backend Introduces @omadia/sandbox (middleware/packages/harness-sandbox): the narrow Sandbox contract from issue #576 (qm competitive analysis) — provision/run/ read/write/list/teardown, optional capabilities (process-sessions/backup/ blob-staging) behind type guards rather than interface fields, and an AgentComputerProfile declaring persistence/egress/process-session posture. v1 backend is plain local Docker (DockerSandboxBackend), built on the same injectable-spawn pattern as src/plugins/builder/buildSandbox.ts's executeBuild seam (see dockerExec.ts's execDocker injection point) so the full backend logic is testable with zero real Docker. Wiring, not just declaration: - profile.egress === false becomes docker run --network none. Proven at two levels: a stub-level argv assertion (always runs) and, behind the opt-in SANDBOX_DOCKER_TEST=1 gate, a real container attempting an outbound wget and observing it fail — not just that the flag was passed. - read/write/list are traversal-hardened against a fixed sandbox root (pathGuard.ts's clampSandboxPathPosix), same discipline as the #772 broker and zipExtractor.ts's zip-slip guard: absolute paths, NUL bytes, and any ../ resolution outside the root are rejected before a single docker exec is issued (asserted directly — traversal tests check the stub recorded zero calls). - Container naming is a deterministic function of the scope key (sha256(scopeKey)[0:24]), so provision() re-attaches to an already-running container across backend-instance restarts without needing a DB-backed registry yet — that scope-durability bookkeeping (last-used timestamps for a reaper, RO-layer content-hash tracking, multi-backend routing) is P3's job, not a blocker for this backend working correctly today. No orchestrator touch (by design — P1 scope per the phase cut). No new runtime dependencies: the backend shells out to the docker CLI via node:child_process, mirroring dev-runner-shim's dockerd.ts and buildSandbox.ts's Node-builtins-only constraint. Tests: middleware/test/sandbox/{pathGuard,agentComputerProfile, dockerSandbox}.test.ts. 30 stub-tier tests (always run, no Docker required) + 2 real-Docker tests gated on SANDBOX_DOCKER_TEST=1 (both verified green locally against an actual daemon, including the egress block). Root npm test / typecheck / build / lint all green with this package included in the workspace chain. Mutation-checked: inverting the egress condition in dockerSandbox.ts and disabling the escape check in pathGuard.ts (with a full package rebuild between runs) both broke the corresponding tests; reverted and confirmed green again before committing. * feat(#576): durable per-scope sandbox — P2 execute tool + command-policy gate Adds the `execute` native tool (packages/harness-orchestrator/src/tools/ executeTool.ts), off by default behind operator config `sandbox_execute_enabled` (honest-inert, same convention as the #575 audience floor and #580 command policy — sharper here because this tool's entire job is running arbitrary commands). ## Security boundary — belt and braces, not a new mechanism orchestrator.ts's dispatchTool already runs every tool call through guardToolCommands (#580) at the ONE existing choke point, keyed on a top-level `command` field — execute's input schema uses exactly that key, so it is automatically gated by the EXISTING seam whenever a deployment installs a commandPolicy provider on the turn context. No deployment does that today (the seam is honest-inert until an operator config UI ships). Relying solely on that opt-in seam would mean execute ships fully open in every deployment that hasn't separately configured a policy — the #748 lesson (fail-open + no evidence = an invisible outage) applied to a much sharper edge than a security screener. So the handler ALSO runs its own command-policy check before ever touching a sandbox: resolveCommandPolicy defaults to defaultCommandPolicy() (the DEFAULT_ORG_FLOOR: recursive rm, force-push, destructive SQL, fork bombs, pipe-to-shell), and a throwing resolver is FAIL-CLOSED — refused, never run. require_approval is surfaced as an explicit refusal naming why, never silently escalated. No parallel policy mechanism: both checks call the same decideCommand/defaultCommandPolicy pure primitives from @omadia/channel-sdk. ## Count, don't just log (#749/#750 pattern) New commandPolicyMetrics.ts (mirrors securityScreenMetrics.ts): in-memory counters for allowed/denied/require_approval/truncated/resolve_failed, plus a per-rule-id tally. Wired into BOTH guardToolCommands (every existing #580 branch now records) and executeTool's own belt-and-braces check, so a broken policy resolver shows up as a resolveFailed streak instead of silence. ## Sandbox wiring execute resolves the calling turn's scope via ScopeId (parseSessionScope/formatSessionScope from @omadia/channel-sdk), falling back to a turn-unique key for an unscoped turn rather than a shared literal (the #445 lesson). Provisions/reuses that scope's sandbox via the injected SandboxBackend (DockerSandboxBackend in plugin.ts's wiring) and runs the command there. Registered in harness-orchestrator's plugin.ts — the SAME composition seam the existing ProcessMemory tools use (OB-76) — not src/index.ts or agentBuilder.ts, neither of which this PR touches. No writeCapabilities annotation: that contract is {dataClass, operation} for structured-write idempotency dedupe; execute's effects are arbitrary and untyped, and re-running the "same" shell command is not safely deduplicable the way replaying a structured write is. Deliberately absent, documented at the call site. ## Tests test/sandbox/executeTool.test.ts (17 tests) — every deny/require_approval/ truncated/resolve_failed path proven to never call SandboxBackend.provision (the security-boundary property), and the permitted path proven to reach provision with the correct scope key + run options. test/ commandPolicyMetrics.test.ts (9 tests) — counter module plus guardToolCommands wiring. All new + existing #580 tests green. ## Mutation-check evidence (dist rebuild between runs, per the phase-4b ## prompt's requirement now that @omadia/orchestrator imports @omadia/sandbox) Short-circuited the deny-decision branch in executeTool.ts (`if (false && decision.decision === 'deny')`), rebuilt @omadia/sandbox and @omadia/orchestrator, reran the suite: both the rm -rf and force-push denial tests failed as expected — the stub sandbox returned exit 0 for a command that should never have reached it. Reverted, rebuilt again, confirmed green. Full workspace build/typecheck/lint/test green after merging latest origin/main (7115 tests, 0 fail, 12 pre-existing skips). * feat(#576): durable per-scope sandbox — P3 scope durability + RO-layer content hash + reaper Migration 0044_sandbox_registry.sql (chosen with margin: 0040-0043 were occupied by concurrent credential-work PRs at authoring time, including an observed numbering collision at 0040 across two of them that has since been resolved upstream — noted in the migration's own header for the record, not something this PR touches). ## SandboxRegistry — the durable bookkeeping Docker doesn't give for free New SandboxRegistry interface (get/upsert/touch/delete/listAll) in @omadia/sandbox: InMemorySandboxRegistry (tests, and any deployment that hasn't wired a durable store) and PostgresSandboxRegistry (migration 0044's sandbox_registry table). DockerSandboxBackend gained an OPTIONAL registry constructor option — omitted (the default), the backend behaves byte-identical to what P1/P2 shipped (deterministic container naming, zero registry calls); provided, provision() records (scope, container name, profile, lastUsedAt) and re-attaches via the registry's STORED sandboxRef rather than recomputing the deterministic name, which is the seam a future non-deterministic backend needs. Regression-tested directly (dockerSandboxRegistry.test.ts's first suite asserts the no-registry path is unchanged). ## Reaper — orphaned (idle, non-persistent) sandbox cleanup reapOrphanedSandboxes takes "now" as a REQUIRED external parameter, never derived from the registry entries themselves — the #709/#710 clock-race lesson (an idle check must anchor to a clock independent of the row being checked). profile.persistent === true entries are never reaped regardless of idle time. A teardown failure leaves the registry row intact (for retry) and is reported in failedScopeKeys rather than silently dropped, and does not abort the sweep for the rest. Not wired to a scheduler in this PR — the function is a complete, directly callable, fully tested capability (same shape as DockerSandbox.teardown() itself, which also isn't auto-invoked by anything); hooking it to a cron/ routine is a scheduling-system integration, out of #576's substrate scope. ## RO-layer content-hash materialization computeContentHash (deterministic, order-independent, NUL-separated path/content encoding so shifted-boundary inputs can't collide) + syncReadOnlyLayer, which calls Sandbox.write only when the computed hash differs from the caller-supplied previousHash. #576 is deliberately the mechanism here, not the consumer: what actually constitutes a scope's RO layer (org files, skills) is the issue's OWN framing of what "builds on" this sandbox, not something #576 decides — inventing a content source would be scope creep into a separate concept. The primitive is fully exercised against a stub Sandbox (contentHash.test.ts proves the skip-when-unchanged behavior directly against the write call count). ## Tests - contentHash.test.ts (8), reaper.test.ts (6), dockerSandboxRegistry.test.ts (4) — all stub/pure, no Docker or Postgres needed, always run in npm test. - postgresSandboxRegistry.pg.test.ts (5) — gated on GRAPH_PG_TEST_URL/ MEMORY_PG_TEST_URL/DATABASE_URL, same probePgTest convention as postgresCredentialStore.pg.test.ts. Verified locally against a real postgres:16-alpine container (docker run, migration 0044 applied verbatim via psql, all 5 tests green) — not just trusted to work under CI's pg service. ## Mutation-check evidence (dist rebuild between runs) Removed the persistent-skip guard ("if (entry.profile.persistent) continue;") in reaper.ts, rebuilt @omadia/sandbox: the 'never reaps a persistent sandbox' test failed as expected (a persistent scope was reaped). Reverted, rebuilt again, confirmed green. (P1's egress/traversal mutations and P2's deny-bypass mutation were separately re-verified in their own PRs; this round targets the one new security-relevant branch P3 adds.) Full workspace build/typecheck/lint/test green after merging latest origin/main (7223 tests, 0 fail, 12 pre-existing skips). * fix(#576): reword dockerExec.ts comment to stop tripping the #470 core-decoupling ratchet The comment referencing 'dev-runner-shim' by name matched the ratchet's 'dev-runner' literal pattern (scripts/check-core-decoupling.mjs), which counts references to the Dev Platform being extracted in epic #470 — an unrelated coincidence (dev-runner-shim is a real, unrelated package; the ratchet's pattern list is deliberately broad-literal). Reworded to drop the package name while keeping the same intent (Node-builtins-only constraint on this spawn seam). Confirmed the ratchet passes locally after this change: 'Dev Platform references held at 3296' (baseline unchanged, not lowered — this was never a real Dev Platform reference, just a name collision). * fix(#576): sync package-lock.json peerDependencies entry for @omadia/orchestrator -> @omadia/sandbox npm install after the P1 merge regenerated this — the peerDependencies entry added to harness-orchestrator/package.json in the original P2 commit had not been reflected into package-lock.json's own copy of that block.

Weegy added 4 commits August 20, 2026 14:13

Weegy mentioned this pull request Aug 20, 2026

feat(#578): keychain-asks — request, owner-approval, grant (phase 3/4) #774

Merged

Weegy changed the base branch from feat/578-keychain to main August 20, 2026 13:32

Merge branch 'main' into feat/578-keychain-p2-broker

c10e4fc

Weegy mentioned this pull request Aug 20, 2026

feat(#576): durable per-scope sandbox — P1 interface + Docker backend #776

Merged

Weegy merged commit 4e09853 into main Aug 20, 2026
9 checks passed

Weegy deleted the feat/578-keychain-p2-broker branch August 20, 2026 13:40

Navigation Menu

Uh oh!

feat(#578): credential broker — the egress-stamping layer (phase 2/4) - #772

feat(#578): credential broker — the egress-stamping layer (phase 2/4)#772
Weegy merged 5 commits into
mainfrom
feat/578-keychain-p2-broker

Weegy commented Aug 20, 2026 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

Weegy commented Aug 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

What this delivers

Security-relevant design points (each has its own test)

What's still out of scope (later phases / follow-up)

Blast radius

Tests

Mutation-check evidence

Open questions for Marcel

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Weegy commented Aug 20, 2026 •

edited

Loading