[Harbor 2/4] eval sidecar: verifier, token auth, HTTP API (Mode A)#4
[Harbor 2/4] eval sidecar: verifier, token auth, HTTP API (Mode A)#4varunursekar wants to merge 2 commits into
Conversation
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
|
|
||
| # Mode A | ||
| task: str | None = None | ||
| task_project: str | None = None |
There was a problem hiding this comment.
Security, the headline one. In Mode A this defaults to None, so the Evaluator runs uv in the agent's own project and loads the @task.evaluation() scorer from the agent's committed code. Verifier.finalize → evaluate_admin then scores the hidden split with that agent-controlled scorer, so a committed scorer that returns 1.0 wins the leaderboard. The token/volume/object-copy machinery secures transport, not scorer provenance. For a gradeable boundary the verifier must score with a sidecar-baked task project: require task_project/inner_task for Mode A and reject configs where the scorer would resolve from the agent repo. This is the default posture and the single most important gap in the stack.
| tok = generate_token() | ||
| p = write_admin_token(tmp_path / "t", tok) | ||
| assert read_admin_token(p) == tok | ||
| assert (p.stat().st_mode & 0o777) == 0o600 |
There was a problem hiding this comment.
For the security-critical PR, the OS-level claims are asserted only as a 0o600 mode bit plus happy-path mocks. Please add adversarial coverage: (1) agent /eval on a no_access split → rejected end-to-end; (2) an integration test that finalize does not execute an agent-supplied scorer (the Mode A provenance gap); (3) a uid-drop test that agent.user cannot read the token file.
| workspace = await GitWorkspace.create(config.repo_path) | ||
|
|
||
| budget = BudgetLedger( | ||
| [SplitBudget(**b) for b in config.budgets], |
There was a problem hiding this comment.
build_components rebuilds the BudgetLedger from config.budgets on every start, and the ledger writes persist_path on each mutation but never reads it back. So a sidecar restart mid-trial silently resets spent budget — metering is not crash-safe despite the durable-ledger framing. Add a load-if-exists path that seeds from the persisted ledger when the file exists, and from config.budgets only when it does not.
| """Best candidate by recorded score on the selection split (excludes baseline).""" | ||
| if self.engine.db is None: | ||
| raise NoCandidateError("auto_best mode but no experiment database.") | ||
| df = self.engine.db.get_experiments_df(fill_score=default_minimum_score) |
There was a problem hiding this comment.
auto_best ranks candidates solely by the recorded mean_score from engine.db.get_experiments_df, but that DB is written by the agent's metered /eval calls and the verifier shares the same in-process engine/DB. The only admin-independent scoring at finalize is evaluate_admin on the test target — after selection, and only on the chosen winner. So a candidate that records an inflated selection score selects itself. (Mode B in #5 sharpens this: the recorded selection score is the reward a nested run of the candidate emitted, which the optimizer controls.) Before trusting the ranking, re-score under admin: re-run evaluate_admin on the selection split for the top-K and re-rank, or at minimum re-score the winner and fail closed on deviation.
| for sa in split_accesses: | ||
| if sa.split == split: | ||
| return sa.access | ||
| return SplitAccessLevel.viewable |
There was a problem hiding this comment.
This fails open, the wrong default for a trust boundary. tier_for_split is the gate _route_results uses to decide whether to write full per-sample SampleResults (labels included) to the agent volume; an author who budgets a split but forgets to add it to split_accesses lands here and gets viewable → ground truth on the agent-readable volume. Default to the most restrictive tier (no_access) and update the docstring, which currently advertises the fail-open as intended. If an unlisted split should ever be visible, make that explicit and logged.
| async def evaluate(self, req: EvalRequest, *, admin: bool = False) -> EvalSummary: | ||
| sha = await self._transfer_commit(req.commit) | ||
| exp = await self.engine.evaluate(replace(req, commit=sha), admin=admin) | ||
| result_path = self._route_results(exp, admin=admin) |
There was a problem hiding this comment.
tier_for_split() now treats unlisted splits as no_access, and /status reports those budgeted-but-unlisted splits as not agent-evaluable. But this path still transfers and scores the commit before _route_results() suppresses only the result files. If a config includes a budget for a hidden or unlisted split, the agent can call /eval and receive mean_score, n_samples, and budget data in the returned EvalSummary. Please reject non-admin evals whose tier is no_access before calling engine.evaluate(), or ensure those splits cannot be present in the agent budget allowlist.
Artifacts
Repro: focused pytest exercising non-admin hidden split EvaluationSidecar.evaluate behavior
- Contains supporting evidence from the run (text/x-python; charset=utf-8).
Repro: verbose pytest output showing engine.evaluate call and leaked EvalSummary fields
- Keeps the command output available without making the summary code-heavy.
Prompt To Fix With AI
This is a comment left during a code review.
Path: vero/src/vero/harbor/server.py
Line: 70-73
Comment:
**Gate hidden evals**
`tier_for_split()` now treats unlisted splits as `no_access`, and `/status` reports those budgeted-but-unlisted splits as not agent-evaluable. But this path still transfers and scores the commit before `_route_results()` suppresses only the result files. If a config includes a budget for a hidden or unlisted split, the agent can call `/eval` and receive `mean_score`, `n_samples`, and budget data in the returned `EvalSummary`. Please reject non-admin evals whose tier is `no_access` before calling `engine.evaluate()`, or ensure those splits cannot be present in the agent budget allowlist.
How can I resolve this? If you propose a fix, please make it concise.
|
Empirical finding from a live Mode B smoke run (GAIA example, nested Modal evals, combined stack #3-#6 + fix PRs #7-#9): With because the selector excludes Two thoughts:
(Fix PR #8 keeps the exclusion unchanged; this is a separate follow-up decision. Happy to implement either piece.) |
The evaluation engine, run in a sidecar container, with the trust boundary that makes an optimization run leaderboard-gradeable: - `EvaluationSidecar` (server.py): agent-facing handlers — commit transfer from the untrusted agent repo (git fetch, hooks off, object copy) and tier-gated write-routing of results across the agent-readable and admin volumes. - `Verifier` (verifier.py): commit selection (submit | auto_best) + hidden-split scoring. - Per-trial admin token (auth.py), written root:600 so the optimizer (de-privileged) cannot read it; only the verifier (root, shared mode) can. - FastAPI surface (app.py): /eval, /submit, /status for the agent (metered, redacted); /finalize for the verifier (token-gated). `vero harbor serve` (serve.py) assembles the engine + sidecar + verifier from a ServeConfig and runs it under uvicorn. - `vero harbor` CLI clients (cli.py): serve | eval | submit | status | finalize (build/run land with the compiler). HarborConfig + the Mode-B dataset partition helpers (config.py, dataset.py) are included so the harbor package imports cleanly. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ers, budget reload Trust-boundary hardening for the eval sidecar (review findings on PR #4): - protocol: tier_for_split fails CLOSED (unlisted split -> no_access, with a warning), instead of fail-open to viewable. - serve: Mode A now requires task_project. build_components refuses to start a Mode A config without it, so the hidden-split/admin scorer is always loaded from the sidecar-baked task project, never the agent's committed repo (a committed scorer returning 1.0 can no longer win the reward). - verifier: auto_best no longer trusts the agent-influenced recorded score. It shortlists by recorded score, then re-runs evaluate_admin (the trusted scorer) on the selection split for the top-K and ranks by the admin score; fails closed. - serve: reload the persisted budget ledger on startup so a sidecar restart does not reset spent budget to full. - tests: adversarial coverage (no_access -> 400, finalize does not run an agent-supplied scorer via a cheating-agent repo, admin token 0o600/unreadable). Also folds in the valid Greptile findings on this PR: clear stale per-sample result files on re-eval, type reward_mode as Literal[submit, auto_best], and filter auto_best candidates by selection dataset id. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
varunursekar
force-pushed
the
harbor-2-sidecar
branch
from
767570e to
38709b1
Compare
| target = ref or "HEAD" | ||
| fetch = await workspace.sandbox.run( | ||
| [ | ||
| "git", | ||
| "-c", | ||
| "core.hooksPath=/dev/null", | ||
| "-c", | ||
| "protocol.file.allow=always", | ||
| "-C", | ||
| root, | ||
| "fetch", | ||
| "--no-tags", | ||
| "--no-recurse-submodules", | ||
| f"file://{self.agent_repo_path}", | ||
| target, | ||
| ], | ||
| timeout=120, | ||
| ) | ||
| if fetch.returncode != 0: | ||
| raise CommitTransferError( | ||
| f"git fetch of {target!r} from agent repo failed: {fetch.stderr}" | ||
| ) | ||
| rev = await workspace.sandbox.run( | ||
| ["git", "-C", root, "rev-parse", "FETCH_HEAD"], timeout=30 | ||
| ) | ||
| if rev.returncode != 0: | ||
| raise CommitTransferError(f"rev-parse FETCH_HEAD failed: {rev.stderr}") | ||
| return rev.stdout.strip() |
There was a problem hiding this comment.
FETCH_HEAD is a single file inside the sidecar's repo; with max_concurrency=20 and uvicorn handling concurrent /eval requests, two in-flight _transfer_commit calls can race: fetch B overwrites FETCH_HEAD after fetch A finishes but before A's rev-parse FETCH_HEAD runs. A then resolves B's SHA and evaluates B's commit while attributing the result to A's request — a trust-boundary violation where one agent can inadvertently (or deliberately via rapid concurrent calls) evaluate a different commit.
The fix is to write each transfer to a unique local ref (<target>:<unique-local-ref>) and resolve that ref, then delete it, so concurrent transfers never share state.
Prompt To Fix With AI
This is a comment left during a code review.
Path: vero/src/vero/harbor/server.py
Line: 118-145
Comment:
**FETCH_HEAD shared across concurrent transfers**
`FETCH_HEAD` is a single file inside the sidecar's repo; with `max_concurrency=20` and uvicorn handling concurrent `/eval` requests, two in-flight `_transfer_commit` calls can race: fetch B overwrites `FETCH_HEAD` after fetch A finishes but before A's `rev-parse FETCH_HEAD` runs. A then resolves B's SHA and evaluates B's commit while attributing the result to A's request — a trust-boundary violation where one agent can inadvertently (or deliberately via rapid concurrent calls) evaluate a different commit.
The fix is to write each transfer to a unique local ref (`<target>:<unique-local-ref>`) and resolve that ref, then delete it, so concurrent transfers never share state.
How can I resolve this? If you propose a fix, please make it concise.
| p = Path(path) | ||
| p.parent.mkdir(parents=True, exist_ok=True) | ||
| p.write_text(token) | ||
| p.chmod(mode) | ||
| return p |
There was a problem hiding this comment.
write_text creates the file with the process umask (commonly 0o644), then chmod restricts it. In the window between the two syscalls the token is world-readable — any process sharing the same filesystem can race to read it. Since this token gates /finalize (the reward-writing endpoint), a leak lets agent.user call /finalize directly.
The fix is to open the file with the target mode in a single atomic step so it is never created with wider permissions.
| p = Path(path) | |
| p.parent.mkdir(parents=True, exist_ok=True) | |
| p.write_text(token) | |
| p.chmod(mode) | |
| return p | |
| import os | |
| p = Path(path) | |
| p.parent.mkdir(parents=True, exist_ok=True) | |
| fd = os.open(p, os.O_WRONLY | os.O_CREAT | os.O_TRUNC, mode) | |
| try: | |
| os.write(fd, token.encode()) | |
| finally: | |
| os.close(fd) | |
| return p |
Prompt To Fix With AI
This is a comment left during a code review.
Path: vero/src/vero/harbor/auth.py
Line: 24-28
Comment:
**Token readable during write-then-chmod window**
`write_text` creates the file with the process umask (commonly 0o644), then `chmod` restricts it. In the window between the two syscalls the token is world-readable — any process sharing the same filesystem can race to read it. Since this token gates `/finalize` (the reward-writing endpoint), a leak lets `agent.user` call `/finalize` directly.
The fix is to open the file with the target mode in a single atomic step so it is never created with wider permissions.
```suggestion
import os
p = Path(path)
p.parent.mkdir(parents=True, exist_ok=True)
fd = os.open(p, os.O_WRONLY | os.O_CREAT | os.O_TRUNC, mode)
try:
os.write(fd, token.encode())
finally:
os.close(fd)
return p
```
How can I resolve this? If you propose a fix, please make it concise.2 participants
Draft · Stack 2 of 4 — targets
harbor-1-core(review [1/4] first; its diff is the base here).The evaluation engine run in a sidecar container, plus the trust boundary that makes a run leaderboard-gradeable. This is the security-critical PR — a focused/security review is worthwhile here.
EvaluationSidecar(server.py): commit transfer from the untrusted agent repo (git fetch, hooks off, object copy) + tier-gated result write-routing across the agent-readable and admin volumes.Verifier(verifier.py): commit selection (submit | auto_best) + hidden-split scoring.root:600— unreadable by the de-privileged optimizer, readable by the verifier (root, shared mode)./eval/submit/status(agent; metered, redacted) and/finalize(verifier; token-gated).vero harbor serveassembles engine+sidecar+verifier from aServeConfig.HarborConfig/partition helpers so the package imports cleanly (build/runlight up in [3/4]).Stack: [1/4] core → this → [3/4] compiler → [4/4] docs.
🤖 Generated with Claude Code
Greptile Summary
This PR introduces the eval sidecar layer for Harbor:
EvaluationSidecar(commit transfer + tier-gated result routing),Verifier(commit selection + hidden-split scoring), per-trial admin token auth, a FastAPI HTTP surface, and thevero harbor serveentrypoint that wires everything together from aServeConfig. The three previously-flagged issues (stale result dirs, auto_best dataset scoping, reward_mode validation) are addressed in this revision.server.py: git fetch from the untrusted agent repo uses hooks-off +file://object copy; result write-routing is tier-gated (viewable / non_viewable / no_access) with the stale-directory fix (shutil.rmtreebefore re-writing).verifier.py:auto_bestshortlists by agent-recorded score then re-ranks with a trusted admin re-score, filtering byselection_dataset_idwhen the column is present;reward_modeis nowLiteral[\"submit\", \"auto_best\"]inServeConfig.auth.py/app.py: admin token is generated withsecrets.token_urlsafe, bearer-token check usessecrets.compare_digest, but the token file is written then chmod'd (TOCTOU window) and concurrent transfers shareFETCH_HEAD(race on concurrent/evalcalls).Confidence Score: 3/5
Three defects in the security-critical paths need fixes before merging: a FETCH_HEAD race in concurrent commit transfers, a write-then-chmod window on the admin token file, and a missing fallback for dataset_id in the verifier re-score loop.
The FETCH_HEAD file is overwritten by whichever git fetch completes last; with uvicorn serving concurrent /eval requests, two in-flight _transfer_commit calls can race and one resolves the other's SHA — breaking the commit-attribution invariant the trust boundary rests on. The token write-then-chmod window leaves the admin token briefly readable with default umask permissions. The verifier dataset_id fallback passes None to evaluate_admin when the column is absent, which can load the wrong dataset or raise at runtime.
vero/src/vero/harbor/server.py (_transfer_commit FETCH_HEAD race), vero/src/vero/harbor/auth.py (write_admin_token TOCTOU), vero/src/vero/harbor/verifier.py (_best_from_db dataset_id fallback)
Security Review
server.py:118–145): Concurrent_transfer_commitcalls share the singleFETCH_HEADfile in the sidecar's git repo. Withmax_concurrency=20and uvicorn serving concurrent/evalrequests, two in-flight fetches can race: one call reads the other's SHA, causing an evaluation to run on the wrong commit while attributing the result to the original request. This breaks the commit-attribution trust boundary.auth.py:24–28):write_admin_tokencreates the file with the default umask (typically 0o644) then callschmod(0o600). During the window between the two syscalls the admin token — which gates the reward-writing/finalizeendpoint — is world-readable on a filesystem accessible to the agent process.check_admincorrectly usessecrets.compare_digest.Important Files Changed
Sequence Diagram
%%{init: {'theme': 'neutral'}}%% sequenceDiagram participant Agent as Agent (agent.user) participant App as FastAPI App participant Sidecar as EvaluationSidecar participant AgentRepo as Agent Repo (mounted) participant SidecarRepo as Sidecar Repo participant Engine as EvaluationEngine participant AgentVol as Agent Volume participant AdminVol as Admin Volume Note over App,AdminVol: Startup App->>AdminVol: write_admin_token (root:600) Note over Agent,AgentVol: Agent eval loop Agent->>App: "POST /eval {commit, split}" App->>Sidecar: "evaluate(req, admin=False)" Sidecar->>AgentRepo: git fetch file:// (hooks off) AgentRepo-->>SidecarRepo: "object copy -> FETCH_HEAD" SidecarRepo-->>Sidecar: "rev-parse FETCH_HEAD -> sha" Sidecar->>Engine: "evaluate(commit=sha)" Engine-->>Sidecar: Experiment result Sidecar->>Sidecar: _route_results (tier_for_split) alt "tier = viewable" Sidecar->>AgentVol: summary.json + per-sample files else "tier = non_viewable" Sidecar->>AgentVol: summary.json only else "tier = no_access" Note over AgentVol: nothing written end App-->>Agent: EvalSummary (aggregate only) Note over Agent,AdminVol: Optional submit Agent->>App: "POST /submit {commit}" App->>Sidecar: submit(commit) Sidecar->>AdminVol: submission.json (sha) Note over App,AdminVol: Verifier finalizes (admin only) App->>App: POST /finalize (Bearer token) App->>App: check_admin (constant-time) App->>Verifier: finalize() alt "reward_mode = submit" Verifier->>AdminVol: "read submission.json -> sha" else "reward_mode = auto_best" Verifier->>Engine: get_experiments_df Verifier->>Engine: evaluate_admin (top-K re-score) Engine-->>Verifier: "admin scores -> winner sha" end loop each VerificationTarget Verifier->>Engine: evaluate_admin(hidden split) Engine-->>Verifier: score end Verifier-->>App: "{reward_key: score}" App-->>Verifier CLI: reward dict -> reward.json%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%% sequenceDiagram participant Agent as Agent (agent.user) participant App as FastAPI App participant Sidecar as EvaluationSidecar participant AgentRepo as Agent Repo (mounted) participant SidecarRepo as Sidecar Repo participant Engine as EvaluationEngine participant AgentVol as Agent Volume participant AdminVol as Admin Volume Note over App,AdminVol: Startup App->>AdminVol: write_admin_token (root:600) Note over Agent,AgentVol: Agent eval loop Agent->>App: "POST /eval {commit, split}" App->>Sidecar: "evaluate(req, admin=False)" Sidecar->>AgentRepo: git fetch file:// (hooks off) AgentRepo-->>SidecarRepo: "object copy -> FETCH_HEAD" SidecarRepo-->>Sidecar: "rev-parse FETCH_HEAD -> sha" Sidecar->>Engine: "evaluate(commit=sha)" Engine-->>Sidecar: Experiment result Sidecar->>Sidecar: _route_results (tier_for_split) alt "tier = viewable" Sidecar->>AgentVol: summary.json + per-sample files else "tier = non_viewable" Sidecar->>AgentVol: summary.json only else "tier = no_access" Note over AgentVol: nothing written end App-->>Agent: EvalSummary (aggregate only) Note over Agent,AdminVol: Optional submit Agent->>App: "POST /submit {commit}" App->>Sidecar: submit(commit) Sidecar->>AdminVol: submission.json (sha) Note over App,AdminVol: Verifier finalizes (admin only) App->>App: POST /finalize (Bearer token) App->>App: check_admin (constant-time) App->>Verifier: finalize() alt "reward_mode = submit" Verifier->>AdminVol: "read submission.json -> sha" else "reward_mode = auto_best" Verifier->>Engine: get_experiments_df Verifier->>Engine: evaluate_admin (top-K re-score) Engine-->>Verifier: "admin scores -> winner sha" end loop each VerificationTarget Verifier->>Engine: evaluate_admin(hidden split) Engine-->>Verifier: score end Verifier-->>App: "{reward_key: score}" App-->>Verifier CLI: reward dict -> reward.jsonPrompt To Fix All With AI
Reviews (3): Last reviewed commit: "fix(harbor): Mode A scorer provenance, a..." | Re-trigger Greptile