feat(api): enumerate authorized exports via loopback collection GET - #443
feat(api): enumerate authorized exports via loopback collection GET#443seonghobae wants to merge 1 commit into
Conversation
GAP-003A unique slice stacked on export retrieval GET: loopback GET /v1/exports lists metric-free purpose-bound identities on AnalysisRunLiveService / tepp-loopback. LineageWeave refused. NaruonLiveService stays POST-only. ADR 0075.
|
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 end = (start + limit).min(items.len()); | ||
| let next_cursor = (end < items.len()).then(|| items[end - 1].export_id.clone()); | ||
| (items[start..end].to_vec(), next_cursor) |
There was a problem hiding this comment.
🟡 Invalid page limits panic callers
Calling page_export_collection_items with zero and remaining items indexes below zero. Large limits can also overflow, crashing direct library consumers.
Prompt for agents
Make page_export_collection_items safe for every public input. The exported helper currently accepts an arbitrary usize, although only the HTTP parser enforces 1..=64. Zero can underflow end - 1 and large values can overflow start + limit. Either return Result and validate the public limit or implement checked/saturating bounds while defining zero-limit cursor behavior. Add direct helper tests for zero, usize::MAX, empty input, and cursors at or beyond the end.
Was this helpful? React with 👍 or 👎 to provide feedback.
| pub fn new(items: Vec<ExportRetrieval>, next_cursor: Option<String>) -> Result<Self, ApiError> { | ||
| if items.len() > EXPORT_COLLECTION_MAX_LIMIT { | ||
| return Err(ApiError::LimitExceeded); | ||
| } | ||
| if let Some(cursor) = next_cursor.as_deref() { | ||
| require_nonempty(cursor)?; | ||
| if cursor.len() > EXPORT_COLLECTION_CURSOR_MAX_LEN { | ||
| return Err(ApiError::LimitExceeded); | ||
| } | ||
| if cursor.contains('/') || cursor.contains('\0') { | ||
| return Err(ApiError::InvalidWirePayload); | ||
| } | ||
| } | ||
| let collection = Self { items, next_cursor }; | ||
| let payload = to_json(&collection)?; | ||
| require_byte_limit(&payload, DEFAULT_ANALYSIS_RUN_BYTE_LIMIT)?; | ||
| refuse_metrics_on_export_retrieval_payload(&payload)?; | ||
| Ok(collection) |
There was a problem hiding this comment.
🟡 Invalid receipts enter valid collections
ExportCollection::new accepts malformed public receipt values without item validation. Callers can publish denied decisions, empty identities, or unsupported versions as valid pages.
Prompt for agents
Validate every ExportRetrieval in ExportCollection::new and to_json before accepting or serializing the page. ExportRetrieval currently keeps validate private, so expose an appropriate crate-level validation method or reconstruct each item through its validated API. Also provide a validated collection deserialization entry point if collections are consumed from JSON. Test malformed decision codes, contract versions, empty fields, unknown purposes, and oversized identities.
Was this helpful? React with 👍 or 👎 to provide feedback.
| let (page, next_cursor) = page_export_collection_items(items, cursor.as_deref(), limit); | ||
| let collection = ExportCollection::new(page, next_cursor)?; | ||
| Ok(json_response(200, "OK", collection.to_json()?)) |
There was a problem hiding this comment.
🟡 Valid maximum pages return errors
With 64 valid receipts containing heavily escaped identifiers, page_export_collection_items builds a response beyond 64 KiB. The accepted maximum limit then returns 413.
Prompt for agents
Build collection pages against both the requested item count and DEFAULT_ANALYSIS_RUN_BYTE_LIMIT. If the requested rows exceed the serialized response bound, return the largest nonempty prefix that fits and set next_cursor to its last export_id. Ensure a single valid receipt always fits, and add an endpoint test with 64 receipts whose artifact IDs and idempotency keys require maximal JSON escaping.
Was this helpful? React with 👍 or 👎 to provide feedback.
| let limit = | ||
| parse_export_collection_page_limit(headers.get("tepp-page-limit").map(String::as_str))?; | ||
| let cursor = parse_export_collection_page_cursor( | ||
| headers.get("tepp-page-cursor").map(String::as_str), | ||
| )?; |
There was a problem hiding this comment.
🔍 Unknown headers bypass collection contract
list_exports ignores arbitrary and misspelled pagination headers. ADR 0075 requires unknown collection keys to fail closed, so these requests need rejection.
Was this helpful? React with 👍 or 👎 to provide feedback.
| items.sort_by(|left, right| left.export_id.cmp(&right.export_id)); | ||
| let start = cursor.map_or(0, |cursor| { | ||
| items | ||
| .iter() | ||
| .position(|item| item.export_id.as_str() > cursor) | ||
| .unwrap_or(items.len()) | ||
| }); |
There was a problem hiding this comment.
📝 Info: Hash order does not affect pagination
page_export_collection_items sorts every receipt by its canonical export identity before applying the cursor. Unordered map iteration therefore cannot reorder pages.
Was this helpful? React with 👍 or 👎 to provide feedback.
| let items = self | ||
| .authorized_exports | ||
| .values() | ||
| .map(|stored| stored.retrieval.clone()) | ||
| .collect(); | ||
| let (page, next_cursor) = page_export_collection_items(items, cursor.as_deref(), limit); |
There was a problem hiding this comment.
📝 Info: Current service avoids page races
list_exports clones one current view while the single-request service holds exclusive access. Export insertion cannot interleave during page construction.
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); | ||
| } | ||
| if headers.contains_key("idempotency-key") { | ||
| return Err(ApiError::InvalidWirePayload); | ||
| } | ||
| let limit = | ||
| parse_export_collection_page_limit(headers.get("tepp-page-limit").map(String::as_str))?; | ||
| let cursor = parse_export_collection_page_cursor( | ||
| headers.get("tepp-page-cursor").map(String::as_str), | ||
| )?; | ||
| let items = self | ||
| .authorized_exports | ||
| .values() | ||
| .map(|stored| stored.retrieval.clone()) | ||
| .collect(); | ||
| let (page, next_cursor) = page_export_collection_items(items, cursor.as_deref(), limit); | ||
| let collection = ExportCollection::new(page, next_cursor)?; |
There was a problem hiding this comment.
🟥 Export capabilities exposed without authorization
Any local process claiming naruon can enumerate all export capabilities. This bypasses per-export capability possession and exposes every artifact receipt across tenants.
Was this helpful? React with 👍 or 👎 to provide feedback.
1 participant
Folded into #444
Closed as
superseded_by_fold, not discarded. #444's head contains this PR as its direct ancestor and has been retargeted to this PR's former base, so the export collection GET implementation/tests and this review history remain intact while queue WIP is reduced.Canonical landing vehicle: #444 (
feat(api): consolidate export collection GET and CLI).Do not reopen unless the folded head demonstrably loses unique behavior or evidence.