feat(constation): production Lium image-attestation wire#41
Conversation
Introduce DigestAllowlist, AttestationNonceService, ConstationRunner and related pure hosts so prism scoring can fail-closed on tamper-evidence without TEE claims. Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Persist BASE SoT image digests and single-use nonces for production constation. Lookup/consume rules stay in compute modules. Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Mount internal allowlist/nonce/bundle endpoints on the proxy, keep public challenge routes on Prism via proxy, and attach sealed bundles on result forward when present. Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Publish challenge/answer on the prism app (BASE proxy surface), inject bundle checkers at work_unit result ingest, and keep elevation fail-closed without insecure auto-inject in production. Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Add unit and ASGI coverage for durable repos, S1/S3 production ingest, S8 prism challenge/answer roundtrip, and forwarder bundle attach. Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Surface scripts for local constation path validation without claiming production TEE guarantees. Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
|
Warning Review limit reached
Next review available in: 39 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (23)
📝 WalkthroughWalkthroughThis change adds the Constation attestation protocol, digest allowlisting, durable nonce handling, Lium execution services, Prism fail-closed scoring gates, audited break-glass handling, HTTP routes, persistence wiring, and offline/live end-to-end validation. ChangesConstation attestation platform
Estimated code review effort: 5 (Critical) | ~120 minutes 🚥 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 |
Satisfy CI ruff/format gates on the production constation wire branch. Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Move attestation router import to module top, silence router attr assignment for mypy, and wrap long lines under prism line-length 100. Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
The two new unit test files perturbed siblings in the full alphabetical run, failing CI while passing in isolation: - test_e2e_lium_attestation_offline loaded scripts/e2e_lium_attestation.py by path, which inserts packages/challenges/prism/src into sys.path and imports prism_challenge. That left the lazily-imported prism dispatch adapter resolvable for the rest of the session, so test_validator_challenge_adapters::test_prism_adapter_unavailable_raises hit PrismGatewayConfigError instead of the expected unavailable path. - test_constation_forward_attach imported bittensor inside _minimal_proof, and that import forces every already-created logger to CRITICAL. The file sorts before the validator tests, so test_validator_weight_submitter::test_logs_do_not_include_wallet_secrets saw an empty caplog. Both are now wrapped in inlined snapshot/restore guards, matching the existing precedent in test_supervisor_weights, so the offending state is contained at the source rather than papered over in the victim tests. Local: pytest tests/unit 2300 passed, ruff, ruff format, mypy src tests clean.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 17
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/challenges/prism/src/prism_challenge/app.py (1)
442-472: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winRetryable constation infra faults are surfaced as permanent 422.
_evaluate_constation_gateraisesResultIngestionError("infra_fault:…")when the fault is retryable, but theexcept ResultIngestionErrormapping only returns 503 forfinalization_failed; every other reason becomes 422. The forwarder therefore treats a retryable infra fault as a permanent rejection. Compounding this,constation_attemptis never threaded through from this route (it always defaults to 1), so the retry-exhausted / break-glass branches in ingestion are unreachable via HTTP.🐛 Proposed fix for the status mapping
except ResultIngestionError as exc: # A transient finalization failure is retryable -> 503 so the forwarder retries; the # permanent rejections (bad proof / tampered / implausible manifest) stay 422. code = ( status.HTTP_503_SERVICE_UNAVAILABLE - if exc.reason == "finalization_failed" + if exc.reason == "finalization_failed" + or exc.reason.startswith("infra_fault:") else status.HTTP_422_UNPROCESSABLE_ENTITY )🤖 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 `@packages/challenges/prism/src/prism_challenge/app.py` around lines 442 - 472, Update the route’s ResultIngestionError mapping to return 503 for retryable “infra_fault:” reasons, while retaining 422 for permanent ingestion failures. Thread the request’s constation_attempt value through the route into the ingestion and _evaluate_constation_gate flow instead of relying on the default of 1, so retry-exhausted and break-glass handling is reachable and preserved.
🟡 Minor comments (11)
scripts/prism_lium_attestation_e2e.py-885-894 (1)
885-894: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winDead
BASE_LIVE_PROVIDER_TESTScheck in_detect_live.The
if ... != "1": passblock has no effect, soautogoes live purely on credential presence — which contradicts the module docstring stating live rentals are gated onBASE_LIVE_PROVIDER_TESTS=1. Either enforce the gate or delete the block and fix the docstring.🤖 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 `@scripts/prism_lium_attestation_e2e.py` around lines 885 - 894, Update _detect_live to enforce BASE_LIVE_PROVIDER_TESTS being set to "1" before attempting credential and SSH key loading, returning False otherwise; remove the no-op pass block while preserving the existing SystemExit handling.packages/challenges/prism/tests/test_prod_constation_kwargs.py-5-5 (1)
5-5: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winDrop
_test_constation_kwargsfrom the import — CIrufffails with F401.It is never referenced; the insecure-seam behavior is exercised through
_constation_ingest_kwargs.🔧 Proposed fix
-from prism_challenge.app import _constation_ingest_kwargs, _test_constation_kwargs +from prism_challenge.app import _constation_ingest_kwargs🤖 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 `@packages/challenges/prism/tests/test_prod_constation_kwargs.py` at line 5, Remove the unused _test_constation_kwargs symbol from the import in test_prod_constation_kwargs.py, retaining only _constation_ingest_kwargs, which is the symbol used by the tests.Source: Pipeline failures
packages/challenges/prism/tests/test_prism_attestation_routes_s8.py-7-8 (1)
7-8: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winRemove the unused
pytestimport — it failsruffin CI.No fixtures, marks, or
pytestAPIs are used in this module.🔧 Proposed fix
-import pytest from fastapi.testclient import TestClient🤖 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 `@packages/challenges/prism/tests/test_prism_attestation_routes_s8.py` around lines 7 - 8, Remove the unused pytest import from the test module, leaving the TestClient import and all test behavior unchanged.Source: Pipeline failures
scripts/prism_lium_attestation_e2e.py-41-48 (1)
41-48: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winBootstrap in-repo and install
prism_recipebefore importing it.
packages/challenges/prism/srcexists, soprism_challengeimports here need not resolve from a sibling checkout, but the path list still reaches forprism-recipeoutside the repo. This script also importsprism_recipeeagerly, and there is noprism_recipechecked in at the referenced sibling path, so the first import can fail: match the other e2e bootstrap style for Prism, and add an in-repo/installable path forprism_recipe.🤖 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 `@scripts/prism_lium_attestation_e2e.py` around lines 41 - 48, Update the bootstrap path setup before the eager imports in scripts/prism_lium_attestation_e2e.py: resolve prism_challenge from the in-repo packages/challenges/prism/src location, and add the repository’s installable prism_recipe source/path so prism_recipe imports succeed without relying on a sibling checkout. Follow the existing Prism e2e bootstrap convention and remove the invalid sibling prism-recipe path.tests/unit/test_compute_lium_client.py-1024-1025 (1)
1024-1025: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winThis assertion cannot fail.
str(type(err))is the class repr (<class '...LiumTemplateDigestMismatchError'>), which never contains a template id, so the "must not silently return the stale template id" intent isn't tested. The raise itself already proves no id was returned — either drop the line or assert onstr(err)content deliberately.🤖 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 `@tests/unit/test_compute_lium_client.py` around lines 1024 - 1025, Remove the ineffective assertion on str(type(err)) in the test covering LiumTemplateDigestMismatchError. The exception raise already verifies that no template ID is returned; if retaining an assertion for the stale ID, check str(err) deliberately instead and validate the intended message content.packages/challenges/prism/src/prism_challenge/audit.py-94-103 (1)
94-103: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winFail-open risk when a result object has
.ok = None.
getattr(constation_ok_result, "ok", None)treats an explicitNonethe same as “no.okattribute”, so a result object representing an indeterminate/pending constation can fall through tobool(constation_ok_result)and grant tier 1. Usehasattror a sentinel to distinguish these cases before evaluating.ok.🤖 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 `@packages/challenges/prism/src/prism_challenge/audit.py` around lines 94 - 103, The _constation_truthy function must distinguish an explicit result object .ok value of None from an object without an .ok attribute. Use a sentinel or equivalent attribute-presence check, and return the boolean value of .ok when the attribute exists so indeterminate results remain false rather than falling through to bool(constation_ok_result).packages/challenges/prism/src/prism_challenge/breakglass.py-41-52 (1)
41-52: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winRoute break-glass audit events to a durable sink.
ingest_work_unit_resultpasses throughbreak_glass_audit_logtoevaluate_break_glass, but production call sites and tests only provideBreakGlassAuditLog, which stores entries only in process memory. For compliance-sensitive override records, ensure production wires a durable implementation rather than the in-memory default.
[data_integrity_and_registration]🤖 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 `@packages/challenges/prism/src/prism_challenge/breakglass.py` around lines 41 - 52, Update production wiring around BreakGlassAuditLog and ingest_work_unit_result so break-glass audit events use a durable persistent sink instead of the in-memory default. Preserve BreakGlassAuditLog as an injectable test double, but ensure evaluate_break_glass receives the durable implementation in production and entries remain available through the existing audit interface.packages/challenges/prism/src/prism_challenge/attestation_routes.py-39-39 (1)
39-39: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winRemove the unused
Headerimport (ruff F401, failing CI).🔧 Proposed fix
-from fastapi import APIRouter, Depends, Header, HTTPException, Query, Request, status +from fastapi import APIRouter, Depends, HTTPException, Query, Request, status🤖 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 `@packages/challenges/prism/src/prism_challenge/attestation_routes.py` at line 39, Remove the unused Header symbol from the FastAPI imports in the attestation routes module, leaving the other imported symbols unchanged.Source: Pipeline failures
packages/challenges/prism/src/prism_challenge/attestation_routes.py-92-98 (1)
92-98: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick winDrop
_bearer_okor wire it intoauthenticate_internal.
packages/challenges/prism/src/prism_challenge/attestation_routes.pydefines_bearer_ok, but prism internal routes useauthenticate_internaland_bearer_okis unused in this module. If removing it preserves all intended auth behavior, drop the dead helper; otherwise connect it to the internal dependency before changing the default.🤖 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 `@packages/challenges/prism/src/prism_challenge/attestation_routes.py` around lines 92 - 98, Remove the unused _bearer_ok helper from attestation_routes.py if authenticate_internal already preserves the intended authorization behavior; otherwise integrate _bearer_ok into authenticate_internal’s internal-route authentication flow before modifying any defaults.packages/challenges/prism/tests/conftest.py-113-120 (1)
113-120: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winMake
no_auto_constationa registered pytest marker.
packages/challenges/prism/pyproject.tomldefines onlydistributed_gloounder[tool.pytest.ini_options] markers, so using this undeclared marker emitsPytestUnknownMarkWarningand fails with--strict-markers. If this marker should remain opt-in/unregistered, addpytest.warns(FutureWarning(FutureWarningPytestUnknownMarkWarning), ...)around this fixture instead of checking the marker directly.🤖 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 `@packages/challenges/prism/tests/conftest.py` around lines 113 - 120, Register the no_auto_constation marker in the pytest marker configuration in pyproject.toml, alongside distributed_gloo, with a description of its purpose. Keep the existing request.node.get_closest_marker check in _auto_constation_for_legacy_ingestion unchanged.src/base/master/constation/routes.py-211-223 (1)
211-223: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winInvalid
variantinregister_digestraises an unhandledValueError→ 500.
ImageVariant(body.variant.strip().lower())isn't guarded; a bad client value surfaces as an internal server error instead of a clean 422.🛡️ Proposed fix
async def register_digest(body: RegisterDigestBody) -> dict[str, str]: - record = DigestRecord( - commit_sha=body.commit_sha, - tree_sha=body.tree_sha, - variant=ImageVariant(body.variant.strip().lower()), - digest=body.digest, - ) + try: + variant = ImageVariant(body.variant.strip().lower()) + except ValueError as exc: + raise HTTPException( + status.HTTP_422_UNPROCESSABLE_ENTITY, detail=f"invalid variant: {exc}" + ) from exc + record = DigestRecord( + commit_sha=body.commit_sha, + tree_sha=body.tree_sha, + variant=variant, + digest=body.digest, + )🤖 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/base/master/constation/routes.py` around lines 211 - 223, Update register_digest to validate ImageVariant conversion errors and translate an invalid body.variant into the API’s standard 422 validation response, rather than allowing ValueError to propagate as a 500. Keep valid variant normalization and DigestRecord registration unchanged.
🧹 Nitpick comments (35)
src/base/compute/constation_poller.py (3)
232-234: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueStale comment.
The comment claims the interval is slept "in slices", but Line 234 issues a single
sleep_fn(interval)call.🤖 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/base/compute/constation_poller.py` around lines 232 - 234, Update the comment immediately above the sleep_fn(interval) call to accurately describe a single logical sleep for the interval, removing the claim that the wait occurs in slices.
350-350: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueHoist
fault_class_forto the module-level import.
base.compute.constation_typesis already imported at Lines 18-23; there is no cycle to break here.🤖 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/base/compute/constation_poller.py` at line 350, Move the fault_class_for import from its current local scope to the existing module-level imports in constation_poller.py, alongside the other imports from base.compute.constation_types. Remove the redundant local import while preserving all existing call sites.
105-115: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winToken bucket leaks a token on every throttled call.
try_takereturns the wait time but never decrementstokenson that path, and the caller at Lines 180-182 sleeps and then proceeds without re-acquiring. Each throttle event therefore issues one poll without paying a token, so the effective rate drifts aboverate_limit_per_second.♻️ Make acquisition explicit
- def try_take(self) -> float: - """Return 0 if a token was taken, else seconds to wait.""" + async def acquire(self, sleep_fn: SleepFn) -> None: + """Block until one token is available, then consume it.""" + while True: + wait = self._try_take() + if wait <= 0: + return + await sleep_fn(wait) + + def _try_take(self) -> float: now = self.now_fn()🤖 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/base/compute/constation_poller.py` around lines 105 - 115, Update the token acquisition flow around ConstationPoller.try_take so a throttled call does not proceed after sleeping without acquiring a token. Make the caller explicitly retry try_take after waiting, or otherwise reserve/decrement the token before returning the wait duration; preserve immediate success for available tokens and enforce the configured rate limit.src/base/compute/constation_runner.py (1)
158-193: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRedundant final corroboration on the last sample.
The loop at Lines 159-176 already evaluates every sample including
result.samples[-1], so Lines 178-182 recompute the same outcome. Capture the last loop result instead.♻️ Proposed simplification
+ corr = None for sample in result.samples: corr = evaluate_corroboration( lium_declared_digest=sample.lium_declared_digest, sidecar_digest=sample.sidecar_digest, ) if not corr.ok: ... - - last = result.samples[-1] - corr = evaluate_corroboration( - lium_declared_digest=last.lium_declared_digest, - sidecar_digest=last.sidecar_digest, - ) + assert corr is not None # samples is non-empty (checked above)🤖 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/base/compute/constation_runner.py` around lines 158 - 193, Remove the redundant evaluate_corroboration call after the sample-validation loop. Capture the loop’s corroboration result for the final sample and reuse it in the successful _record call, while preserving the existing mismatch handling and empty-result assumptions.src/base/compute/constation_custody.py (2)
88-135: 🔒 Security & Privacy | 🔵 Trivial | 💤 Low valueConsider a
revoke(miner_hotkey)API.The module docstring calls out mid-run revocation as a mitigation, but there is no way to drop a stored token once a key is known-bad;
registercan only overwrite. A smallrevoke/discardmethod would let control-plane code purge onlium_auth_revoked.🤖 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/base/compute/constation_custody.py` around lines 88 - 135, The custody class needs an API to remove a stored token for a known-bad key. Add a `revoke(miner_hotkey)` or equivalent discard method alongside `unlock_api_key`, validate the hotkey with `_require_nonblank`, and remove its entry from `_by_hotkey` without decrypting or logging the API key; preserve safe behavior when no token is registered.
46-47: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
_SupportsBalanceis unused.Nothing in this module (or
__all__) references it;default_lium_probeis typed againstLiumClient. Either drop it or use it to typedefault_lium_probe/ClientFactoryso tests can pass balance-only doubles (the E2E harness inscripts/e2e_lium_attestation.pyinjects a non-LiumClientobject).🤖 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/base/compute/constation_custody.py` around lines 46 - 47, Remove the unused _SupportsBalance protocol, or apply it consistently to default_lium_probe and ClientFactory so balance-only test doubles are accepted instead of requiring LiumClient. Ensure the chosen approach supports the non-LiumClient object injected by the E2E harness.scripts/e2e_lium_attestation.py (3)
376-376: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueBoth ternary branches are identical.
duration_seconds=10.0 if not adversarial else 10.0always evaluates to10.0.♻️ Proposed simplification
- duration_seconds=10.0 if not adversarial else 10.0, + duration_seconds=10.0,🤖 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 `@scripts/e2e_lium_attestation.py` at line 376, Update the duration_seconds assignment in the relevant attestation setup to use the constant value 10.0 directly, removing the redundant conditional expression while preserving the current behavior.
449-458: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
gap_obs/gap_budgetare assigned identically in both branches.Only
bundle_digestandlium_for_bundleactually differ; hoist the gap values out of the conditional.♻️ Proposed simplification
+ gap_obs = run_record.constation_observed_max_gap_seconds + gap_budget = run_record.constation_gap_budget_seconds if adversarial: bundle_digest = DIGEST_SWAPPED lium_for_bundle: str | None = DIGEST_HONEST # Lium still declares original - gap_obs = run_record.constation_observed_max_gap_seconds - gap_budget = run_record.constation_gap_budget_seconds else: bundle_digest = sidecar_digest lium_for_bundle = lium_declared - gap_obs = run_record.constation_observed_max_gap_seconds - gap_budget = run_record.constation_gap_budget_seconds🤖 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 `@scripts/e2e_lium_attestation.py` around lines 449 - 458, Hoist the identical assignments to gap_obs and gap_budget out of the adversarial conditional. Keep the if/else branches focused only on assigning bundle_digest and lium_for_bundle, preserving their existing branch-specific values.
384-384: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winTemp directory is never cleaned up.
run_offlineis invoked four times bytests/unit/test_e2e_lium_attestation_offline.py(two direct calls plus two CLI runs), each leaving a staged training corpus, artifacts, and a SQLite DB behind. The path is returned in the bag for debugging, so consider cleaning it on success (or honoring an env flag to keep it).🤖 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 `@scripts/e2e_lium_attestation.py` at line 384, Update the temporary-directory lifecycle around the run_offline flow that creates tmp with tempfile.mkdtemp: clean up the staged corpus, artifacts, and SQLite database after successful completion while preserving the returned path for debugging as required. Ensure cleanup also occurs on failure where appropriate, and provide an environment-controlled option to retain the directory when debugging is requested.tests/unit/test_attestation_nonce_repository.py (1)
144-163: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueTest name/docstring claims a race but the consumes are sequential.
r1/r2are awaited one after the other, so this only covers repeat consumption across handles, not concurrency. Considerasyncio.gather(or rename) so the atomic-claim path is actually exercised.🤖 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 `@tests/unit/test_attestation_nonce_repository.py` around lines 144 - 163, The test test_atomic_double_consume_from_two_handles currently performs sequential consumes despite claiming to exercise a race. Run a.consume and b.consume concurrently with asyncio.gather, preserving the existing assertions that exactly one result is NonceConsumeHit and the other is ALREADY_CONSUMED.scripts/prism_lium_attestation_e2e.py (1)
816-819: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRedundant assertion.
The first clause is subsumed by the
startswith("miner_fault")check, and_ingest(expect_score=False)already asserts the same thing. Also, the comment promises a "tampered bundle is forced" case that this code does not exercise (bundle=None).🤖 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 `@scripts/prism_lium_attestation_e2e.py` around lines 816 - 819, Update the tampered-bundle assertion near _ingest to remove the redundant reason check, since _ingest(expect_score=False) already validates the miner-fault outcome. Ensure this test actually forces a tampered bundle instead of passing bundle=None, and revise the misleading comment to match the exercised scenario.packages/challenges/prism/tests/test_prism_no_tee_absence.py (1)
454-468: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winRe-raising defeats the guard's intent.
Any
ResultIngestionError(even a non-tee_requiredone) now propagates and fails the test, so the "never raises tee_required" assertion can never be the reported reason. Prefer failing explicitly with the reason in the message.♻️ Proposed change
except ResultIngestionError as exc: - assert exc.reason != "tee_required" - raise + pytest.fail(f"ingestion raised unexpectedly: {exc.reason}")🤖 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 `@packages/challenges/prism/tests/test_prism_no_tee_absence.py` around lines 454 - 468, Update the ResultIngestionError handling around ingest_work_unit_result so it does not re-raise the exception after checking its reason. Instead, explicitly fail the test with the captured error reason, while preserving the assertion that the reason must not be "tee_required".tests/unit/test_attestation_http.py (1)
171-195: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDead first payload/key setup.
keyfrom the fixture and the initialAttestationPayload(with the placeholder"a"*64response) are both immediately overwritten. Drop the first block.♻️ Proposed cleanup
- key: bytes = harness["key"] - payload = AttestationPayload( - nonce="nonce-1", - digest=DIGEST, - pod_id=BINDING.pod_id, - variant="cuda", - sealed_manifest_hashes=dict(MANIFEST), - build_secret_response="a" * 64, - ) - # build_secret_response must be valid hex from real helper for structure; - # sign with derived key after computing real response. from base.attestation.payload import compute_build_secret_response secret = b"build-secret-fixture" key = derive_attestation_key(secret)🤖 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 `@tests/unit/test_attestation_http.py` around lines 171 - 195, Remove the initial harness key assignment and placeholder AttestationPayload construction in the affected test, retaining only the later key derivation and payload built with compute_build_secret_response. Keep the signing and attestation flow unchanged.tests/unit/test_compute_lium_client.py (1)
896-898: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueTautological assertions.
pytest.raises(LiumAuthError)already guarantees a non-None exception of that type; lines 897-898 add nothing. Keep only thestatus_codecheck.🤖 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 `@tests/unit/test_compute_lium_client.py` around lines 896 - 898, Remove the redundant non-None and type assertions from the pytest.raises validation, retaining only the status_code assertion for the captured exception.packages/challenges/prism/tests/test_prod_constation_http_ingest.py (1)
83-88: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMisleading helper name.
_zip_b64returns raw zip bytes, not base64; it's posted asapplication/octet-stream. Rename to_zip_bytes.🤖 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 `@packages/challenges/prism/tests/test_prod_constation_http_ingest.py` around lines 83 - 88, Rename the helper function _zip_b64 to _zip_bytes to accurately reflect that it returns raw ZIP bytes. Update every reference to this helper, preserving its current byte output and application/octet-stream usage.tests/unit/test_compute_attestation_nonce.py (1)
223-230: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueTest name overstates coverage.
NonceBindingvalidates in its own constructor, so theValueErrorfires in_binding(...)beforesvc.issueis reached; this asserts binding validation, not issue-time rejection. Consider renaming or constructing the invalid binding outsidepytest.raises.🤖 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 `@tests/unit/test_compute_attestation_nonce.py` around lines 223 - 230, The test name and assertions incorrectly attribute constructor validation to svc.issue. Rename test_issue_rejects_blank_binding_fields to reflect NonceBinding validation, or construct each invalid binding before the pytest.raises block so the test specifically exercises issue-time rejection.tests/unit/test_compute_constation_service.py (2)
385-413: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
test_poller_poll_cap_fail_closednever exercises the cap. Withduration_seconds=0.0only start+end run, so themax_polls=2limit is never exceeded and the fail-closed branch is untested. Add a case with a duration that would require interval polls and assert the poll-cap failure reason.🤖 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 `@tests/unit/test_compute_constation_service.py` around lines 385 - 413, The test test_poller_poll_cap_fail_closed currently uses duration_seconds=0.0, so it never exceeds max_polls. Add a duration requiring interval polls, run the poller with the existing max_polls=2 configuration, and assert the result is unsuccessful with the poll-cap failure reason while preserving the existing start/end success assertion if useful.
452-469: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMove
import re(andPath) to module scope. Re-importing inside the loop body adds noise for no benefit.🤖 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 `@tests/unit/test_compute_constation_service.py` around lines 452 - 469, Move the Path and re imports from test_corroboration_module_never_claims_independent to module scope, keeping the test’s existing file-reading loop and regular-expression assertions unchanged.packages/challenges/prism/tests/test_constation_scoring_gate.py (3)
347-347: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueLocal
dataclasses.replaceimports repeated inline.
from dataclasses import replaceis imported inside two separate test bodies instead of once at module scope. Minor, but hoisting it to the top-level imports removes the duplication.Also applies to: 446-446
🤖 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 `@packages/challenges/prism/tests/test_constation_scoring_gate.py` at line 347, Move the dataclasses.replace import from the two test bodies to the module-level imports in test_constation_scoring_gate.py, then remove both inline imports while preserving the existing replace usages in those tests.
192-203: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUnused duplicate of
_ok_checkers.
_fail_manifest_checkersis byte-identical to_ok_checkers(lines 179-189) and no call site references it in this file. Its name is also misleading — sealed-manifest mismatch in the tests is actually forced by mutatingreported_sealed_manifest_hasheson the bundle (see lines 349-352, 448-451), not by this checker set. Consider removing it to avoid confusing future readers.🧹 Proposed removal
-def _fail_manifest_checkers(): - def allow(**_k: Any) -> CheckOutcome: - return CheckOutcome(ok=True, reason="ok") - - def nonce(**_k: Any) -> CheckOutcome: - return CheckOutcome(ok=True, reason="ok") - - def sig(_s: object) -> CheckOutcome: - return CheckOutcome(ok=True, reason="ok") - - return allow, nonce, sig - -🤖 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 `@packages/challenges/prism/tests/test_constation_scoring_gate.py` around lines 192 - 203, Remove the unused, duplicate _fail_manifest_checkers helper and its nested allow, nonce, and sig checkers. Keep _ok_checkers and the existing sealed-manifest mismatch setup that mutates reported_sealed_manifest_hashes unchanged.
363-658: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRepeated boilerplate across the todo-22/23 async tests.
test_valid_bundle_writes_score_with_attestation_mode,test_no_bundle_writes_no_score_miner_fault,test_manifest_mismatch_miner_fault_no_score,test_infra_fault_constation_unavailable_no_score,test_revoked_digest_no_score,test_breakglass_admits_infra_run_and_writes_score, andtest_breakglass_refuses_miner_faultall repeat the same setup: stage train data, monkeypatchDockerExecutor.run, build the app, derive the signer, and seed a submission. Extracting a shared async fixture (e.g. yieldingapp,signer,db_path) would cut a large amount of duplicated setup and make future test additions cheaper.🤖 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 `@packages/challenges/prism/tests/test_constation_scoring_gate.py` around lines 363 - 658, Extract the repeated setup from the seven async tests into a shared async pytest fixture, yielding the initialized app, signer, database path, and seeded submission as appropriate. Update test_valid_bundle_writes_score_with_attestation_mode, test_no_bundle_writes_no_score_miner_fault, test_manifest_mismatch_miner_fault_no_score, test_infra_fault_constation_unavailable_no_score, test_revoked_digest_no_score, test_breakglass_admits_infra_run_and_writes_score, and test_breakglass_refuses_miner_fault to consume the fixture while preserving each test’s distinct hotkey or submission-specific setup.packages/challenges/prism/src/prism_challenge/constation_checkers.py (2)
82-83: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winSilent exception swallowing hurts observability.
Catching bare
Exceptionand returning only a stringified reason, with no logging, means production transport failures (timeouts, DNS errors, 5xx) leave no trace for on-call debugging — this is especially important given they currently get folded into the same fail-closed bucket as genuine miner rejections (see companion comment inconstation.py).🤖 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 `@packages/challenges/prism/src/prism_challenge/constation_checkers.py` around lines 82 - 83, Update the exception handler in the checker method containing the CheckOutcome return to log the caught transport exception with appropriate context before returning the fail-closed outcome. Preserve the existing reason and outcome behavior, while ensuring timeouts, DNS failures, and server errors are visible to operational logs.
73-88: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winReuse a single HTTP client; verify this isn't a blocking call inside an async handler.
Each call to
_post_outcomespins up a brand-newhttpx.Client(Lines 76-78), so a singleconstation_okevaluation opens/tears down up to 3 separate connections instead of reusing one. Store the client (or anhttpx.Client/httpx.AsyncClientinstance) onselfand reuse it across calls, closing it via an explicitclose()/context-manager on theBaseHttpConstationClientitself.Separately: this uses the synchronous
httpx.Client, making a blocking network call. Given the context snippet showsingestion.pycallingconstation_ok(...)withoutawait, please confirm this code path never runs inside anasync defrequest handler — if it does, these sequential blocking calls (up totimeout_seach) would stall the event loop under load.♻️ Reuse client across calls
def __init__( self, *, base_url: str, token: str, timeout_s: float = 10.0, transport: httpx.AsyncBaseTransport | httpx.BaseTransport | None = None, ) -> None: if not base_url.strip(): raise ValueError("constation base_url must be non-empty") self._base_url = base_url.rstrip("/") self._token = token self._timeout_s = timeout_s self._transport = transport + self._client = httpx.Client(timeout=timeout_s, transport=transport) + + def close(self) -> None: + self._client.close() @@ def _post_outcome(self, path: str, body: dict[str, Any]) -> CheckOutcome: url = f"{self._base_url}{path}" try: - with httpx.Client( - timeout=self._timeout_s, transport=self._transport - ) as client: - response = client.post(url, json=body, headers=self._headers()) - response.raise_for_status() - data = response.json() + response = self._client.post(url, json=body, headers=self._headers()) + response.raise_for_status() + data = response.json() except Exception as exc: # noqa: BLE001 - map to checker fail-closed return CheckOutcome(ok=False, reason=f"checker_transport_error:{exc}")🤖 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 `@packages/challenges/prism/src/prism_challenge/constation_checkers.py` around lines 73 - 88, Update BaseHttpConstationClient to create and retain one HTTP client, reuse it in _post_outcome, and close it through the class’s explicit lifecycle or context-manager API. Inspect the constation_ok call path to verify synchronous _post_outcome never runs inside an async request handler; if it does, convert the path to use httpx.AsyncClient and await the network calls.packages/challenges/prism/src/prism_challenge/config.py (1)
305-307: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winConsider file-based loading for
constation_internal_token, consistent with other secrets in this class.Every other internal-auth secret here (
shared_token,docker_broker_token,hf_token) has a companion_filefield plus a resolution helper (internal_token(),hf_token_value()) so production can mount the secret instead of passing it via raw env var.constation_internal_tokencurrently has neither, even though it's used the same way (bearer token for BASE's internal constation endpoints).♻️ Suggested addition mirroring the existing `shared_token_file` pattern
constation_base_url: str | None = None constation_internal_token: str | None = Field(default=None, repr=False) + constation_internal_token_file: str | None = Field(default=None, repr=False) + + def resolved_constation_internal_token(self) -> str | None: + if self.constation_internal_token: + return self.constation_internal_token + if self.constation_internal_token_file and Path(self.constation_internal_token_file).exists(): + return Path(self.constation_internal_token_file).read_text(encoding="utf-8").strip() + return None🤖 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 `@packages/challenges/prism/src/prism_challenge/config.py` around lines 305 - 307, Update the configuration class around constation_internal_token to add a companion file-based secret field and a resolution helper, mirroring the existing shared_token_file/internal_token and hf_token/hf_token_value patterns. Ensure callers use the resolved token so mounted-file secrets take precedence or follow the established precedence behavior, while preserving the current raw-token fallback.packages/challenges/prism/src/prism_challenge/ingestion.py (3)
543-551: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueType
repositoryinstead ofAny.
_reject_no_scoreonly needssubmission_status; annotating with the concrete repository type (as used elsewhere in this module) keeps the helper checkable.🤖 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 `@packages/challenges/prism/src/prism_challenge/ingestion.py` around lines 543 - 551, Update the repository parameter annotation in _reject_no_score from Any to the concrete repository type already used elsewhere in this module, while preserving its existing submission_status usage and behavior.
306-319: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
score_written=Trueon the conflict path is misleading.This call writes nothing; the flag reads as "this ingest wrote a score". Callers distinguishing "score exists" from "score written now" will get the wrong signal. Consider documenting the intended semantics on the field (or setting
Falsehere) so the conflict and idempotent paths are unambiguous.🤖 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 `@packages/challenges/prism/src/prism_challenge/ingestion.py` around lines 306 - 319, Update the conflict-path IngestionOutcome construction to set score_written to False, since this path does not write a score. Preserve the existing outcome fields and semantics for the idempotent and successful ingestion paths.
489-497: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDead conditional in the retry-exhausted branch.
This return is only reachable when
constation_attempt >= max_constation_attempts, so the ternary'selse reasonarm can never be taken. Simplify to avoid implying a second behavior.♻️ Proposed simplification
return _ConstationGate( admit=False, constation_ok=False, - reason=infra_fault_reason(INFRA_FAULT_RETRY_EXHAUSTED) - if constation_attempt >= max_constation_attempts - else reason, + reason=infra_fault_reason(INFRA_FAULT_RETRY_EXHAUSTED), message="infra fault retry budget exhausted; no score", retryable=False, )🤖 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 `@packages/challenges/prism/src/prism_challenge/ingestion.py` around lines 489 - 497, In the retry-exhausted return constructing _ConstationGate, remove the unreachable conditional around reason and always use infra_fault_reason(INFRA_FAULT_RETRY_EXHAUSTED) for the reason field. Keep the existing admission, retryability, message, and other return values unchanged.packages/challenges/prism/tests/test_constation_ok.py (1)
345-352: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAsserting on docstring prose is brittle.
test_module_docstring_states_sole_elevation_predicate_m14breaks on innocuous rewording while proving nothing about behavior. The neighbouringtest_module_exposes_no_tier_grant_apialready enforces the real contract.🤖 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 `@packages/challenges/prism/tests/test_constation_ok.py` around lines 345 - 352, Remove the brittle prose-based test function test_module_docstring_states_sole_elevation_predicate_m14, including its docstring-content assertions. Retain the neighboring test_module_exposes_no_tier_grant_api, which already verifies the intended behavioral contract.packages/challenges/prism/src/prism_challenge/routes.py (1)
49-53: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMid-module import for router wiring.
from .attestation_routes import build_attestation_public_routersits below module-level statements. If it isn't there to break an import cycle, move it to the top with the other imports (and add a comment if it is).🤖 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 `@packages/challenges/prism/src/prism_challenge/routes.py` around lines 49 - 53, Move the build_attestation_public_router import to the top import section in routes.py alongside the other module imports, unless it is required to break an import cycle; if so, retain its position and document that reason with a comment.src/base/master/constation/__init__.py (1)
8-19: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueTest-app builder exported from the production package surface.
create_constation_test_appimports FastAPI and exists for unit tests; re-exporting it in__all__puts a test helper on the public API. Consider having tests import it frombase.master.constation.routesdirectly.🤖 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/base/master/constation/__init__.py` around lines 8 - 19, Remove create_constation_test_app from the production package’s __all__ and its corresponding import in base.master.constation.__init__.py; update tests to import it directly from base.master.constation.routes while preserving build_constation_router’s public export.packages/challenges/prism/src/prism_challenge/attestation_routes.py (1)
101-129: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low value
_signed_from_wireindexes required payload keys directly.
pl_raw["nonce"]etc. raiseKeyError, whichverify_signaturecatches (→signature_error:…) butbuild_constation_router's master-side twin maps to 422. Here the internalverify_attestationroute has no guard, so a malformed body yields a 500. Convert missing keys toValueErrorfor consistent handling.🤖 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 `@packages/challenges/prism/src/prism_challenge/attestation_routes.py` around lines 101 - 129, Update _signed_from_wire to validate all required fields in pl_raw before constructing AttestationPayload, converting missing nonce, digest, pod_id, variant, sealed_manifest_hashes, or build_secret_response keys into ValueError instead of allowing KeyError. Preserve the existing payload parsing and error message handling for valid input.packages/challenges/prism/src/prism_challenge/app.py (1)
88-104: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSilent
except Exception: passhides checker wiring failures.A misconfigured in-process service degrades to the bundle-only path (fail-closed, good) but leaves no trace for operators. Log at warning with the exception.
♻️ Proposed change
- except Exception: - pass + except Exception: # noqa: BLE001 - degrade to fail-closed, but make it visible + logger.warning("in-process constation checkers unavailable", exc_info=True)🤖 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 `@packages/challenges/prism/src/prism_challenge/app.py` around lines 88 - 104, Update the exception handler around make_inprocess_checkers in the app setup flow to log a warning that includes the caught exception, then retain the existing bundle-only fallback behavior.packages/challenges/prism/tests/conftest.py (1)
150-167: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffAutouse patching of a production function is a wide default-on seam.
Every prism test outside
prod_constation*modules gets a valid bundle injected unless it happens to pass one of the three sentinel kwargs. A newly added fail-closed test that forgets the marker will silently pass through the seam. An explicit opt-in fixture (used by the legacy tests) would keep the default honest; at minimum, key the opt-out on the marker only, not on kwarg shape.🤖 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 `@packages/challenges/prism/tests/conftest.py` around lines 150 - 167, The autouse wrapper around ingest_work_unit_result is too broad and can mask fail-closed tests. Replace the default-on injection behavior with an explicit opt-in fixture or marker used by the legacy tests; if retaining the wrapper, gate it solely on that marker rather than checking sentinel kwargs, while preserving the existing production-module exclusions and bound-name patching.src/base/master/constation/routes.py (1)
161-168: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicated phase-normalization logic.
The random/mid→interval remap and
{start,interval,end}validation block is copy-pasted betweenattestation_challengeandissue_nonce. Extract a small_normalize_phase(phase: str) -> strhelper to keep the two code paths in sync.Also applies to: 189-196
🤖 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/base/master/constation/routes.py` around lines 161 - 168, Extract the shared random/mid-to-interval remapping and allowed-phase validation from attestation_challenge and issue_nonce into a _normalize_phase(phase: str) -> str helper. Replace both duplicated blocks with calls to this helper, preserving the existing normalized values and HTTPException behavior.src/base/cli_app/main.py (1)
1154-1158: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winHardcoded nonce TTL instead of a settings-driven value.
DurableAttestationNonceService(session_factory, ttl=timedelta(hours=2))is a magic constant, unlike the neighboringSqlAlchemyMinerNonceStore(..., ttl_seconds=settings.master.upload_nonce_ttl_seconds)pattern in the same function. Consider adding asettings.master.constation_nonce_ttl_seconds(or similar) so ops can tune it without a code change.🤖 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/base/cli_app/main.py` around lines 1154 - 1158, Replace the hardcoded two-hour TTL in the constation nonce service initialization with a settings-driven value, adding and wiring an appropriate master configuration field such as constation_nonce_ttl_seconds and converting it to a timedelta. Follow the existing settings-based nonce TTL pattern used nearby while preserving DurableAttestationNonceService behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: ae4d1413-5961-4d0c-ba03-a31c8b0d7a16
📒 Files selected for processing (59)
alembic/versions/0017_image_digest_allowlist.pyalembic/versions/0018_attestation_nonces.pypackages/challenges/prism/src/prism_challenge/app.pypackages/challenges/prism/src/prism_challenge/attestation_routes.pypackages/challenges/prism/src/prism_challenge/audit.pypackages/challenges/prism/src/prism_challenge/breakglass.pypackages/challenges/prism/src/prism_challenge/config.pypackages/challenges/prism/src/prism_challenge/constation.pypackages/challenges/prism/src/prism_challenge/constation_checkers.pypackages/challenges/prism/src/prism_challenge/ingestion.pypackages/challenges/prism/src/prism_challenge/proof.pypackages/challenges/prism/src/prism_challenge/queue.pypackages/challenges/prism/src/prism_challenge/routes.pypackages/challenges/prism/tests/conftest.pypackages/challenges/prism/tests/test_constation_bundle_wire.pypackages/challenges/prism/tests/test_constation_ok.pypackages/challenges/prism/tests/test_constation_scoring_gate.pypackages/challenges/prism/tests/test_execution_backend_constation_gate.pypackages/challenges/prism/tests/test_lium_client.pypackages/challenges/prism/tests/test_prism_attestation_routes_s8.pypackages/challenges/prism/tests/test_prism_audit_effective_tier.pypackages/challenges/prism/tests/test_prism_no_tee_absence.pypackages/challenges/prism/tests/test_prism_result_ingestion.pypackages/challenges/prism/tests/test_prism_scoring_characterization_baseline.pypackages/challenges/prism/tests/test_prod_constation_http_ingest.pypackages/challenges/prism/tests/test_prod_constation_kwargs.pyscripts/e2e_lium_attestation.pyscripts/prism_lium_attestation_e2e.pysrc/base/attestation/__init__.pysrc/base/attestation/payload.pysrc/base/cli_app/main.pysrc/base/compute/__init__.pysrc/base/compute/attestation_nonce.pysrc/base/compute/constation_corroboration.pysrc/base/compute/constation_custody.pysrc/base/compute/constation_poller.pysrc/base/compute/constation_runner.pysrc/base/compute/constation_types.pysrc/base/compute/digest_allowlist.pysrc/base/compute/lium.pysrc/base/db/__init__.pysrc/base/db/models.pysrc/base/master/app_proxy.pysrc/base/master/challenge_work_source.pysrc/base/master/constation/__init__.pysrc/base/master/constation/allowlist_repository.pysrc/base/master/constation/bundle_store.pysrc/base/master/constation/nonce_repository.pysrc/base/master/constation/routes.pytests/unit/test_attestation_http.pytests/unit/test_attestation_nonce_repository.pytests/unit/test_attestation_payload.pytests/unit/test_compute_attestation_nonce.pytests/unit/test_compute_constation_service.pytests/unit/test_compute_digest_allowlist.pytests/unit/test_compute_lium_client.pytests/unit/test_constation_forward_attach.pytests/unit/test_digest_allowlist_repository.pytests/unit/test_e2e_lium_attestation_offline.py
💤 Files with no reviewable changes (1)
- packages/challenges/prism/tests/test_lium_client.py
| def _test_constation_kwargs(app_settings: PrismSettings, result_payload: object) -> dict: | ||
| """Supply a valid constation bundle only under allow_insecure_signatures (unit tests). | ||
|
|
||
| Production (allow_insecure_signatures=False) never auto-injects — missing bundle | ||
| fail-closes with no score (P1 / todo 22). Result payloads may still carry an | ||
| explicit constation block later; this helper is the test seam only. | ||
| """ | ||
| if not getattr(app_settings, "allow_insecure_signatures", False): | ||
| return {} | ||
| # If the payload already embeds constation markers, do not override. | ||
| if isinstance(result_payload, dict) and result_payload.get("constation_bundle") is not None: | ||
| return {} | ||
| from .constation import CheckOutcome, ConstationBundle | ||
|
|
||
| def _ok(**_k: object) -> CheckOutcome: | ||
| return CheckOutcome(ok=True, reason="ok") | ||
|
|
||
| man = {"route-test-harness.py": "a" * 64} | ||
| digest = "sha256:" + ("11" * 32) | ||
| bundle = ConstationBundle( | ||
| commit_sha="a" * 40, | ||
| tree_sha="b" * 40, | ||
| variant="cuda", | ||
| digest=digest, | ||
| work_unit_id="route-wu", | ||
| miner_hotkey="route-hk", | ||
| pod_id="route-pod", | ||
| nonce="route-nonce", | ||
| signed_attestation={"route": True}, | ||
| expected_sealed_manifest_hashes=dict(man), | ||
| reported_sealed_manifest_hashes=dict(man), | ||
| lium_declared_digest=digest, | ||
| constation_gap_budget_seconds=30.0, | ||
| constation_observed_max_gap_seconds=1.0, | ||
| ) | ||
| return { | ||
| "constation_bundle": bundle, | ||
| "check_allowlist": _ok, | ||
| "check_nonce": _ok, | ||
| "verify_constation_signature": lambda _s: _ok(), | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Test-only bundle injection lives in production code guarded solely by a settings flag.
_test_constation_kwargs fabricates an always-valid constation bundle whenever allow_insecure_signatures is true. A single misconfiguration in a deployed environment silently disables the entire P1 fail-closed gate. Consider asserting a non-production marker as well (e.g. also require a dedicated test-mode setting), or moving this seam into the test conftest, which already provides _auto_constation_for_legacy_ingestion.
🤖 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 `@packages/challenges/prism/src/prism_challenge/app.py` around lines 106 - 146,
Restrict `_test_constation_kwargs` to an explicit non-production/test-mode
setting in addition to `allow_insecure_signatures`, or remove the production
helper and reuse the existing conftest `_auto_constation_for_legacy_ingestion`
seam. Ensure deployed configurations cannot trigger fabricated constation bundle
injection through the insecure-signature flag alone.
| def verify_signature(signed: object) -> CheckOutcome: | ||
| if verify_attestation_payload is None: | ||
| return CheckOutcome(ok=False, reason="attestation_helpers_missing") | ||
| if verify_key is None: | ||
| # Soft path for harnesses that don't sign: accept dict markers. | ||
| if isinstance(signed, dict) and signed.get("sig"): | ||
| return CheckOutcome(ok=True, reason="ok") | ||
| return CheckOutcome(ok=False, reason="empty_key") |
There was a problem hiding this comment.
🔒 Security & Privacy | 🔴 Critical | ⚡ Quick win
Signature verification is bypassable when attestation_verify_key is unset.
With no key on app.state, any bundle whose signed_attestation is a dict with a truthy sig passes as ok=True. app.py::_constation_ingest_kwargs wires these exact checkers into production ingest whenever constation_base_url/token are not configured, so a miner can forge {"sig": "x"} and satisfy the signature mechanism of constation_ok. This "soft path for harnesses" must not be reachable from the production checker set — fail closed with empty_key when no key is configured.
🔒 Proposed fix
if verify_key is None:
- # Soft path for harnesses that don't sign: accept dict markers.
- if isinstance(signed, dict) and signed.get("sig"):
- return CheckOutcome(ok=True, reason="ok")
return CheckOutcome(ok=False, reason="empty_key")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def verify_signature(signed: object) -> CheckOutcome: | |
| if verify_attestation_payload is None: | |
| return CheckOutcome(ok=False, reason="attestation_helpers_missing") | |
| if verify_key is None: | |
| # Soft path for harnesses that don't sign: accept dict markers. | |
| if isinstance(signed, dict) and signed.get("sig"): | |
| return CheckOutcome(ok=True, reason="ok") | |
| return CheckOutcome(ok=False, reason="empty_key") | |
| def verify_signature(signed: object) -> CheckOutcome: | |
| if verify_attestation_payload is None: | |
| return CheckOutcome(ok=False, reason="attestation_helpers_missing") | |
| if verify_key is None: | |
| return CheckOutcome(ok=False, reason="empty_key") |
🤖 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 `@packages/challenges/prism/src/prism_challenge/attestation_routes.py` around
lines 188 - 195, Update verify_signature in the attestation checker to fail
closed whenever verify_key is unset: return CheckOutcome(ok=False,
reason="empty_key") without accepting signed dict markers. Preserve normal
verification through verify_attestation_payload when a key is configured.
| @public_route(tags=["attestation"]) | ||
| @router.post("/attestation/answer") | ||
| async def attestation_answer(request: Request, body: AnswerBody) -> dict[str, str]: | ||
| ensure_default_constation_services(request.app) | ||
| answers: list[dict[str, Any]] = request.app.state.attestation_answers | ||
| answers.append(body.model_dump()) | ||
| return {"status": "accepted"} |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Unauthenticated public endpoint appends to an unbounded in-memory list.
attestation_answer is a public_route with no rate limit or cap; app.state.attestation_answers grows without bound per request, giving anyone a trivial memory-exhaustion vector. Bound the list (e.g. collections.deque(maxlen=...)) or persist/drop instead of accumulating.
🛡️ Proposed direction
- if getattr(state, "attestation_answers", None) is None:
- state.attestation_answers = []
+ if getattr(state, "attestation_answers", None) is None:
+ state.attestation_answers = deque(maxlen=1024)🤖 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 `@packages/challenges/prism/src/prism_challenge/attestation_routes.py` around
lines 309 - 315, Bound storage in attestation_answer so repeated unauthenticated
requests cannot grow request.app.state.attestation_answers without limit.
Initialize or replace the collection with a bounded deque using an appropriate
maxlen, while preserving body.model_dump() recording and the accepted response.
| @router.post( | ||
| "/internal/v1/constation/verify_attestation", | ||
| dependencies=[Depends(authenticate_internal)], | ||
| ) | ||
| async def verify_attestation( | ||
| request: Request, body: VerifyAttestationBody | ||
| ) -> dict[str, Any]: | ||
| if body.key_hex: | ||
| request.app.state.attestation_verify_key = bytes.fromhex(body.key_hex) | ||
| checkers = make_inprocess_checkers(request.app) | ||
| outcome = checkers["verify_constation_signature"](body.signed) | ||
| return {"ok": outcome.ok, "reason": outcome.reason} |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
verify_attestation mutates the process-wide verify key from request input.
request.app.state.attestation_verify_key = bytes.fromhex(body.key_hex) persists beyond this request, so an internal caller permanently changes the key used by ingestion's in-process checkers. Two issues: (1) the key should be scoped to this call, not stored on shared state; (2) bytes.fromhex on malformed input raises ValueError → 500 instead of 422.
🤖 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 `@packages/challenges/prism/src/prism_challenge/attestation_routes.py` around
lines 372 - 383, Update verify_attestation so any key supplied via body.key_hex
is used only for this invocation and never assigned to
request.app.state.attestation_verify_key; pass the request-scoped key through
the checker construction or verification path instead. Validate malformed
hexadecimal input and convert the resulting ValueError into the endpoint’s
normal 422 validation response.
| def constation_ok( | ||
| bundle: ConstationBundle, | ||
| *, | ||
| check_allowlist: AllowlistChecker, | ||
| check_nonce: NonceChecker, | ||
| verify_signature: SignatureVerifier, | ||
| ) -> ConstationResult: | ||
| """Return whether ``bundle`` satisfies every elevation requirement. | ||
|
|
||
| **M14 — sole elevation predicate.** No other Prism path may grant a tier. | ||
| This function returns a structured :class:`ConstationResult`; it does not | ||
| itself assign tiers. Downstream ``effective_tier`` wiring (separate todo) | ||
| must call this predicate and must not bypass it. | ||
|
|
||
| Check order (first failure wins, distinct reason each): | ||
|
|
||
| 1. allowlist hit | ||
| 2. nonce valid | ||
| 3. signature valid | ||
| 4. sealed manifest hashes match | ||
| 5. corroboration not contradicting (negative only) | ||
| 6. constation gap within budget | ||
|
|
||
| All checkers are injected so unit tests never need live Lium, BASE DB, or | ||
| network I/O. | ||
| """ | ||
| allow = check_allowlist( | ||
| digest=bundle.digest, | ||
| commit_sha=bundle.commit_sha, | ||
| tree_sha=bundle.tree_sha, | ||
| variant=bundle.variant, | ||
| ) | ||
| if not allow.ok: | ||
| mapped = _ALLOWLIST_MAP.get( | ||
| allow.reason, ConstationFailReason.ALLOWLIST_FAILED | ||
| ) | ||
| return ConstationResult(ok=False, reason=mapped, detail=allow.reason) | ||
|
|
||
| nonce = check_nonce( | ||
| nonce=bundle.nonce, | ||
| work_unit_id=bundle.work_unit_id, | ||
| miner_hotkey=bundle.miner_hotkey, | ||
| pod_id=bundle.pod_id, | ||
| ) | ||
| if not nonce.ok: | ||
| mapped = _NONCE_MAP.get(nonce.reason, ConstationFailReason.NONCE_FAILED) | ||
| return ConstationResult(ok=False, reason=mapped, detail=nonce.reason) | ||
|
|
||
| sig = verify_signature(bundle.signed_attestation) | ||
| if not sig.ok: | ||
| return ConstationResult( | ||
| ok=False, | ||
| reason=ConstationFailReason.SIGNATURE_INVALID, | ||
| detail=sig.reason, | ||
| ) | ||
|
|
||
| if not _manifests_match( | ||
| bundle.expected_sealed_manifest_hashes, | ||
| bundle.reported_sealed_manifest_hashes, | ||
| ): | ||
| return ConstationResult( | ||
| ok=False, | ||
| reason=ConstationFailReason.SEALED_MANIFEST_MISMATCH, | ||
| ) | ||
|
|
||
| # Corroboration is negative-only: mismatch fails; absence or agreement | ||
| # does not grant elevation by itself (other checks already required). | ||
| if bundle.lium_declared_digest is not None: | ||
| declared = bundle.lium_declared_digest.strip().lower() | ||
| sidecar = bundle.digest.strip().lower() | ||
| if declared != sidecar: | ||
| return ConstationResult( | ||
| ok=False, | ||
| reason=ConstationFailReason.CORROBORATION_MISMATCH, | ||
| detail=f"lium={declared} sidecar={sidecar}", | ||
| ) | ||
|
|
||
| if bundle.constation_observed_max_gap_seconds > bundle.constation_gap_budget_seconds: | ||
| return ConstationResult( | ||
| ok=False, | ||
| reason=ConstationFailReason.CONSTATION_GAP, | ||
| detail=( | ||
| f"observed={bundle.constation_observed_max_gap_seconds}" | ||
| f" budget={bundle.constation_gap_budget_seconds}" | ||
| ), | ||
| ) | ||
|
|
||
| return ConstationResult(ok=True, reason=ConstationFailReason.OK) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate and inspect the relevant module and surrounding logic.
fd -a 'constation.py|constation_checkers.py' . | sed 's#^\./##'
echo '--- constellation.py outline ---'
ast-grep outline packages/challenges/prism/src/prism_challenge/constation.py || true
echo '--- constation.py relevant lines ---'
cat -n packages/challenges/prism/src/prism_challenge/constation.py | sed -n '1,440p'
echo '--- constation_checkers.py relevant lines ---'
cat -n packages/challenges/prism/src/prism_challenge/constation_checkers.py | sed -n '1,420p'Repository: BaseIntelligence/base
Length of output: 26780
Distinguish checker transport failures from miner-attributable check failures.
constation_ok maps unknown allowlist/nonce failures to generic miner buckets, and any failed signature outcome to SIGNATURE_INVALID; classify_constation_fault then returns miner_fault:<code> for all of them. The http client adapters can emit transport/malformed-rejected reasons for 5xx, timeouts, or invalid JSON, which should be classified as infra faults such as constation_unavailable / lium_5xx / network_partition instead of miner faults.
🤖 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 `@packages/challenges/prism/src/prism_challenge/constation.py` around lines 187
- 274, Update constation_ok and classify_constation_fault so checker transport
or malformed-response reasons from check_allowlist, check_nonce, and
verify_signature are preserved and classified as infrastructure faults rather
than mapped to miner failures. Keep genuine miner-attributable failures mapped
to the existing allowlist, nonce, and signature reasons, while recognizing the
adapters’ unavailable, 5xx, timeout, and invalid-JSON reason codes as
constation_unavailable, lium_5xx, or network_partition as appropriate.
| def _normalize_digest(value: Any) -> str | None: | ||
| return _optional_str(value) | ||
|
|
||
|
|
||
| def _digests_allow_reuse(existing: str | None, requested: str | None) -> bool: | ||
| """Reuse only when digests agree, or neither side pins a digest. | ||
|
|
||
| A requested pin against a missing or different stored digest is a hard | ||
| mismatch — never silent name-only reuse under a new pin. | ||
| """ | ||
| if requested is None and existing is None: | ||
| return True | ||
| if requested is None: | ||
| # Caller did not pin; reusing a digest-bearing template is still name-only | ||
| # but does not introduce a false pin claim. Allowed for legacy callers. | ||
| return True | ||
| return existing is not None and existing == requested |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
_normalize_digest doesn't normalize — case differences become hard mismatches.
It is a pure alias for _optional_str, so the comparison at Line 558 is case-sensitive exact string equality. A stored SHA256:ABC… vs requested sha256:abc… raises LiumTemplateDigestMismatchError and blocks provisioning. Note evaluate_corroboration in src/base/compute/constation_corroboration.py Line 57 does lowercase before comparing, so the two digest comparisons in this stack disagree.
🐛 Proposed fix
def _normalize_digest(value: Any) -> str | None:
- return _optional_str(value)
+ text = _optional_str(value)
+ return text.lower() if text is not None else None📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def _normalize_digest(value: Any) -> str | None: | |
| return _optional_str(value) | |
| def _digests_allow_reuse(existing: str | None, requested: str | None) -> bool: | |
| """Reuse only when digests agree, or neither side pins a digest. | |
| A requested pin against a missing or different stored digest is a hard | |
| mismatch — never silent name-only reuse under a new pin. | |
| """ | |
| if requested is None and existing is None: | |
| return True | |
| if requested is None: | |
| # Caller did not pin; reusing a digest-bearing template is still name-only | |
| # but does not introduce a false pin claim. Allowed for legacy callers. | |
| return True | |
| return existing is not None and existing == requested | |
| def _normalize_digest(value: Any) -> str | None: | |
| text = _optional_str(value) | |
| return text.lower() if text is not None else None | |
| def _digests_allow_reuse(existing: str | None, requested: str | None) -> bool: | |
| """Reuse only when digests agree, or neither side pins a digest. | |
| A requested pin against a missing or different stored digest is a hard | |
| mismatch — never silent name-only reuse under a new pin. | |
| """ | |
| if requested is None and existing is None: | |
| return True | |
| if requested is None: | |
| # Caller did not pin; reusing a digest-bearing template is still name-only | |
| # but does not introduce a false pin claim. Allowed for legacy callers. | |
| return True | |
| return existing is not None and existing == requested |
🤖 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/base/compute/lium.py` around lines 542 - 558, Update _normalize_digest to
canonicalize digest strings consistently with evaluate_corroboration, including
lowercasing the normalized value before _digests_allow_reuse compares existing
and requested digests. Preserve None handling and the existing mismatch behavior
for genuinely different digest values.
| if "constation_bundle" not in payload and self._bundle_lookup is not None: | ||
| looked = self._bundle_lookup(work_unit_id) | ||
| if hasattr(looked, "__await__"): | ||
| looked = await looked | ||
| if isinstance(looked, dict): | ||
| payload["constation_bundle"] = looked |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
No exception isolation around bundle_lookup.
If bundle_lookup raises (it's documented as a general async-capable callable, not necessarily the current trivial dict lookup), the exception propagates and aborts forward_result entirely — even though a missing bundle is a legitimate, expected outcome that should just leave constation_bundle unset (Prism already fail-closes on a missing bundle). Wrap the lookup so bundle-lookup failures don't block forwarding the underlying result.
🛡️ Proposed fix
if "constation_bundle" not in payload and self._bundle_lookup is not None:
- looked = self._bundle_lookup(work_unit_id)
- if hasattr(looked, "__await__"):
- looked = await looked
- if isinstance(looked, dict):
- payload["constation_bundle"] = looked
+ try:
+ looked = self._bundle_lookup(work_unit_id)
+ if hasattr(looked, "__await__"):
+ looked = await looked
+ except Exception: # noqa: BLE001 - bundle attach must not block forwarding
+ looked = None
+ if isinstance(looked, dict):
+ payload["constation_bundle"] = looked📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if "constation_bundle" not in payload and self._bundle_lookup is not None: | |
| looked = self._bundle_lookup(work_unit_id) | |
| if hasattr(looked, "__await__"): | |
| looked = await looked | |
| if isinstance(looked, dict): | |
| payload["constation_bundle"] = looked | |
| if "constation_bundle" not in payload and self._bundle_lookup is not None: | |
| try: | |
| looked = self._bundle_lookup(work_unit_id) | |
| if hasattr(looked, "__await__"): | |
| looked = await looked | |
| except Exception: # noqa: BLE001 - bundle attach must not block forwarding | |
| looked = None | |
| if isinstance(looked, dict): | |
| payload["constation_bundle"] = looked |
🤖 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/base/master/challenge_work_source.py` around lines 444 - 449, Update the
bundle lookup flow in forward_result around self._bundle_lookup so exceptions
from either synchronous or awaited lookup execution are caught and treated as a
missing bundle. Preserve successful dict results by setting
payload["constation_bundle"], but leave the key unset on lookup failure so
forwarding continues.
| async def register(self, record: DigestRecord) -> None: | ||
| """Insert a binding; identical re-register is a no-op. | ||
|
|
||
| Conflicting rebind (same digest, different commit/tree/variant) raises | ||
| ValueError (matches pure DigestAllowlist.register). | ||
| """ | ||
| async with self._session_scope(self._session_factory) as session: | ||
| row = ( | ||
| await session.execute( | ||
| select(ImageDigestAllowlistEntry).where( | ||
| ImageDigestAllowlistEntry.digest == record.digest | ||
| ) | ||
| ) | ||
| ).scalar_one_or_none() | ||
| if row is not None: | ||
| bound = DigestRecord( | ||
| commit_sha=row.commit_sha, | ||
| tree_sha=row.tree_sha, | ||
| variant=row.variant, | ||
| digest=row.digest, | ||
| ) | ||
| if bound != record: | ||
| raise ValueError( | ||
| f"digest {record.digest!r} already bound to " | ||
| f"commit={bound.commit_sha} tree={bound.tree_sha} " | ||
| f"variant={bound.variant.value}; cannot rebind to " | ||
| f"commit={record.commit_sha} tree={record.tree_sha} " | ||
| f"variant={record.variant.value}" | ||
| ) | ||
| return | ||
| session.add( | ||
| ImageDigestAllowlistEntry( | ||
| commit_sha=record.commit_sha, | ||
| tree_sha=record.tree_sha, | ||
| variant=record.variant.value, | ||
| digest=record.digest, | ||
| ) | ||
| ) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Second unique constraint conflict is unhandled.
ImageDigestAllowlistEntry also enforces uq_image_digest_allowlist_commit_tree_variant. register() only pre-checks conflicts by digest; if a new digest is registered for a (commit_sha, tree_sha, variant) already bound to a different digest, the DB will raise an IntegrityError on flush/commit that this method never catches — surfacing as an unhandled 500 from register_digest rather than the documented ValueError ("matches pure DigestAllowlist.register"). Recommend also checking/handling this constraint (query by the triple, or catch IntegrityError and translate to ValueError).
🤖 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/base/master/constation/allowlist_repository.py` around lines 59 - 96, The
register method must handle conflicts on the commit_sha/tree_sha/variant
uniqueness constraint, not only digest conflicts. In register, check for an
existing ImageDigestAllowlistEntry with the same triple and raise the documented
ValueError when its digest differs; also translate any race-condition
IntegrityError during persistence into the same ValueError behavior, while
preserving identical re-registration as a no-op.
| class ConstationBundleStore: | ||
| """Keyed by work_unit_id; last-write wins. Forwarder attaches stored bundles.""" | ||
|
|
||
| def __init__(self) -> None: | ||
| self._by_wu: dict[str, dict[str, Any]] = {} | ||
|
|
||
| def put(self, work_unit_id: str, bundle: dict[str, Any]) -> None: | ||
| key = work_unit_id.strip() | ||
| if not key: | ||
| raise ValueError("work_unit_id must be non-empty") | ||
| if not isinstance(bundle, dict): | ||
| raise TypeError("bundle must be a dict") | ||
| self._by_wu[key] = copy.deepcopy(bundle) | ||
|
|
||
| def get(self, work_unit_id: str) -> dict[str, Any] | None: | ||
| key = work_unit_id.strip() | ||
| blob = self._by_wu.get(key) | ||
| return copy.deepcopy(blob) if blob is not None else None | ||
|
|
||
| def pop(self, work_unit_id: str) -> dict[str, Any] | None: | ||
| key = work_unit_id.strip() | ||
| blob = self._by_wu.pop(key, None) | ||
| return copy.deepcopy(blob) if blob is not None else None | ||
|
|
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Unbounded growth: bundles are never evicted.
Entries are only ever added (put) or read (get); the wired call site (cli_app/main.py's _bundle_lookup) uses get, never pop, and there's no TTL/eviction pass anywhere in this cohort. Over a long-running master process this dict grows by one entry per work unit indefinitely, risking a slow memory leak/OOM.
Consider a bounded cache (e.g., TTL-based expiry alongside the nonce lifetime, or evicting on pop once a result has been durably forwarded/acknowledged).
🤖 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/base/master/constation/bundle_store.py` around lines 9 - 32, Bound the
storage managed by ConstationBundleStore so entries cannot accumulate
indefinitely. Update the store’s put/get flow to apply an eviction policy,
preferably TTL expiry aligned with the nonce lifetime, or ensure _bundle_lookup
uses pop only after durable forwarding/acknowledgement; preserve deep-copy and
key-validation behavior while removing expired or consumed bundles.
| class AnswerBody(BaseModel): | ||
| model_config = {"extra": "allow"} | ||
|
|
||
| nonce: str = Field(min_length=1) | ||
| phase: str | None = None | ||
|
|
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Unauthenticated /v1/attestation/answer stores unbounded arbitrary payloads in memory.
attestation_answer has no auth dependency (by design — it's the public sidecar route) and AnswerBody allows arbitrary extra fields (extra: "allow"). Every accepted answer is appended to answers forever with no cap or expiry, and is exposed via router._answers. An attacker can spam this route with large/unbounded bodies to grow master memory unboundedly (DoS).
Cap the retained history (e.g., bounded ring buffer per binding) or drop the accumulation entirely if it's only a test/debug hook not consumed by production logic.
Also applies to: 178-181, 302-303
🤖 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/base/master/constation/routes.py` around lines 70 - 75, Bound or remove
the retained answer history used by AnswerBody and attestation_answer: replace
the unbounded answers accumulation with a fixed-size ring buffer per binding, or
eliminate it if it is only a test/debug hook. Ensure accepted requests cannot
retain arbitrary extra payloads indefinitely, and update the related
router._answers exposure and handling at the referenced code paths to preserve
only the intended bounded/debug behavior.
Unblocks publishing the constation/attestation modules merged in #41/#42, which no released wheel currently contains. Aligns pyproject, release manifest, and uv.lock so github-release identity asserts pass for tag v3.2.0. Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
2 participants
Summary
Wires production constation A→Z so Prism miners can score with image attestation fail-closed (no TEE claims).
Architecture
/challenges/prism/v1/attestation/*— not as master-owned challenge routes.result.constation_bundlewhen present; Prism re-verifies at ingest (never trusts a bareconstation_okflag).constation_okonly.Commits
feat(compute)— pure attestation/constation librariesfeat(db)— allowlist + nonce tables + alembicfeat(master)— durable hosts, internal router, bundle_lookup bootfeat(prism)— public attestation routes + production ingest kwargstest(constation)— S1/S3/S8 + unit coveragechore(scripts)— offline e2e harnessesVerification (local)
Out of scope / residual
Test plan
test_prism_attestation_routes_s8challenge/answer + nonce consumetest_prod_constation_http_ingestS1 honest score + S3 missing bundle fail-closedtest_attestation_httpmaster durable SoT surfacestest_constation_forward_attachbundle embed on forwardSummary by CodeRabbit
New Features
Bug Fixes