fix: isolate bounded-query private state from agent mounts - #6758
Conversation
Move broker-only bounded-query state outside agent-visible mounts and reject realpath or symlink overlaps before staging. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 3ece9a45-56aa-4e6e-8e6b-079d0e114651
There was a problem hiding this comment.
Pull request overview
Moves bounded-query private state outside agent-visible mounts and adds isolation checks and cleanup support.
Changes:
- Separates broker-private and agent-ingress roots under
/var/tmp. - Adds realpath-aware overlap validation across sandbox mounts.
- Updates broker control state, cleanup, diagnostics, tests, and documentation.
Show a summary per file
| File | Description |
|---|---|
src/services/optional-services.ts |
Clarifies mandatory ingress mounts. |
src/services/bounded-query-service.ts |
Mounts private control state and ingress-only agent paths. |
src/services/bounded-query-service.test.ts |
Updates service mount expectations. |
src/services/bounded-query-compose.test.ts |
Tests ingress-only Compose wiring. |
src/docker-manager-diagnostics.test.ts |
Uses relocated audit path. |
src/bounded-query/types.ts |
Updates seed-map documentation. |
src/bounded-query/paths.ts |
Defines disjoint private and ingress roots. |
src/bounded-query/paths.test.ts |
Tests the new layout. |
src/bounded-query/mount-policy.ts |
Adds mount-overlap preflight. |
src/bounded-query/mount-policy.test.ts |
Covers overlap and symlink cases. |
src/bounded-query/manager.ts |
Integrates preflight, creation, and cleanup. |
src/bounded-query/manager.test.ts |
Updates lifecycle tests. |
src/artifact-preservation.ts |
Locates audits in the private root. |
docs/bounded-queries.md |
Documents private audit isolation. |
docs/awf-config-spec.md |
Specifies the new filesystem and mount policy. |
containers/bounded-query/Dockerfile |
Creates the broker control mount point. |
containers/bounded-query/broker/server.js |
Writes readiness state to the control mount. |
containers/bounded-query/broker/config.js |
Defines the private control path. |
CLAUDE.md |
Updates bounded-query architecture guidance. |
Review details
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comments suppressed due to low confidence (1)
src/bounded-query/manager.test.ts:236
- This
--keep-containerstest deliberately preserves both roots, but itsfinallyblock removes only the private root. The ingress socket/skill tree remains in/var/tmpafter the test; clean upcleanupPaths.ingressRoottoo.
fs.rmSync(cleanupPaths.root, { recursive: true, force: true });
- Files reviewed: 19/19 changed files
- Comments generated: 3
- Review effort level: Balanced
| releaseSeedPermissions(resolveBoundedQueryPaths(workDir).seedsDir); | ||
| const paths = resolveBoundedQueryPaths(workDir); | ||
| releaseSeedPermissions(paths.seedsDir); | ||
| fs.rmSync(paths.root, { recursive: true, force: true }); |
| // 2 masking mounts (hide the bounded-query root) + 2 socket mounts + 2 skill mounts = 6 | ||
| expect(boundedQueryMounts).toHaveLength(6); | ||
| // Masking mounts are read-only; socket mounts are read-write; skill mounts are read-only | ||
| expect(boundedQueryMounts).toHaveLength(4); |
| const root = path.join(privateBaseDir, `awf-bounded-query-private-${rootIdentity}`); | ||
| const ingressRoot = path.join(privateBaseDir, `awf-bounded-query-ingress-${rootIdentity}`); | ||
| const runDir = path.join(ingressRoot, 'run'); | ||
| const agentDir = path.join(ingressRoot, 'skill'); |
lpcox
changed the title
Update staging, skill, and compose expectations for the disjoint private and ingress roots. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 3ece9a45-56aa-4e6e-8e6b-079d0e114651
|
✅ Copilot review passed with no inline comments. @lpcox Add the |
|
| Metric | Base | PR | Delta |
|---|---|---|---|
| Lines | 99.13% | 98.94% | 📉 -0.19% |
| Statements | 99.04% | 98.86% | 📉 -0.18% |
| Functions | 99.28% | 99.28% | ➡️ +0.00% |
| Branches | 95.87% | 95.53% | 📉 -0.34% |
📁 Per-file Coverage Changes (3 files)
| File | Lines (Before → After) | Statements (Before → After) |
|---|---|---|
src/bounded-query/manager.ts |
100.0% → 88.8% (-11.22%) | 100.0% → 89.3% (-10.72%) |
src/artifact-preservation.ts |
96.2% → 96.2% (+0.04%) | 96.2% → 96.2% (+0.03%) |
src/log-directory-setup.ts |
96.2% → 100.0% (+3.78%) | 96.3% → 100.0% (+3.71%) |
✨ New Files (1 files)
src/bounded-query/mount-policy.ts: 91.5% lines
Coverage comparison generated by scripts/ci/compare-coverage.ts
Exercise rootless repair, cleanup failures, and all mount-policy rejection paths while removing unreachable post-mkdir checks. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 3ece9a45-56aa-4e6e-8e6b-079d0e114651
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Keep rootless cleanup available to teardown and focused tests. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 3ece9a45-56aa-4e6e-8e6b-079d0e114651
|
| Metric | Base | PR | Delta |
|---|---|---|---|
| Lines | 99.13% | 99.17% | 📈 +0.04% |
| Statements | 99.04% | 99.08% | 📈 +0.04% |
| Functions | 99.28% | 99.29% | 📈 +0.01% |
| Branches | 95.87% | 95.85% | 📉 -0.02% |
📁 Per-file Coverage Changes (2 files)
| File | Lines (Before → After) | Statements (Before → After) |
|---|---|---|
src/artifact-preservation.ts |
96.2% → 96.2% (+0.04%) | 96.2% → 96.2% (+0.03%) |
src/log-directory-setup.ts |
96.2% → 100.0% (+3.78%) | 96.3% → 100.0% (+3.71%) |
✨ New Files (1 files)
src/bounded-query/mount-policy.ts: 100.0% lines
Coverage comparison generated by scripts/ci/compare-coverage.ts
|
📰 DEVELOPING STORY: Smoke Docker Sbx reports failed. Our correspondents are investigating the incident... |
|
📡 Smoke OTel Tracing completed. All tracing scenarios validated. ✅ |
|
✅ Smoke Claude passed |
|
🔌 Smoke Services — All services reachable! ✅ |
|
✨ The prophecy is fulfilled... Smoke Codex has completed its mystical journey. The stars align. 🌟 |
|
📰 VERDICT: Smoke Copilot has concluded. All systems operational. This is a developing story. 🎤 |
|
✅ Smoke Copilot BYOK completed. Copilot BYOK mode operational. 🔓 |
|
Chroot tests passed! Smoke Chroot - All security and functionality tests succeeded. |
|
✅ Security Guard completed successfully! Security review complete for PR #6758. No security issues found. Changes strengthen security by isolating bounded-query broker-private state from agent mounts, implementing proper directory permissions (0o700), and preventing agent access to broker readiness markers. |
|
🛡️ Smoke Copilot Network Isolation confirmed the egress allowlist is enforced. ✅ |
|
✅ Smoke Copilot BYOK AOAI (api-key) completed. Copilot AOAI BYOK (api-key) mode operational. 🔓 |
|
✅ Smoke Copilot BYOK AOAI (Entra) completed. Copilot AOAI BYOK (Entra) mode operational. 🔓 |
|
✅ Smoke Gemini completed. All facets verified. 💎 |
|
✅ Contribution Check completed successfully! Contribution check complete: no guideline issues found in PR #6758. The PR includes focused tests, documentation updates, clear summary/validation, and files are placed in the expected directories. |
|
Smoke Test: Copilot Engine — @lpcox
Overall: PASS
|
|
@lpcox Smoke test results:
Running in direct BYOK mode (AWF_AUTH_TYPE=github-oidc + AWF_AUTH_AZURE_* + COPILOT_PROVIDER_BASE_URL) via api-proxy → Azure OpenAI (Foundry, o4-mini-aw) authenticated via Microsoft Entra Overall status: PASS
|
github-actions
Bot
added
the
smoke-copilot-network-isolation
|
EGRESS_RESULT allow=pass deny=pass ✅ Allowed domain (api.github.com) reachable: HTTP 200 Overall: PASS Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
To allow these domains, add them to the network:
allowed:
- defaults
- "example.com"See Network Configuration for more information.
|
|
Smoke test summary for
Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
To allow these domains, add them to the network:
allowed:
- defaults
- "registry.npmjs.org"See Network Configuration for more information.
|
Smoke Test Results: Copilot BYOK (Direct) Mode✅ GitHub MCP - MCP connectivity verified Mode: Direct BYOK (COPILOT_PROVIDER_API_KEY forwarded to api-proxy sidecar)
|
|
Smoke test: GitHub Actions services connectivity
Overall: FAIL — host.docker.internal could not be resolved from the AWF sandbox.
|
Smoke Test: Claude Engine Validation
Overall result: PASS
|
Smoke Test: API Proxy OpenTelemetry Tracing
Overall: all scenarios pass or are expected-pending; no issues found.
|
Chroot Version Comparison Results
Overall: FAILED — Node.js version mismatch between host and chroot environments (v24.18.0 vs v22.23.1). Since not all tests passed, the
|
Smoke Test Results
Overall status: FAIL
|
|
@lpcox
|
🏗️ Build Test Suite Results
Overall: 8/8 ecosystems passed — PASS Note: Java Maven builds required
|
Summary
/var/tmp--keep-containersdiagnosticsThis is layer 1 of the approved bounded-query multi-sandbox stack. It was developed on PR #6755's head; because #6755 merged and its branch was deleted before this PR was opened, this branch was rebased onto the merged parent and now targets
main. It does not add sbx transport or sbx query execution.Validation
node --checkpassed for bounded-query broker config/server/healthcheckgit diff --checkpassed/tmp, workspace/session-state mounts, valid layouts, Compose ingress-only mounts, DinD translation, and lifecycle cleanup