Skip to content

docs(security): claims-vs-roadmap table for sandbox controls#205

Open
kevinnft wants to merge 2 commits into
alphacrack:mainfrom
kevinnft:docs/security-claims-table
Open

docs(security): claims-vs-roadmap table for sandbox controls#205
kevinnft wants to merge 2 commits into
alphacrack:mainfrom
kevinnft:docs/security-claims-table

Conversation

@kevinnft

Copy link
Copy Markdown

Summary

Closes #168. Closes #65.

Restructures the sandbox portion of SECURITY.md into a claims-vs-roadmap table so adopters can see what is enforced today (with code pointers), planned for v0.8 (#64), or a known limitation.

Changes

  • Remove the incorrect claim that network egress is already allowlisted
  • Table covering caps, non-root, RO mount, verify credentials, docker socket, egress, proxy, disk quotas, red-team harness, argv key exposure
  • Align docs/security.md (add missing egress tradeoff + link to table)

Out of scope

  • No code changes to sandbox.py / agent.py / verify.py

Test plan

  • Docs-only; pytest still largely green (1 pre-existing CLI path-wrap flake unrelated)

Replace the overstated "egress is allowlisted" wording with an honest
table of enforced-today vs planned (v0.8 / alphacrack#64) vs known limitations,
each with code pointers. Align docs/security.md with the table.

Closes alphacrack#168
Closes alphacrack#65

Signed-off-by: NossXBT <kevinnft@users.noreply.github.com>

@alphacrack alphacrack left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @kevinnft — the hard part of this issue is done, and done carefully: you read the source and refused to write down claims you couldn't find a line for. Every "enforced today" row verifies against the code. Three blockers, all mechanical.

First, the things you got right that are easy to miss:

  • The non-root row cites images/base/Dockerfile, not a --user flag. That's the careful answer — Sandbox.user defaults to None and neither agent.py nor verify.py passes it, so citing a --user flag would have been exactly the phantom-control claim #65 exists to prevent.
  • You quietly dropped "your network" from "can't reach your host, your network, or your other runs." Correct — with --network bridge the container can reach the host LAN. That's the #65 bug class caught in a second place nobody had flagged.
  • You improved on #168's own draft by noting verify.py also gates the docker socket; the issue only mentioned agent.py.

B1 — the docs build breaks, and CI won't tell you. docs/security.md:21 links ../SECURITY.md, which escapes docs_dir:

WARNING - Doc file 'security.md' contains a link '../SECURITY.md', but the target is not found
Aborted with 1 warnings in strict mode!

mkdocs build --strict passes on main and fails on this branch. The catch: .github/workflows/docs.yml triggers only on push to main, so there's no PR-time signal — this would fail the Pages deploy after merge. Use the absolute form this repo already uses at docs/security.md:43 and docs/index.md:62: https://github.com/alphacrack/readme2demo/blob/main/SECURITY.md.

B2 — the ## Scope section was deleted. 12 lines defining what's in scope for a report (sandbox escapes, isolation bypasses, credential leakage, grounding bypasses) are gone from the head revision, and it isn't mentioned in the PR description. #168 lists this explicitly as out of scope: "leave the reporting/scope/supported-versions sections as-is." It's also the only place telling a reporter what to file, and the grounding-bypass clause is load-bearing here. Please restore it verbatim.

B3 — the hardening rows are unscoped, and the render container isn't covered. The table claims cap-drop / no-new-privileges / PID caps as flat "enforced today" citing sandbox.py start() — but the render stage never goes through Sandbox. render.py:122-129 builds its own docker run with --memory, --cpus, --network and no --cap-drop ALL, no --security-opt no-new-privileges, no --pids-limit; _generate_gif_preview has no limits at all. That container replays the target repo's own commands on camera, so it is an untrusted-execution surface.

This is pre-existing imprecision you inherited from the old prose — but it's blocking because architecture/README.md:64-70 already gets it right (agent container (hardened*), verify container (fresh, hardened*), and pointedly render container with no asterisk). So the new table is currently less precise than an existing doc and contradicts it, which trips two of #168's criteria at once.

Cheapest fix: scope the header to | Claim (agent + verify containers) | and add one honest row for the render container as a known limitation.

Nits: bare #64/#51 refs render as dead text on the MkDocs site (GitHub autolinks them, MkDocs doesn't) — same file already uses full URLs three lines up; the credential bullet lost the specific var names (ANTHROPIC_API_KEY/CLAUDE_CODE_OAUTH_TOKEN, OPENAI_API_KEY/GEMINI_API_KEY via LLM_API_KEY), which an operator scoping a low-limit key actually needs; #52 never got linked (#168 asked to link it); render.py is a third socket-mount site the row omits; and I'd drop the "earlier wording was incorrect and has been removed" parenthetical — git history and #65 already record that, and a security doc narrating its own edits reads oddly a release later.

On closing both issues: Closes #168. Closes #65. is correct — keep it. #65 is fully satisfied (all three tradeoffs reworded and synced; grep -rni allowlist --include="*.md" now returns only the line this PR deletes). #168 is satisfied except the two criteria B3 trips. So: fix B1–B3, then merge and let both close — no need to split them.

Using symbol pointers (sandbox.py start()) instead of file:line is a deviation from #168's literal wording that I'd keep — the issue itself warned line numbers drift, and your header caveat is the right call.

- Absolute SECURITY.md link from docs/security.md (mkdocs strict)
- Restore ## Scope section verbatim
- Scope claims table to agent+verify; document render container gap
- Restore credential env var names; link alphacrack#51/alphacrack#52/alphacrack#64 with full URLs

Signed-off-by: NossXBT <kevinnft@users.noreply.github.com>
@kevinnft

Copy link
Copy Markdown
Author

Thanks for the careful review — B1–B3 (and the nits you called out) are in the latest push:

Closes #168. Closes #65. kept as you suggested. Ready for another look.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants