Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions crates/tepp_api/src/analysis_run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@ pub const DEFAULT_ANALYSIS_RUN_BYTE_LIMIT: usize = 64 * 1024;
/// Supported analysis-run status/read contract version.
pub const ANALYSIS_RUN_STATUS_CONTRACT_VERSION: u16 = 1;

/// Versioned analysis-run status/read path served by the TEPP HTTP boundary.
pub const ANALYSIS_RUN_STATUS_PATH: &str = "/v1/analysis-runs";

/// Request to create a durable analysis run.
#[derive(Clone, Debug, Deserialize, Eq, PartialEq, Serialize)]
#[serde(deny_unknown_fields)]
Expand Down
119 changes: 119 additions & 0 deletions crates/tepp_api/src/analysis_run_status_http.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,119 @@
//! Provider-owned analysis-run status/read HTTP exchange contracts.
//!
//! This module exposes a fail-closed `GET /v1/analysis-runs/{run_id}` exchange
//! builder so modular consumers (, ) can poll accepted runs

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Empty placeholders in module docstring

The module docstring reads modular consumers (, ) with the intended names dropped, leaving empty parentheses.

Suggested change
//! builder so modular consumers (, ) can poll accepted runs
//! builder so modular consumers (`LineageWeave`, `Naruon`) can poll accepted runs
Open in Devin Review

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

//! without inventing routes, retry coefficients, or credential headers
//! locally. TEPP remains the sole authority for lifecycle state, terminal
//! results, and evidence binding.

use crate::naruon_http::{NaruonHttpExchange, compose_https_target, standard_headers};
use crate::wire::require_nonempty;
use crate::{ANALYSIS_RUN_STATUS_PATH, ApiError};

/// Maximum length accepted for an opaque run identity in the status path.
pub const ANALYSIS_RUN_ID_MAX_LEN: usize = 128;

/// Build a provider-owned `GET` analysis-run status exchange.
///
/// The caller supplies the TEPP origin and the opaque server-assigned run
/// identity returned in the accepted receipt. The builder refuses non-`https`
/// origins and empty or oversized run identifiers but performs no credential
/// injection: the caller must supply its own authorization header through the
/// transport layer if the deployment requires it.
///
/// # Errors
///
/// Returns [`ApiError::InvalidWirePayload`] for a non-`https` origin, a
/// table-access URL, an empty run identifier, or a run identifier exceeding
/// [`ANALYSIS_RUN_ID_MAX_LEN`] bytes.
Comment on lines +26 to +28

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Docstring names the wrong error for oversized run id

The # Errors docstring says an over-length run identifier returns ApiError::InvalidWirePayload, but the code returns ApiError::LimitExceeded and the test asserts the same. The documented contract does not match behavior.

Suggested change
/// Returns [`ApiError::InvalidWirePayload`] for a non-`https` origin, a
/// table-access URL, an empty run identifier, or a run identifier exceeding
/// [`ANALYSIS_RUN_ID_MAX_LEN`] bytes.
/// Returns [`ApiError::InvalidWirePayload`] for a non-`https` origin, a
/// table-access URL, or an empty run identifier, and
/// [`ApiError::LimitExceeded`] for a run identifier exceeding
/// [`ANALYSIS_RUN_ID_MAX_LEN`] bytes.
Open in Devin Review

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

