Add Runner OCI token

[epompeii]

This changeset allow the Runner to pull from the OCI registry with a short lived, project scoped token.

[github-actions]

🤖 Claude Code Review

PR: #666
Base: devel
Head: u/ep/runner-token
Commit: bbc76004108c64e7f9be2e7d4bf125a7ecce9565

---

Here's my review:

---

PR Review: Runner OCI Tokens + Job Claim API Refactoring

Summary

This PR makes three related changes:

1. New JsonClaimedJob response type — replaces the generic JsonJob at claim time with a purpose-built type that includes a short-lived OCI pull token
2. Runner OCI tokens — a new JWT token type (RunnerOciClaims) scoped to a runner UUID (instead of an email), for OCI registry pull access
3. Removal of the PATCH job update endpoint — job status transitions now happen exclusively via the WebSocket channel; the HTTP PATCH /v0/runners/{runner}/jobs/{job} endpoint is removed

Positives

• Clean API surface reduction. Removing the PATCH endpoint in favor of the WebSocket channel eliminates a redundant code path and simplifies the state machine. The WebSocket handler already has the same state transition logic with better TOCTOU protection.
• Type safety. JsonClaimedJob makes all claim-time fields non-optional (config, spec, oci_token, timeout), removing a class of unwrap/expect calls in the runner agent.
• Strong token separation. Runner OCI tokens use RunnerUuid as the sub claim, which is structurally incompatible with user OCI tokens (Email subject). The tests at lib/bencher_token/src/key.rs:438-468 verify bidirectional rejection.
• Scope minimization. Runner OCI tokens are pull-only and scoped to a single project repository, following least-privilege.
• Good test coverage. New tests for runner token pull, HEAD, wrong-repo rejection, push-only rejection, and base-endpoint rejection.

Issues

1. Double expiration check in validate_oci_runner (Minor)

validate_oci_runner (lib/bencher_token/src/key.rs:178-196) manually checks exp < now after decoding. However, jsonwebtoken::decode with Validation already checks exp by default (unless validate_exp is set to false). The existing validate() method has the same pattern, so this is consistent within the codebase, but it means there's a (harmless) redundant check. The custom check does add exp/now to the error for diagnostics, which is the likely justification.

2. Test helper uses u32::MAX TTL (Minor)

In lib/bencher_api_tests/src/oci.rs:56, oci_runner_pull_token passes u32::MAX as the TTL. The new_oci_runner method handles overflow via checked_add(...).unwrap_or(now), which would produce a token that expires immediately if the addition overflows. In practice now + u32::MAX (~136 years) won't overflow i64, so this works fine, but it's worth noting that test tokens effectively never expire, which may mask real expiration-related bugs in integration tests.

3. validate_pull_identity error swallowing (Worth Noting)

In plus/api_oci/src/auth.rs:170-177, when the runner token validation fails, the code falls through to try user token validation. If both fail, only the user token error is surfaced. This is acceptable for the "try runner first, fall back to user" pattern, but if both token types fail, the error message will always say "Invalid or expired token" without indicating that a runner token was also attempted. A debug-level log for the runner validation failure could help with troubleshooting.

4. OCI_RUNNER_TOKEN_TTL placement (Nit)

The constant OCI_RUNNER_TOKEN_TTL at plus/api_runners/src/jobs/mod.rs:127 is declared between the doc comment for try_claim_job and the function signature. It's syntactically valid but reads oddly — looks like part of the doc comment block. Consider moving it up near the other constants (lines 36-43).

5. WebSocket path change is a breaking change

The WebSocket endpoint moved from /v0/runners/{runner}/jobs/{job}/channel to /v0/runners/{runner}/jobs/{job}. Since the old PATCH endpoint at the same path was removed, there's no collision, but any existing runner agents targeting the old path will break. This is fine if runner agents are updated in lockstep, but worth calling out.

6. Missing #[cfg(feature = "plus")] on OCI module (Bug Fix)

