feat(sidecar): add http listen mode for direct attestation dial - #2
Merged
echobt merged 1 commit intoJul 27, 2026
Merged
Conversation
The sidecar was pull-only (run / answer-once) and never bound a socket, so BASE could not reach it. Add a stdlib ThreadingHTTPServer exposing POST /v1/sidecar/attest and GET /healthz, reusing the existing measurement and signing path rather than duplicating it. Uses only the standard library so the image stays hermetic: no fastapi, uvicorn or any new dependency. Requests are size-capped and malformed input fails closed (400/404/405/413/500) without leaking stack traces or secret material. Ports are published for Lium via EXPOSE 8787 on both Dockerfiles.
📝 WalkthroughWalkthroughAdds an HTTP listen mode for sidecar attestation, a ChangesSidecar listen mode
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant HTTPServer
participant AttestationSidecar
Client->>HTTPServer: POST /v1/sidecar/attest with nonce and phase
HTTPServer->>AttestationSidecar: answer_challenge(Challenge)
AttestationSidecar-->>HTTPServer: ChallengeAnswer
HTTPServer-->>Client: JSON attestation response
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/prism_recipe/sidecar/__main__.py (1)
17-17: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate wire-construction logic; reuse
answer_to_wire.
_cmd_answer_oncemanually rebuilds the exact same dict shape (signed_attestation_to_wire+phase/baked_manifest_match/mismatched_paths) thatlisten.answer_to_wirenow encapsulates. Since a test (test_wire_shape_matches_answer_once) explicitly guards that these two paths stay identical, keeping one source of truth avoids future drift.♻️ Proposed refactor
-from prism_recipe.sidecar.listen import serve_forever +from prism_recipe.sidecar.listen import answer_to_wire, serve_forever- wire = signed_attestation_to_wire(answer.signed) - wire["phase"] = answer.phase.value - wire["baked_manifest_match"] = answer.baked_manifest_match - wire["mismatched_paths"] = list(answer.mismatched_paths) + wire = answer_to_wire(answer)Also applies to: 110-114
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/prism_recipe/sidecar/__main__.py` at line 17, Update _cmd_answer_once to construct its response through listen.answer_to_wire instead of manually combining signed_attestation_to_wire with phase, baked_manifest_match, and mismatched_paths. Reuse this existing helper as the single source of truth while preserving the current wire response shape and values.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/prism_recipe/sidecar/__main__.py`:
- Line 176: Preserve an explicit zero retry budget in the retry configuration
branches by replacing the retry_budget fallback expressions with an explicit
None check, matching the neighboring interval_count, interval_min_s, and
interval_max_s handling. Update both retry_budget assignments around the
affected configuration logic so None still selects the default while 0 remains
fail-fast.
---
Nitpick comments:
In `@src/prism_recipe/sidecar/__main__.py`:
- Line 17: Update _cmd_answer_once to construct its response through
listen.answer_to_wire instead of manually combining signed_attestation_to_wire
with phase, baked_manifest_match, and mismatched_paths. Reuse this existing
helper as the single source of truth while preserving the current wire response
shape and values.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: e0e75a3c-aeda-4458-9c70-45d35b5a916e
📒 Files selected for processing (5)
DockerfileDockerfile.cudasrc/prism_recipe/sidecar/__main__.pysrc/prism_recipe/sidecar/listen.pytests/test_sidecar_listen.py
| interval_min_s=args.interval_min_s if args.interval_min_s is not None else 0.0, | ||
| interval_max_s=args.interval_max_s if args.interval_max_s is not None else 0.0, | ||
| base_url=args.base_url, | ||
| retry_budget=retry_budget or 3, |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
retry_budget=0 is silently overridden to the default.
Both branches use retry_budget or <default>, but interval_count/interval_min_s/interval_max_s on the same lines correctly use an explicit is not None check to preserve 0. Since 0 is a falsy-but-valid value for retry_budget (fail-fast, no retries), 0 or 3 (and 0 or env_cfg.retry_budget) silently discards an explicit --retry-budget 0.
🐛 Proposed fix
- retry_budget=retry_budget or 3,
+ retry_budget=retry_budget if retry_budget is not None else 3,- retry_budget=retry_budget or env_cfg.retry_budget,
+ retry_budget=retry_budget if retry_budget is not None else env_cfg.retry_budget,Also applies to: 191-191
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/prism_recipe/sidecar/__main__.py` at line 176, Preserve an explicit zero
retry budget in the retry configuration branches by replacing the retry_budget
fallback expressions with an explicit None check, matching the neighboring
interval_count, interval_min_s, and interval_max_s handling. Update both
retry_budget assignments around the affected configuration logic so None still
selects the default while 0 remains fail-fast.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
run/answer-once) and never bound a socket, so BASE had no way to reach it.Changes
src/prism_recipe/sidecar/listen.py—ThreadingHTTPServerfromhttp.server.POST /v1/sidecar/attest— body{"nonce": "<str>", "phase": "start"|"interval"|"end"}(phase defaults tostart). Returns the same signed wire payload asanswer-once.GET /healthz— cheap liveness, no measurement, no signing.build_server(config, host, port)factory;hostis explicit (no implicit0.0.0.0inside the library) so tests can bind port0.src/prism_recipe/sidecar/__main__.py— newservesubcommand (--host, default0.0.0.0;--port, default8787), identity resolution shared withrun.Dockerfile/Dockerfile.cuda—EXPOSE 8787so the port can be published as a Liuminternal_portsentry.Measurement and signing are reused from
service.py/wire.py, not reimplemented.Failure behaviour (fail-closed)
Test plan
uv run pytest tests/test_sidecar_listen.py -q— 12 cases (healthz, happy-path attest, malformed JSON, unknown path, wrong method, oversized body); server runs on a background thread bound to port0and is torn down by fixture.uv run pytest -q— 145 passed, 5 skipped. The single failuretest_gpu_train.py::test_run_gpu_short_train_cpu_fingerprintistorch_not_installed, environmental and pre-existing: that test imports onlyprism_recipe.gpu_trainand has zero references to the sidecar.uv run ruff check .— clean.uv run ruff format --checkon the three touched files — clean. (Repo-wideformat --checkreports 18 pre-existing unformatted files that this PR deliberately does not touch.)curl /healthz->200 {"status":"ok"};curl -X POST /v1/sidecar/attest->200signed payload; malformed body ->400 {"error":"malformed JSON"}. Process killed and port confirmed released.CI
Repro imagecheck on this head SHA. The Dockerfile change (EXPOSE) is deterministic, so the dual no-cache build must still yield identical digests.Notes
pyproject.tomland lockfiles untouched.baserepo.Summary by CodeRabbit
New Features
Bug Fixes
Tests