pub fn naruon_analysis_run_status_exchange(
origin: &str,
run_id: &str,
idempotency_key: &str,
) -> Result<NaruonHttpExchange, ApiError> {
require_nonempty(run_id)?;
if run_id.len() > ANALYSIS_RUN_ID_MAX_LEN {
return Err(ApiError::LimitExceeded);
}
let encoded_run_id = encode_path_segment(run_id);
let target_path = format!("{ANALYSIS_RUN_STATUS_PATH}/{encoded_run_id}");
let target_url = compose_https_target(origin, &target_path)?;
Ok(NaruonHttpExchange {
method: "GET",
target_url,
headers: standard_headers(idempotency_key),
Comment on lines +34 to +44

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟨 Unvalidated idempotency key allows header injection

naruon_analysis_run_status_exchange copies idempotency_key straight into the idempotency-key header via standard_headers with no validation. Sibling builder naruon_export_exchange (naruon_http.rs:92) rejects control characters via require_nonempty first; this one does not. A key containing CR/LF is carried into the header, enabling header injection.

Open in Devin Review

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

body: String::new(),
})
Comment on lines +41 to +46

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: Status builder hardcodes naruon consumer

standard_headers stamps tepp-consumer: naruon, yet the module advertises LineageWeave as a consumer too. Unlike lineageweave_analysis_run_exchange (lineageweave_http.rs:37), no variant swaps the consumer code, so a LineageWeave status poll is reported as naruon.

Open in Devin Review

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

}

/// Percent-encode one `URI` path segment without double-encoding safe chars.
fn encode_path_segment(value: &str) -> String {
let mut out = String::with_capacity(value.len() + value.len() / 2);
let hex = b"0123456789ABCDEF";
for byte in value.bytes() {
match byte {
b'A'..=b'Z' | b'a'..=b'z' | b'0'..=b'9' | b'-' | b'.' | b'_' | b'~' => {
out.push(byte as char);
}
_ => {
out.push('%');
out.push(hex[usize::from(byte >> 4)] as char);
out.push(hex[usize::from(byte & 0x0F)] as char);
}
}
}
out
}

#[cfg(test)]
mod tests {
use super::*;

#[test]
fn builds_get_exchange_for_valid_origin_and_run_id() {
let exchange = naruon_analysis_run_status_exchange(
"https://tepp.example.com",
"run-abc-123",
"idem-key-1",
)
.expect("valid origin and run id");
assert_eq!(exchange.method, "GET");
assert_eq!(
exchange.target_url,
"https://tepp.example.com/v1/analysis-runs/run-abc-123"
);
assert!(exchange.body.is_empty());
assert!(!exchange.headers.is_empty());
}

#[test]
fn percent_encodes_unsafe_characters_in_run_id() {
let exchange = naruon_analysis_run_status_exchange(
"https://tepp.example.com",
"run/../../etc",
"key",
)
.expect("unsafe chars are encoded not rejected");
assert!(exchange.target_url.contains("run%2F..%2F..%2Fetc"));
}

#[test]
fn refuses_http_origin() {
let result =
naruon_analysis_run_status_exchange("http://tepp.example.com", "run-1", "k");
assert_eq!(result.unwrap_err(), ApiError::InvalidWirePayload);
}

#[test]
fn refuses_empty_run_id() {
let result = naruon_analysis_run_status_exchange("https://t.example.com", "", "k");
assert_eq!(result.unwrap_err(), ApiError::InvalidWirePayload);
}

#[test]
fn refuses_oversized_run_id() {
let big = "a".repeat(ANALYSIS_RUN_ID_MAX_LEN + 1);
let result = naruon_analysis_run_status_exchange("https://t.example.com", &big, "k");
assert_eq!(result.unwrap_err(), ApiError::LimitExceeded);
}
}
5 changes: 5 additions & 0 deletions crates/tepp_api/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@

mod analysis_result;
mod analysis_run;
mod analysis_run_status_http;
mod analysis_run_live;
mod authorization;
mod corpus_split_manifest;
Expand Down Expand Up @@ -52,6 +53,10 @@ pub use analysis_result::terminal_result_matches_request;
pub use analysis_run::ANALYSIS_RUN_CONTRACT_VERSION;
/// Analysis-run status/read contract version constant.
pub use analysis_run::ANALYSIS_RUN_STATUS_CONTRACT_VERSION;
pub use analysis_run::ANALYSIS_RUN_STATUS_PATH;
pub use analysis_run_status_http::{
ANALYSIS_RUN_ID_MAX_LEN, naruon_analysis_run_status_exchange,
};
/// Accepted analysis-run response.
pub use analysis_run::AnalysisRunAccepted;
/// Analysis-run create request.
Expand Down
Loading