The change in lib/bencher_api_tests/src/lib.rs:38 adding #[cfg(feature = "plus")] to pub mod oci and the lib/bencher_schema/src/context/indexer.rs / mod.rs changes fixing the #[cfg(feature = "plus")] gating look like correctness fixes for non-plus builds. Good catches.

Security

• Token isolation is solid. The RunnerUuid vs Email subject type difference ensures runner tokens can't be confused with user tokens at the type level.
• Scope is properly constrained. Runner OCI tokens are pull-only, project-scoped, with a 10-minute TTL — appropriate for the image-pull use case.
• Rate limiting skipped for runners. The apply_auth_rate_limit function in plus/api_oci/src/auth.rs:500-510 skips rate limiting for runner OCI pulls with a comment that runners already have per-runner rate limiting on the claim endpoint. This is reasonable since the claim endpoint is the gateway, but note that a compromised runner token could make unlimited OCI pull requests within the 10-minute TTL window.

Breaking Changes

• JsonJob no longer returned from the claim endpoint (now JsonClaimedJob)
• PATCH /v0/runners/{runner}/jobs/{job} removed
• WebSocket path changed from .../channel to .../jobs/{job}
• JsonUpdateJob, JsonUpdateJobResponse, JobUpdateStatus types removed from the API schema and TypeScript types

All breaking changes are internal to the runner agent protocol (not user-facing API), so they're manageable with a coordinated deploy.

Verdict

The changes are well-structured and improve the runner protocol. The main items to consider are the debug logging in validate_pull_identity for troubleshooting and ensuring runner agents are updated in lockstep with this API change.

---

_{Model: claude-opus-4-6}

[github-actions]

Bencher Report

Branch	u/ep/runner-token
Testbed	ubuntu-22.04

Click to view all benchmark results

Benchmark	Latency	Benchmark Result microseconds (µs) (Result Δ%)	Upper Boundary microseconds (µs) (Limit %)
Adapter::Json	📈 view plot 🚷 view threshold	3.76 µs (+9.04%) Baseline: 3.45 µs	4.65 µs (80.91%)
Adapter::Magic (JSON)	📈 view plot 🚷 view threshold	3.67 µs (+6.33%) Baseline: 3.45 µs	4.60 µs (79.89%)
Adapter::Magic (Rust)	📈 view plot 🚷 view threshold	25.69 µs (-0.19%) Baseline: 25.74 µs	29.50 µs (87.08%)
Adapter::Rust	📈 view plot 🚷 view threshold	2.87 µs (+3.06%) Baseline: 2.78 µs	3.09 µs (92.72%)
Adapter::RustBench	📈 view plot 🚷 view threshold	2.87 µs (+3.09%) Baseline: 2.79 µs	3.09 µs (92.96%)
head_version_insert/batch/10	📈 view plot 🚷 view threshold	92.74 µs (+4.37%) Baseline: 88.85 µs	104.30 µs (88.92%)
head_version_insert/batch/100	📈 view plot 🚷 view threshold	229.66 µs (+1.63%) Baseline: 225.97 µs	240.42 µs (95.52%)
head_version_insert/batch/255	📈 view plot 🚷 view threshold	450.15 µs (+0.15%) Baseline: 449.48 µs	475.37 µs (94.69%)
head_version_insert/batch/50	📈 view plot 🚷 view threshold	153.85 µs (+3.06%) Baseline: 149.29 µs	167.34 µs (91.94%)
threshold_query/join/10	📈 view plot 🚷 view threshold	136.98 µs (+2.73%) Baseline: 133.34 µs	153.06 µs (89.50%)
threshold_query/join/20	📈 view plot 🚷 view threshold	149.92 µs (+1.34%) Baseline: 147.93 µs	162.76 µs (92.11%)
threshold_query/join/5	📈 view plot 🚷 view threshold	129.66 µs (+2.85%) Baseline: 126.06 µs	144.08 µs (89.99%)
threshold_query/join/50	📈 view plot 🚷 view threshold	188.60 µs (-0.17%) Baseline: 188.93 µs	206.53 µs (91.32%)

🐰 View full continuous benchmarking report in Bencher