feat(api): consolidate export collection GET and CLI - #444
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.
Publish tepp-export-list list so operators mint naruon GET /v1/exports onto spawned tepp-loopback TCP. Receipts stay metric-free. LineageWeave is refused. NaruonLiveService stays POST-only. ADR 0076.
|
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 parsed: ExportCollection = | ||
| serde_json::from_str(&response.body).map_err(|_| ApiError::InvalidWirePayload)?; | ||
| let collection = ExportCollection::new(parsed.items, parsed.next_cursor)?; |
There was a problem hiding this comment.
🟡 Malformed export entries reach stdout
When a 200 response contains malformed export entries, render_export_collection_cli_stdout validates only the page and accepts them. Operators receive invalid entries as valid output.
Prompt for agents
Validate every nested ExportRetrieval before rendering an ExportCollection response. The current flow in crates/tepp_api/src/export_collection_cli.rs deserializes directly into public fields and calls ExportCollection::new, but that constructor validates only page size, cursor, total bytes, and forbidden metric keys. Reconstruct each item through ExportRetrieval's complete validation, or make ExportCollection reconstruction validate all items centrally. Add tests for unsupported contract versions, empty or oversized identifiers, denied decision codes, and unknown purposes in a 200 response.
Was this helpful? React with 👍 or 👎 to provide feedback.
| let mut body = String::new(); | ||
| stdin | ||
| .read_to_string(&mut body) | ||
| .map_err(|_| ApiError::InvalidWirePayload)?; |
There was a problem hiding this comment.
🔴 Piped input can exhaust memory
With nonterminal input, read_export_collection_cli_stdin buffers the entire stream before rejecting any nonempty body. A large or endless pipe can exhaust memory or block indefinitely.
Prompt for agents
Bound stdin handling in crates/tepp_api/src/export_collection_cli.rs. Collection GET accepts only an empty body, so detect the first available byte and fail immediately rather than buffering the complete stream. Preserve clear behavior for EOF and read errors, and add tests using a bounded reader that fails if the implementation reads beyond the first byte.
Was this helpful? React with 👍 or 👎 to provide feedback.
| fn main() -> ExitCode { | ||
| match run() { | ||
| Ok(()) => ExitCode::SUCCESS, | ||
| Err(_) => ExitCode::FAILURE, | ||
| } | ||
| } | ||
|
|
||
| fn run() -> Result<(), ApiError> { | ||
| let args: Vec<String> = std::env::args().skip(1).collect(); | ||
| let body = read_export_collection_cli_stdin(io::stdin().is_terminal(), io::stdin())?; | ||
| let invocation = ExportCollectionCliInvocation::from_args(&args, body)?; | ||
| let response = execute_export_collection_cli(&invocation)?; | ||
| let stdout = render_export_collection_cli_stdout(&invocation, &response)?; | ||
| println!("{stdout}"); | ||
| if (200..300).contains(&response.status_code) { | ||
| Ok(()) | ||
| } else { | ||
| Err(ApiError::InvalidWirePayload) | ||
| } |
There was a problem hiding this comment.
🔍 Published CLI lacks process-level coverage
Tests call library functions but never launch tepp-export-list. Its argument parsing, stdin mode, exit status, and stdout remain outside end-to-end coverage.
Was this helpful? React with 👍 or 👎 to provide feedback.
| let mut bytes = Vec::new(); | ||
| stream | ||
| .read_to_end(&mut bytes) | ||
| .map_err(|error| map_io_error(&error))?; | ||
| parse_http_response(&bytes) |
There was a problem hiding this comment.
📝 Info: EOF completion matches the target server
execute_export_collection_cli waits for EOF, while serve_one closes after one response. The current loopback target therefore completes this read normally.
Was this helpful? React with 👍 or 👎 to provide feedback.
| let stdout = render_export_collection_cli_stdout(&invocation, &response)?; | ||
| println!("{stdout}"); | ||
| if (200..300).contains(&response.status_code) { | ||
| Ok(()) | ||
| } else { | ||
| Err(ApiError::InvalidWirePayload) | ||
| } |
There was a problem hiding this comment.
📝 Info: Success status check is redundant
render_export_collection_cli_stdout already accepts only status 200. The later 2xx check cannot change any successful execution.
Was this helpful? React with 👍 or 👎 to provide feedback.
seonghobae
changed the title
| 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.
🔴 Unchecked page limits panic callers
page_export_collection_items underflows at zero and overflows when a cursor start plus limit exceeds usize::MAX. Public callers can crash.
Prompt for agents
The public page_export_collection_items function accepts any usize, but its implementation assumes a validated limit from parse_export_collection_page_limit. A zero limit reaches items[end - 1], and a sufficiently large limit can overflow start + limit. Make the public API validate its own input or change its contract so invalid limits cannot reach the arithmetic. Preserve exclusive-cursor pagination and update direct helper tests for zero and overflow-sized limits.
Was this helpful? React with 👍 or 👎 to provide feedback.
| if !body.trim().is_empty() { | ||
| return Err(ApiError::InvalidWirePayload); |
There was a problem hiding this comment.
🟡 Whitespace GET bodies bypass refusal
list_exports trims the body before testing emptiness. Whitespace-only bodies receive a successful collection despite the empty-body contract.
| if !body.trim().is_empty() { | |
| return Err(ApiError::InvalidWirePayload); | |
| if !body.is_empty() { | |
| return Err(ApiError::InvalidWirePayload); |
Was this helpful? React with 👍 or 👎 to provide feedback.
| let mut bytes = Vec::new(); | ||
| stream | ||
| .read_to_end(&mut bytes) | ||
| .map_err(|error| map_io_error(&error))?; | ||
| parse_http_response(&bytes) |
There was a problem hiding this comment.
🔍 TCP response reads remain unbounded
execute_export_collection_cli buffers until EOF before validating framing. A faulty loopback peer can consume arbitrary memory; add a response-size bound.
Was this helpful? React with 👍 or 👎 to provide feedback.
| fn main() -> ExitCode { | ||
| match run() { | ||
| Ok(()) => ExitCode::SUCCESS, | ||
| Err(_) => ExitCode::FAILURE, | ||
| } |
There was a problem hiding this comment.
🔍 CLI failures hide their cause
main discards every error and returns only a failure code. Operators cannot distinguish bad flags, connection failures, timeouts, or invalid responses.
Was this helpful? React with 👍 or 👎 to provide feedback.
| let items = self | ||
| .authorized_exports | ||
| .values() | ||
| .map(|stored| stored.retrieval.clone()) | ||
| .collect(); |
There was a problem hiding this comment.
🟥 Export listing crosses tenant boundaries
list_exports returns every stored export to any naruon caller without tenant or principal scope. One caller can enumerate other tenants’ export capabilities.
Was this helpful? React with 👍 or 👎 to provide feedback.
seonghobae
left a comment
seonghobae
left a comment
There was a problem hiding this comment.
exact-head COMMENT only (not APPROVE) on 95ab519fdb39c66a574d1452e969b0b80b9e4ba9. Devin COMMENTED is not independent APPROVE.
Unique remains tepp-export-list + ADR 0076 (naruon export collection GET + CLI). naruon_export_collection_exchange is credential-free HTTPS GET; LineageWeave refused; NaruonLiveService stays POST-only. Page limit/cursor fail closed (InvalidWirePayload / LimitExceeded). Do not duplicate collection GET/CLI. Do not re-open closed collection lineages (#449/#450). Do not unstack onto main. Do not add GET to NaruonLiveService.
Still draft. Do not un-draft. Zero exact-head APPROVEs. Do not merge without two independent current-head APPROVEs under ruleset 18156473.
1 participant
Consolidated landing vehicle
This PR folds predecessor #443 into one naruon-facing Analysis Run / export collection application-adapter vehicle. The current head contains #443 as its direct ancestor, so retargeting to #443's former base preserves the collection GET implementation/tests while eliminating one open micro-PR. #443 remains as immutable review/history evidence.
Preserved GET behavior from #443:
GET /v1/exports, naruon-only authorization, empty body,idempotency-keyrefusal, exclusiveexport_idcursor pagination with bounded page limit, metric-freeExportRetrievalrows, LineageWeave refusal, andNaruonLiveServicePOST-only boundary.CLI behavior on this head: published
tepp-export-list list, typed collection exchange, empty-stdin admission, public-bind/localhost/http-origin/unpublished-consumer/credential refusals, pagination flag mapping, and metric-free render output.This is one Analysis Run/export application-adapter landing vehicle, not a bounded context. ADR 0075/0076 remain implementation lineage pending #437 normalization. Merge only after fresh exact-head hosted checks, resolved conversations, and qualifying independent approval under live ruleset 18156473. No predecessor-head evidence transfer or bypass.