Skip to content

feat(benchmark): untrusted candidate sandbox policy and fail-closed run verdict (#9264) - #9606

Merged
JSONbored merged 1 commit into
mainfrom
feat/candidate-sandbox-9264
Jul 29, 2026
Merged

feat(benchmark): untrusted candidate sandbox policy and fail-closed run verdict (#9264)#9606
JSONbored merged 1 commit into
mainfrom
feat/candidate-sandbox-9264

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Scope note

#9264 reads as blocked on the CoCo/SNP fleet (#8535/#8536), but its own requirement 2 is explicit that the sandbox properties hold with or without SNP hardware — and those are enforced by the container runtime, which exists today. This PR builds that half in full. What genuinely remains blocked is running under real SNP, which is hardware procurement, not code.

Two facts drive the design

Attestation is not a sandbox. Attestation proves what ran; it constrains nothing about what that code can reach while running. So no property here may be skipped because attestation is present — and there's a test asserting a fully-unisolated run is refused even with genuineAttestation: true.

The network is the whole ballgame. A candidate that can reach the network can simply fetch the realized future — the outcomes it's being asked to predict — and score perfectly having learned nothing. #9259 leak-proofs the data; this leak-proofs the runtime. Both are required: a perfect snapshot handed to a networked process is not a benchmark.

What's here

  • candidateSandboxArgs returns an argv array, never a shell string. A snapshot path is data we don't fully control; as a string it would be a command-injection seam. Asserted with a path containing '; rm -rf /; echo '. Exactly two mounts exist — task read-only, one output file writable — so no host filesystem of ours is reachable.
  • decideCandidateRunVerdict is fail-closed, matching the attested-run path's existing posture. A run that couldn't establish its claimed isolation doesn't score at all — not scored-and-flagged, not scored at a lower tier. A networked agent's score isn't a weaker measurement of the same thing, it's a measurement of something else, and admitting it at any tier would put a fabricated number on the leaderboard. All failures are named at once so an operator fixes the runner in one pass.
  • The tier is read from what was established. attested requires genuine hardware; everything else is reproducible. There is deliberately no path by which "the plumbing supports attestation" becomes an attested label — benchmark: leaderboard artifacts as spec-conformant EvalScoreRecords #9265's emitter and this agree by construction.
  • A non-positive or non-finite limit falls back to the default rather than being honored. Failing open on a resource bound is the one direction that can't be allowed, so memoryMb: 0 tightens nothing instead of removing the cap.
  • describeCandidateLimits generates the documentation from the same constants the runner uses, so the documented limits can't drift from what's actually enforced — asserted by describing a tightened policy and checking the numbers followed.

Tests (7 node:test cases, 100% of branches)

Every claimed control present in the args; the read-only task mount and single writable output; the shell-metacharacter path staying inert as one argv element; limits failing closed on all four bad inputs plus honored overrides; the fail-closed verdict broken one property at a time (each naming exactly one failure) and all five at once with attestation present; the tier never assumed from plumbing; and the generated-documentation drift guard.

Part of #9264 — running under genuine SNP remains blocked on #8535/#8536, so I've left the issue open rather than closing it.

…un verdict (#9264)

Candidate agents are untrusted by construction: arbitrary third-party code
submitted to be scored. Two facts drive this module.

ATTESTATION IS NOT A SANDBOX. Attestation proves WHAT ran; it constrains
nothing about what that code can reach while running. So every property
here holds with or without SNP hardware and none may be skipped because
attestation is present -- they are enforced by the container runtime,
which exists today, rather than waiting on the TEE fleet (#8535/#8536).

THE NETWORK IS THE WHOLE BALLGAME. A candidate that can reach the network
can fetch the realized future -- the outcomes it is being asked to predict
-- and score perfectly having learned nothing. #9259 leak-proofs the DATA;
this leak-proofs the RUNTIME. Both are required: a perfect snapshot handed
to a networked process is not a benchmark.

The policy is pure and produces an argv ARRAY, never a shell string: a
snapshot path is data we do not fully control, and as a string it would be
a command-injection seam. Asserted with a path full of shell
metacharacters. Exactly two mounts exist -- the task read-only, one output
file writable -- so no host filesystem of ours is reachable.

decideCandidateRunVerdict is FAIL-CLOSED, matching the attested-run path's
posture: a run that could not establish its claimed isolation does not
score at all. Not scored-and-flagged, not scored at a lower tier -- a
networked agent's score is not a weaker measurement of the same thing, it
is a measurement of something else, and admitting it at any tier would put
a fabricated number on the leaderboard. The tier is read from what was
ESTABLISHED, so "the plumbing supports attestation" can never become an
`attested` label.

A non-positive or non-finite resource limit falls back to the default
rather than being honored: failing open on a bound is the one direction
that cannot be allowed. The documented limits table is generated from the
same constants the runner uses, so the doc cannot drift from what is
actually enforced.

Part of #9264; running under genuine SNP remains blocked on #8535/#8536.
@loopover-orb

loopover-orb Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Warning

⏸️ LoopOver review result - manual review recommended

Review updated: 2026-07-29 02:53:44 UTC

3 files · 1 AI reviewer · 3 blockers · CI green · clean

⏸️ Suggested Action - Manual Review

Review summary
The AI review returned blocking findings for this change but did not include a separate narrative summary. Review the blockers below before deciding this PR.

Blockers

Nits — 5 non-blocking
  • DEFAULT_CANDIDATE_LIMITS values (2048 MB, 600s, 256 pids) are bare numeric literals with only a block comment above explaining intent collectively; a per-field inline comment or named constant would make the rationale traceable to each number (candidate-sandbox.ts:36-39).
  • wallClockSeconds is part of CandidateResourceLimits and resolved/validated but never appears in candidateSandboxArgs' output — worth confirming the runner applies it separately (e.g., via a timeout wrapper) rather than it being silently unused.
  • decideCandidateRunVerdict's EstablishedIsolation.genuineAttestation is trusted as a boolean input from the runner with no further validation in this pure module — reasonable given the doc comment, but worth a one-line note that the runner is the sole source of truth for that determination.
  • The --mount syntax embeds spec.taskInputPath/outputPath directly into a single argv string element; this is safe from shell injection per the test, but a path containing a comma would break Docker's own mount-flag parsing — likely acceptable given paths are engine-controlled, not attacker input directly forwarded.
  • Consider extracting the numeric defaults (2048, 600, 256) into named intermediate constants or JSDoc @​default-style annotations for easier future tuning.

Concerns raised — review before merging

📋 Copy for AI agents — paste into your coding agent
Fix the following blocker(s) from this PR review:

1. Per the repo's issue-scope enforcement convention, this PR must close or clearly link an eligible open issue — the description only references \#9264/\#9259/\#8535 as related context, not as an issue this PR closes, so confirm the linkage before marking merge-ready.

2. No linked issue detected: No closing reference or linked issue number was found in the PR metadata/body. — If this PR is intended to solve an issue, link it explicitly in the PR body.

3. Maintainer requires a linked issue: This repo's maintainer focus manifest requires every PR to reference a tracked issue. — Link the relevant issue (for example `Closes #123`) before opening the PR.

Decision drivers

  • ❌ Code review — 3 blockers (1 reviewer)
  • ❌ Gate result — Blocking (Repo-configured hard blocker found.)
Context & advisory signals — never blocks the verdict
Signal Result Evidence
Linked issue ⚠️ Missing No linked issue or no-issue rationale found.
Related work ✅ No active overlap found No same-issue or scoped active PR overlap found.
Change scope ✅ 20/20 Low review scope from cached public metadata (no linked issue context).
Validation posture ✅ 25/25 PR body includes validation/test evidence.
Contributor workload ✅ 10/10 Author activity: 14 registered-repo PR(s), 13 merged, 284 issue(s).
Contributor context ✅ Confirmed Gittensor contributor JSONbored; Gittensor profile; 14 PR(s), 284 issue(s).
Improvement ✅ Minor risk: clean · value: minor
Review context
  • Author: JSONbored
  • Role context: owner (maintainer lane)
  • Public audience mode: oss maintainer
  • Lane context: Repository is registered but has no active allocation in the current snapshot.
  • Public profile languages: not available
  • Official Gittensor activity: 14 PR(s), 284 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Start here: Treat this as maintainer-lane context rather than normal contributor-lane activity.
  • Then work through the remaining 3 steps in the Signals table above.
Signal definitions
  • Related work = same linked issue, overlapping active PRs, or title/path similarity.
  • Change scope = cached public metadata such as size labels, draft state, and review-burden hints.
  • Validation posture = whether the PR provides enough public validation/test evidence for maintainer review.
  • Contributor workload = public contributor activity and cleanup pressure, not a repo-wide quality failure.
  • Contributor context = public GitHub/Gittensor identity context; non-Gittensor status is not a blocker.
🧪 Chat with LoopOver

Ask LoopOver a question about this PR directly in a comment — grounded only in the same cached, public-safe facts shown above, never a new claim.

  • @loopover ask <question> answers contribution-quality Q&A with source citations and freshness.
  • @loopover chat <question> answers in natural prose from cached decision-pack facts via local inference (maintainer/collaborator; read-only).
  • A plain-language @loopover mention with a real question is routed to the closest matching read-only command automatically — no exact syntax required.

Full command reference: https://loopover.ai/docs/loopover-commands

🧪 Experimental — new and may change.

Decision record
  • action: hold · clause: missing_linked_issue
  • config: 011a6edcd6106ac4ff3aa224e70fcf66135af4a3853b07fc29568835a6ae1669 · pack: oss-anti-slop · ci: passed
  • model: claude-code · prompt: 7ac46a3f8ed0bb19bf9274163ef5821c41a4b11125e4c2ffc182d26d1c35f412 · confidence: 0.55
  • record: 565dad1fa69e52d922e5a007e23133c7438c6d68e41c97ad50408918c4a14238 (schema v5, head d25ea2e)

🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed


💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →.

Checked by LoopOver, a quiet PR intelligence layer for OSS maintainers.

  • Re-run LoopOver review

@superagent-security

Copy link
Copy Markdown
Contributor

Superagent didn't find any vulnerabilities or security issues in this PR.

@cloudflare-workers-and-pages

Copy link
Copy Markdown

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Preview URL Updated (UTC)
✅ Deployment successful!
View logs
loopover-ui d25ea2e Commit Preview URL

Branch Preview URL
Jul 28 2026, 11:49 PM

@codecov

codecov Bot commented Jul 28, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.90%. Comparing base (1819298) to head (d25ea2e).
⚠️ Report is 1 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #9606      +/-   ##
==========================================
+ Coverage   89.89%   89.90%   +0.01%     
==========================================
  Files         883      884       +1     
  Lines      111692   111864     +172     
  Branches    26579    26587       +8     
==========================================
+ Hits       100408   100576     +168     
  Misses       9992     9992              
- Partials     1292     1296       +4     
Flag Coverage Δ
backend 95.42% <16.66%> (-0.03%) ⬇️
engine 66.51% <100.00%> (+0.16%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
...opover-engine/src/calibration/candidate-sandbox.ts 100.00% <100.00%> (ø)
packages/loopover-engine/src/index.ts 100.00% <100.00%> (ø)

... and 1 file with indirect coverage changes

@JSONbored JSONbored self-assigned this Jul 28, 2026
@loopover-orb loopover-orb Bot added the manual-review Gittensor contributor context label Jul 29, 2026
@JSONbored
JSONbored merged commit 3200abc into main Jul 29, 2026
8 checks passed
@JSONbored
JSONbored deleted the feat/candidate-sandbox-9264 branch July 29, 2026 04:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

manual-review Gittensor contributor context

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant