diff --git a/crates/proto/src/audit.rs b/crates/proto/src/audit.rs index 3fa3514..315b8a6 100644 --- a/crates/proto/src/audit.rs +++ b/crates/proto/src/audit.rs @@ -23,6 +23,7 @@ pub enum DecisionRecord { } #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[serde(deny_unknown_fields)] pub struct AuditRecord { pub seq: u64, pub time: Timestamp, diff --git a/crates/proto/src/ids.rs b/crates/proto/src/ids.rs index c0d206d..0b23f4f 100644 --- a/crates/proto/src/ids.rs +++ b/crates/proto/src/ids.rs @@ -128,8 +128,14 @@ fn hex_val(c: u8) -> Result { pub struct Timestamp(u64); impl Timestamp { - pub fn from_unix_millis(ms: u64) -> Self { - Timestamp(ms) + /// 9999-12-31T23:59:59.999Z, the last instant RFC 3339 can spell with a four-digit year. + pub const MAX: Timestamp = Timestamp(253_402_300_799_999); + + pub fn from_unix_millis(ms: u64) -> Result { + if ms > Self::MAX.0 { + return Err(ValueError::Timestamp); + } + Ok(Timestamp(ms)) } pub fn unix_millis(&self) -> u64 { @@ -141,7 +147,7 @@ impl Timestamp { .duration_since(std::time::UNIX_EPOCH) .unwrap_or_default(); let ms = u64::try_from(dur.as_millis()).unwrap_or(u64::MAX); - Timestamp(ms) + Timestamp::from_unix_millis(ms).unwrap_or(Timestamp::MAX) } pub fn to_rfc3339(&self) -> String { @@ -157,7 +163,7 @@ impl Timestamp { .duration_since(std::time::UNIX_EPOCH) .map_err(|_| ValueError::Timestamp)?; let ms = u64::try_from(dur.as_millis()).map_err(|_| ValueError::Timestamp)?; - let parsed = Timestamp(ms); + let parsed = Timestamp::from_unix_millis(ms)?; if parsed.to_rfc3339() != s { return Err(ValueError::Timestamp); } diff --git a/crates/proto/src/log.rs b/crates/proto/src/log.rs index f7896b0..45053b5 100644 --- a/crates/proto/src/log.rs +++ b/crates/proto/src/log.rs @@ -5,6 +5,7 @@ use serde::{Deserialize, Serialize}; use crate::{CallId, DataClass, Epoch, Hash32, SessionId, Timestamp}; #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[serde(deny_unknown_fields)] pub struct ToolCall { pub id: String, pub name: String, diff --git a/crates/proto/tests/ids.rs b/crates/proto/tests/ids.rs index 9291f66..ba7f6e6 100644 --- a/crates/proto/tests/ids.rs +++ b/crates/proto/tests/ids.rs @@ -100,7 +100,7 @@ fn hash32_rejects_wrong_length_uppercase_and_non_hex() { #[test] fn timestamp_has_one_spelling() { - let t = Timestamp::from_unix_millis(1_789_632_300_000); + let t = Timestamp::from_unix_millis(1_789_632_300_000).unwrap(); assert_eq!(t.unix_millis(), 1_789_632_300_000); assert_eq!(t.to_rfc3339(), "2026-09-17T08:05:00.000Z"); assert_eq!(Timestamp::parse("2026-09-17T08:05:00.000Z").unwrap(), t); @@ -109,11 +109,13 @@ fn timestamp_has_one_spelling() { r#""2026-09-17T08:05:00.000Z""# ); assert_eq!( - Timestamp::from_unix_millis(0).to_rfc3339(), + Timestamp::from_unix_millis(0).unwrap().to_rfc3339(), "1970-01-01T00:00:00.000Z" ); assert_eq!( - Timestamp::from_unix_millis(1_789_632_300_007).to_rfc3339(), + Timestamp::from_unix_millis(1_789_632_300_007) + .unwrap() + .to_rfc3339(), "2026-09-17T08:05:00.007Z" ); } @@ -141,6 +143,29 @@ fn timestamp_rejects_other_spellings() { assert!(serde_json::from_str::("1789632300000").is_err()); } +#[test] +fn timestamp_range_ends_with_year_9999() { + assert_eq!(Timestamp::MAX.unix_millis(), 253_402_300_799_999); + assert_eq!(Timestamp::MAX.to_rfc3339(), "9999-12-31T23:59:59.999Z"); + assert_eq!( + Timestamp::parse("9999-12-31T23:59:59.999Z").unwrap(), + Timestamp::MAX + ); + assert_eq!( + Timestamp::from_unix_millis(253_402_300_799_999).unwrap(), + Timestamp::MAX + ); + // One millisecond later has no RFC 3339 spelling, so it must not become a Timestamp at all. + assert_eq!( + Timestamp::from_unix_millis(253_402_300_800_000), + Err(ValueError::Timestamp) + ); + assert_eq!( + Timestamp::from_unix_millis(u64::MAX), + Err(ValueError::Timestamp) + ); +} + #[test] fn timestamp_now_is_after_2026() { assert!(Timestamp::now() > Timestamp::parse("2026-01-01T00:00:00.000Z").unwrap()); diff --git a/crates/proto/tests/strict.rs b/crates/proto/tests/strict.rs new file mode 100644 index 0000000..fcf0bb7 --- /dev/null +++ b/crates/proto/tests/strict.rs @@ -0,0 +1,109 @@ +//! Every JSON object in every fixture must reject an unknown key. Do not edit. +//! +//! The other test files check unknown fields in a few hand-picked places. This one checks all of +//! them: it walks each fixture, adds one unknown key to one object at a time, at every depth, and +//! requires that the result no longer decodes. + +use proto::{AuditRecord, Envelope, Grant, LogRecord}; +use serde::de::DeserializeOwned; +use serde_json::Value; + +/// Every copy of `value` that has exactly one extra key in exactly one object. +fn with_one_unknown_key(value: &Value) -> Vec { + let mut out = Vec::new(); + match value { + Value::Object(map) => { + let mut extended = map.clone(); + extended.insert("zz_unknown".to_string(), Value::Bool(true)); + out.push(Value::Object(extended)); + for (key, child) in map { + for changed in with_one_unknown_key(child) { + let mut copy = map.clone(); + copy.insert(key.clone(), changed); + out.push(Value::Object(copy)); + } + } + } + Value::Array(items) => { + for (i, child) in items.iter().enumerate() { + for changed in with_one_unknown_key(child) { + let mut copy = items.clone(); + copy[i] = changed; + out.push(Value::Array(copy)); + } + } + } + _ => {} + } + out +} + +/// Returns how many variations were tried, so callers can check the walk reached nested objects. +fn check(what: &str, text: &str) -> usize { + let value: Value = serde_json::from_str(text).unwrap_or_else(|e| panic!("{what}: {e}")); + assert!( + serde_json::from_value::(value.clone()).is_ok(), + "{what}: fixture must decode" + ); + let variations = with_one_unknown_key(&value); + for changed in &variations { + assert!( + serde_json::from_value::(changed.clone()).is_err(), + "{what}: accepted an unknown key: {changed}" + ); + } + variations.len() +} + +fn fixture(path: &str) -> String { + let full = format!("{}/tests/fixtures/{path}", env!("CARGO_MANIFEST_DIR")); + std::fs::read_to_string(&full).unwrap_or_else(|e| panic!("{full}: {e}")) +} + +#[test] +fn envelopes_reject_unknown_keys_at_every_depth() { + for name in [ + "tool_request.json", + "tool_response_pending.json", + "tool_response_result.json", + "tool_response_failed.json", + "tool_response_denied.json", + "error.json", + ] { + // Envelope, msg and body: three objects. + assert_eq!( + check::(name, &fixture(&format!("wire/{name}"))), + 3, + "{name}" + ); + } +} + +#[test] +fn audit_records_reject_unknown_keys_at_every_depth() { + for (i, line) in fixture("records/audit.jsonl").lines().enumerate() { + // The record and its decision: two objects. + assert_eq!( + check::(&format!("audit.jsonl:{}", i + 1), line), + 2 + ); + } +} + +#[test] +fn log_records_reject_unknown_keys_at_every_depth() { + let mut tried = 0; + for (i, line) in fixture("records/session.jsonl").lines().enumerate() { + tried += check::(&format!("session.jsonl:{}", i + 1), line); + } + // Seven records, plus the one tool call inside the first assistant record. + assert_eq!(tried, 8); +} + +#[test] +fn grants_reject_unknown_keys_at_every_depth() { + let grant: Grant = toml::from_str(&fixture("grant/full.toml")).unwrap(); + let text = serde_json::to_string(&grant).unwrap(); + // The grant and its constraints: two objects. + assert_eq!(check::("full.toml as JSON", &text), 2); +} diff --git a/docs/implementer-log.md b/docs/implementer-log.md index 4c5e4dc..b964491 100644 --- a/docs/implementer-log.md +++ b/docs/implementer-log.md @@ -12,6 +12,7 @@ reviewer adds findings under "Reviews" once per milestone. | M1/05-proto-grant | 2026-09-17 | done | 2 | fail | none | Added crates/proto/src/grant.rs (Mode, Constraints with Default, Grant with serde defaults + deny_unknown_fields) re-exported from lib.rs and toml 1.1.6 as a proto dev-dependency (workspace dep + dependencies.md row); 4 fixture tests pass. cargo-fmt reordered the lib.rs re-exports before the gate. | | M1/06-proto-records | 2026-09-17 | done | 2 | fail | none | Added crates/proto/src/audit.rs (DecisionRecord, AuditRecord) and crates/proto/src/log.rs (ToolCall, LogRecord) re-exported from lib.rs; 3 fixture tests pass, 40 total across the five proto test files. cargo-fmt reordered the lib.rs re-exports before the gate. | | M1/07-brokerd-decision | 2026-09-17 | done | 1 | pass | none | Added Decision (Debug only, private fields), decide (Err(NoGrant) until M3) and the run stub (ToolResponse::Failed) in crates/brokerd; Decision::new carries expect(dead_code). 2 unit + 3 doctests (2 compile_fail) pass; verified the compile_fail guards by temporarily making new pub. `make gate` prints `gate: ok`. | +| M1/08-proto-strictness | 2026-09-17 | done | 1 | pass | none | Added deny_unknown_fields to AuditRecord and ToolCall in crates/proto; bounded Timestamp (MAX const, from_unix_millis -> Result, parse bounds via from_unix_millis, now clamps to MAX) in ids.rs. 45 proto tests pass; `cargo fmt --all` and `make gate` print `gate: ok`. | ## Reviews