feat(api): cancel authorized exports on loopback HTTP - #445
Conversation
POST /v1/exports/{export_id}/cancel removes one metric-free naruon identity
from AnalysisRunLiveService. Receipts stay cancelled=true. LineageWeave is
refused. NaruonLiveService stays POST-only. ADR 0077.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Team Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| let stored = self | ||
| .authorized_exports | ||
| .remove(&replay_key) | ||
| .ok_or(ApiError::InvalidWirePayload)?; | ||
| let cancelled = ExportCancelled::from_retrieval(stored.retrieval)?; |
There was a problem hiding this comment.
🔴 Failed cancellation still removes export
A slash-bearing authorization key makes from_retrieval fail after the export is removed. The client receives 400, but the export disappears permanently.
Prompt for agents
Make cancel_export atomic and compatible with every export accepted by accept_export. Currently idempotency-key values containing '/' or NUL pass authorization and ExportRetrieval validation, but ExportCancelled::from_retrieval rejects them after exports_by_id and authorized_exports have already been mutated. Validate and serialize the cancellation receipt before changing either map, then commit both removals only after all fallible work succeeds. Also align ExportCancelled validation with the accepted ExportRetrieval contract so an authorized export cannot become uncancellable.
Was this helpful? React with 👍 or 👎 to provide feedback.
| require_nonempty(&self.decision_code)?; | ||
| require_nonempty(&self.purpose)?; | ||
| require_nonempty(&self.idempotency_key)?; |
There was a problem hiding this comment.
🟡 Invalid cancellation receipts pass validation
Untrusted JSON with any nonempty purpose or decision passes validate. Consumers can accept fabricated authorization metadata as a valid cancellation.
Prompt for agents
Strengthen ExportCancelled validation to preserve the semantic guarantees of the ExportRetrieval from which receipts are derived. from_json currently accepts any nonempty purpose and decision_code, while a legitimate cancel receipt can only represent the known modular_service_consumer purpose and the allowed purpose_bound_export_allowed decision. Validate those exact values, and add contract tests showing unknown purposes and denied decision codes are rejected.
Was this helpful? React with 👍 or 👎 to provide feedback.
| pub struct ExportCancelled { | ||
| /// Opaque server-assigned export identity. | ||
| pub export_id: String, | ||
| /// Opaque artifact identity that was authorized. | ||
| pub artifact_id: String, | ||
| /// Stable machine-readable authorization decision code. | ||
| pub decision_code: String, | ||
| /// Declared analytical purpose as a wire name. | ||
| pub purpose: String, | ||
| /// Exact per-export idempotency key that minted this identity. | ||
| pub idempotency_key: String, | ||
| /// Always `true` on a successful cancel receipt. | ||
| pub cancelled: bool, |
There was a problem hiding this comment.
🔍 Cancellation payload lacks versioning
ExportCancelled exposes no semantic version field or constant. Future payload changes cannot follow the repository’s explicit wire-versioning policy.
Was this helpful? React with 👍 or 👎 to provide feedback.
| /// Extract the opaque export identity from `POST /v1/exports/{export_id}/cancel`. | ||
| pub use export_cancel_http::export_cancel_path_id; |
There was a problem hiding this comment.
🔍 Routing parser becomes public API
export_cancel_path_id exposes a server-routing detail while the retrieval equivalent stays private. This creates an unnecessary compatibility obligation.
Was this helpful? React with 👍 or 👎 to provide feedback.
| let consumer = require_headers(headers, self.bound_addr, false)?; | ||
| if consumer != NARUON_CONSUMER_CODE { | ||
| return Err(ApiError::InvalidWirePayload); | ||
| } |
There was a problem hiding this comment.
🟥 Local callers can cancel others' exports
The endpoint trusts tepp-consumer: naruon without caller proof or export ownership. Any local process holding an export ID can revoke it.
Was this helpful? React with 👍 or 👎 to provide feedback.
| let export_id = decode_path_segment(encoded)?; | ||
| require_nonempty(&export_id)?; | ||
| if export_id.contains('/') || export_id.contains('\0') { | ||
| return Err(ApiError::InvalidWirePayload); | ||
| } | ||
| if export_id.len() > EXPORT_CANCEL_ID_MAX_LEN { |
There was a problem hiding this comment.
🟨 Oversized encoded identities decode fully
decode_path_segment allocates and decodes the whole segment before enforcing 128 bytes. Repeated requests waste memory and CPU on invalid identities.
Was this helpful? React with 👍 or 👎 to provide feedback.
|
Closing as unsafe mutation lineage; replacement authority is #174. Review proved both a security root cause (any local caller can claim |
1 participant
Summary
GAP-003A unique operator-visible slice: loopback
POST /v1/exports/{export_id}/cancelonAnalysisRunLiveService/tepp-loopbackremoves one authorized naruon export identity.idempotency-key/ extra path segments / slash/NUL identities / credential headers fail closed.cancelled=true. Tenant, principal, source text, RMSE, bias, coverage, SE-gate, andtepp.scientific_acceptance.v1never appear.feat/export-collection-get-gap-003a). Does not unstack onto main.Explicitly not
Another export collection CLI (#444), export collection GET (#443), interpretation-run cancel CLI (#442), interpretation-run cancel HTTP (#440), interpretation-run retrieval CLI (#439), interpretation-run GET-by-id (#438), interpretation-run collection CLI (#436), export-retrieval CLI (#417), export retrieval GET (#411), export-authorize CLI (#410), analysis-run cancel (#361), Leiden, Driver p.16, GAP-010 Figma/export, persistence/Compose (#287).
Does not add GET to
NaruonLiveService. Does not open LineageWeave on this naruon-owned adapter. Does not weaken fail-closed. Does not infer causality. HTTP 200 is not an ADR 0014 claim.Test plan
cargo test -p tepp_api --lib export_cancelcargo test -p tepp_api --lib handler_covers_metric_free_export_cancelcargo test -p tepp_api --test export_cancel_http_contractcargo clippy -p tepp_api --all-targets -- -D warningscargo doc -p tepp_api --no-depspython3 scripts/validate_documentation.pypython3 scripts/check_docstrings.py