Skip to content

feat(api): resolve export identity by idempotency key on loopback - #465

Closed
seonghobae wants to merge 4 commits into
feat/export-retrieval-get-gap-003afrom
feat/export-idempotency-lookup-get-gap-003a
Closed

feat(api): resolve export identity by idempotency key on loopback#465
seonghobae wants to merge 4 commits into
feat/export-retrieval-get-gap-003afrom
feat/export-idempotency-lookup-get-gap-003a

Conversation

@seonghobae

@seonghobae seonghobae commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Folded into #466

This predecessor is closed as superseded_by_fold, not discarded. #466's current head contains this PR as its direct ancestor and has been retargeted to this PR's former base, so the GET implementation, tests, review history, RED 79cb5d6cdbf8f18c82bb990a63d9282a278293e1, and repairs e40b4078762b37e05ca85fd066009bb30bd663c4 / 0fd64f72cb0978d5603a5bf78954bb8e8f35d45a remain preserved while the open WIP count is reduced.

Canonical landing vehicle: #466 (feat/api: consolidate export idempotency lookup GET and CLI).

Do not reopen this one-operation predecessor unless the folded head demonstrably loses unique behavior or evidence.

GAP-003A unique slice: AnalysisRunLiveService serves naruon-only
GET /v1/exports/by-idempotency/{idempotency_key} as a metric-free
export_id lookup. NaruonLiveService stays POST-only. ADR 0093.
@coderabbitai

coderabbitai Bot commented Sep 1, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Team

Run ID: 4dc72e74-6b73-4c7d-addc-2512c5ba96cf

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note

This report is out of date. Scroll down for Devin Review's latest report on this PR.

Devin Review found 7 potential issues.

Devin Review

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔍 Merge requires landing-vehicle consolidation

Repository workflow requires this micro-slice to remain unmerged until folded into the coherent export adapter vehicle and verified at that exact head.

Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔍 Rust verification remains external

Mandatory focused and complete Rust verification remains unconfirmed because this review environment lacks the Cargo executable.

Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Comment thread crates/tepp_api/src/export_idempotency_lookup_http.rs
Comment thread crates/tepp_api/src/export_http.rs Outdated
Comment thread crates/tepp_api/src/export_idempotency_lookup_http.rs Outdated
Comment on lines +371 to +384
let prefix = format!("{consumer}\u{1f}");
let mut matches: Vec<&StoredExport> = self
.authorized_exports
.iter()
.filter(|(replay_key, stored)| {
replay_key.starts_with(&prefix)
&& stored.retrieval.idempotency_key == idempotency_key
})
.map(|(_, stored)| stored)
.collect();
if matches.len() != 1 {
return Err(ApiError::InvalidWirePayload);
}
let stored = matches.remove(0);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📝 Info: Ambiguous tenant matches fail closed

The consumer-wide scan cannot select among tenants safely. Requiring one match avoids arbitrary resolution without exposing tenant identity.

Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Comment on lines +170 to +175
if matches!(
export_idempotency_lookup_path_key(path),
Ok(_) | Err(ApiError::LimitExceeded)
) {
return self.lookup_export_by_idempotency(path, &headers, body);
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📝 Info: Oversized keys retain limit status

Routing LimitExceeded into the lookup handler preserves the documented 413 response instead of converting oversized keys to malformed-path errors.

Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 2 new potential issues.

Devin Review

Comment on lines +220 to +223
if encoded.is_empty() || encoded.contains('/') {
return Err(ApiError::InvalidWirePayload);
}
let key = decode_path_segment(encoded)?;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📝 Info: Encoded slashes preserve route boundaries

export_idempotency_lookup_path_key rejects raw extra segments before decoding %2F. Slash-containing keys round-trip without opening multi-segment lookup paths.

Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Comment on lines +183 to +191
fn contains_forbidden_export_lookup_key(value: &serde_json::Value) -> bool {
match value {
serde_json::Value::Object(object) => object.iter().any(|(key, value)| {
FORBIDDEN_EXPORT_LOOKUP_KEYS.contains(&key.as_str())
|| contains_forbidden_export_lookup_key(value)
}),
serde_json::Value::Array(values) => values.iter().any(contains_forbidden_export_lookup_key),
_ => false,
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📝 Info: Recursive refusal covers nested payloads

contains_forbidden_export_lookup_key traverses objects and arrays. Valid receipts remain accepted, while nested metric or identity fields fail closed.

Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant