From f9fa8696d509485454f38eed65840b0aebe8f77a Mon Sep 17 00:00:00 2001 From: Xiaoju Date: Tue, 11 Aug 2026 20:26:22 +0800 Subject: [PATCH] fix: harden snapshot inputs and registry semantics --- .agent/PROJECT_STATE.md | 13 +- .agent/plans/pre-collector-hardening.md | 155 +++++++++++++ Cargo.lock | 49 +++- Cargo.toml | 1 + crates/systemdiff-cli/Cargo.toml | 1 - crates/systemdiff-cli/src/main.rs | 103 ++++++++- crates/systemdiff-core/Cargo.toml | 3 +- crates/systemdiff-core/src/lib.rs | 186 ++++++++++++++- .../systemdiff-core/tests/registry_startup.rs | 211 ++++++++++++++++++ .../tests/snapshot_document.rs | 57 +++++ .../systemdiff-core/tests/snapshot_fixture.rs | 67 +++++- crates/systemdiff-risk/src/lib.rs | 6 +- docs/architecture.md | 3 + docs/collectors.md | 10 +- docs/data-format.md | 32 ++- docs/threat-model.md | 4 +- fixtures/snapshots/after-v1.json | 4 + fixtures/snapshots/before-v1.json | 2 + 18 files changed, 883 insertions(+), 24 deletions(-) create mode 100644 .agent/plans/pre-collector-hardening.md create mode 100644 crates/systemdiff-core/tests/registry_startup.rs create mode 100644 crates/systemdiff-core/tests/snapshot_document.rs diff --git a/.agent/PROJECT_STATE.md b/.agent/PROJECT_STATE.md index 2a76020..8487ba8 100644 --- a/.agent/PROJECT_STATE.md +++ b/.agent/PROJECT_STATE.md @@ -4,7 +4,7 @@ Last updated: 2026-08-11 ## Current phase -The public repository foundation is complete. The initial commit is on `origin/main`, the first GitHub CI run passed, and the repository's baseline security and branch rules are active. The current phase is pre-collector hardening before implementation of the first real Windows Collector. +The public repository foundation and pre-collector hardening are complete. Snapshot input now has a bounded, version-routed, UTC-validated read path, and Registry startup evidence has explicit view and RunOnce semantics. The next engineering phase is the first real Windows Registry Collector and `snapshot` CLI path after this hardening is merged. ## Implemented components @@ -17,6 +17,9 @@ The public repository foundation is complete. The initial commit is on `origin/m - The first GitHub CI run passed on Windows and Ubuntu. The active `main` ruleset requires pull requests, resolved review threads, and the `Rust (windows-latest)` and `Rust (ubuntu-latest)` checks; it blocks deletion and non-fast-forward updates while retaining an explicit maintainer bypass. - GitHub Secret Scanning, Push Protection, and Private Vulnerability Reporting are enabled. - Registry evidence keeps the native type code, validated typed decode status/value, a full-content SHA-256, and an optional validated 4 KiB lowercase-hex raw prefix rather than assuming every value is a UTF-16LE string. +- The CLI rejects Snapshot files larger than 64 MiB before full decoding, bounds the actual read, and routes `document_type`/`schema_version` before constructing Snapshot v1. +- `captured_at` uses standards-based RFC 3339 parsing and accepts only known UTC represented by `Z` or `+00:00`; input evidence remains an unchanged wire string. +- Registry startup evidence distinguishes Run from RunOnce, validates structured `!`/`*` semantics against the complete raw value name, and preserves prefixed names in identity. Registry view labels have explicit, process-bitness-independent acquisition semantics. - The bootstrap foundation has passed real-toolchain validation and independent architecture, security, and maintainability review. ## Known limitations @@ -25,7 +28,7 @@ The public repository foundation is complete. The initial commit is on `origin/m - `systemdiff snapshot` is intentionally unavailable; the end-to-end MVP is not complete. - The desktop app is a documented future boundary, not a generated Tauri application. - Redaction metadata exists in the schema, but sanitization is not implemented. -- The bootstrap CLI currently reads an entire JSON file before validation; header-first version routing, RFC 3339 timestamp validation, and input resource limits are not implemented. +- The 64 MiB file boundary does not yet impose separate object-count, string-size, nesting-depth, or per-artifact count limits, and parser fuzzing is not configured. A streaming parser is intentionally absent. - Draft fixtures and wire types may change before v0.1; after v0.1, v1 compatibility becomes a release obligation. - Draft v0.1 diffs assume the same Windows installation and the same user/principal context. Cross-host and cross-user identity are out of scope; no SID hash, machine token, or identity framework exists. - No dedicated private Code of Conduct reporting channel is published. GitHub Private Vulnerability Reporting is available only for product security reports. @@ -36,8 +39,12 @@ The public repository foundation is complete. The initial commit is on `origin/m - Rust owns the shared domain, diff, rule, and reporting logic; Windows API access is isolated in one crate. - Tauri 2 with React and TypeScript is proposed for v0.2 and will be validated after the CLI MVP. - Snapshot and diff JSON are separately versioned documents with deterministic serialization expectations. +- Snapshot files are capped at 64 MiB at the CLI boundary and routed by a minimal core header before v1 body construction; no configurable resource policy or migration registry exists. +- Valid read-time `captured_at` values must be RFC 3339 known UTC using `Z` or `+00:00`; future SystemDiff writers will emit canonical `Z`. - Collector failures and privilege limitations are recorded per collector/scope and must not invalidate unrelated evidence. - Unknown cross-version comparisons for the same Collector ID are rejected by default; future explicitly verified compatible version pairs remain possible, but no compatibility framework exists yet. +- `RegistryView::Shared` follows Microsoft's shared-key model; `Registry32` and `Registry64` require their explicit WOW64 selectors; `Native` is reserved for a sole view where no WOW alternate logical view exists. +- RunOnce `!` means deletion is deferred until after the command runs, `*` means the entry runs in Safe Mode, and undocumented combined/repeated forms are retained without inferred behavior. Complete value names remain identity-bearing evidence. - v0.1 comparison is limited to before/after snapshots from the same Windows installation and user/principal context. - SystemDiff remains offline-first and read-only; evidence is never executed or remediated. - Apache-2.0 is the repository license. @@ -45,7 +52,7 @@ The public repository foundation is complete. The initial commit is on `origin/m ## Next milestone -Complete the focused pre-collector hardening work: bounded snapshot input, document-header routing, UTC RFC 3339 validation, explicit Registry view semantics, and correct structured RunOnce prefix evidence. Only then specify and implement the first real Registry Run/RunOnce Collector and `snapshot` CLI path. +Specify and implement the first real Registry Run/RunOnce Collector and `snapshot` CLI path behind deterministic Windows data-source abstractions, including explicit 32/64-bit view coverage, current-user shared coverage, permission/partial outcomes, fixtures, and non-elevated tests. ## Major unresolved questions diff --git a/.agent/plans/pre-collector-hardening.md b/.agent/plans/pre-collector-hardening.md new file mode 100644 index 0000000..448a04a --- /dev/null +++ b/.agent/plans/pre-collector-hardening.md @@ -0,0 +1,155 @@ +# Pre-collector hardening + +Status: Complete +Owner: Codex +Last updated: 2026-08-11 + +## Goal + +Harden the untrusted Snapshot-file boundary and the draft Registry startup evidence model before any real Windows Collector is implemented. This plan delivers GitHub issue [#3](https://github.com/XiaojuCH/SystemDiff/issues/3) without expanding into collection, remediation, identity, risk, desktop, or release work. + +## User-visible outcome + +The existing `systemdiff diff` command will reject oversized Snapshot files before an unbounded full-file allocation, route supported JSON documents by header before constructing a v1 `Snapshot`, and reject invalid or non-UTC `captured_at` values. Contributors implementing the first Registry Collector will have explicit Registry-view contracts and structured, Microsoft-grounded RunOnce prefix evidence that preserves complete value names and collision-free identities. + +## Current architecture and context + +- `systemdiff-cli::load_snapshot` currently performs `fs::read` followed by direct `serde_json::from_slice::`. +- `systemdiff-diff::diff_snapshots` is the first production call to `Snapshot::validate`; document type and schema checks therefore occur only after the full v1 wire type is constructed. +- `captured_at` is a `String` whose only current constraint is non-empty. +- `RegistryView` has stable enum variants but no rustdoc definition, and `Native` is ambiguous. +- `RegistryStartupEntry` preserves the complete raw `value_name`, but has no structured Run/RunOnce distinction or prefix semantics. +- ADR 0002 requires header-first routing. The threat model identifies unbounded Snapshot JSON as TM-002. +- Draft schema v1 is explicitly mutable before v0.1; no released compatibility promise is being rewritten. + +Read-only investigation used the project explorer, windows-researcher, and test-engineer roles. Microsoft references: + +- [Run and RunOnce Registry Keys](https://learn.microsoft.com/en-us/windows/win32/setupapi/run-and-runonce-registry-keys) +- [RunOnce Registry Key](https://learn.microsoft.com/en-us/windows-hardware/drivers/install/runonce-registry-key) +- [Registry Keys Affected by WOW64](https://learn.microsoft.com/en-us/windows/win32/winprog64/shared-registry-keys) +- [Accessing an Alternate Registry View](https://learn.microsoft.com/en-us/windows/win32/winprog64/accessing-an-alternate-registry-view) +- [Registry Key Security and Access Rights](https://learn.microsoft.com/en-us/windows/win32/sysinfo/registry-key-security-and-access-rights) + +## Constraints + +- Remain offline-first, read-only, synchronous, and deterministic. +- Treat Snapshot JSON as untrusted; report precise document and size failures without parsing evidence as commands. +- Do not add a streaming JSON parser, async runtime, configurable policy framework, v2 dispatch registry, or migration framework. +- Accept RFC 3339 `captured_at` values only when they use known UTC (`Z` or `+00:00`); reject non-zero offsets and RFC 3339 `-00:00` unknown-local-offset notation. Preserve the input string rather than canonicalizing read evidence. +- Preserve `RegistryView::{Shared, Native, Registry32, Registry64}`. `Native` must never mean a process-bitness-dependent default view for a redirected key. +- Preserve the full Registry value name in evidence and canonical identity. Do not strip `!` or `*` for identity. +- Do not infer semantics for combined, repeated, or marker-only RunOnce prefixes that Microsoft does not document. +- Do not modify Scheduled Task raw XML, Collector traits, `change_id`, risk behavior, sanitizer design, MSRV policy, GUI/Tauri, or release tooling. +- Do not implement a real Collector, Registry API call, or `systemdiff snapshot`. + +## Implementation steps + +1. Create issue #3, complete read-only architecture/Windows/test research, and record decisions in this plan. +2. Add a 64 MiB CLI Snapshot input maximum. Open the file, inspect handle metadata, reject an oversized length, then perform a bounded `MAX + 1` read and recheck actual bytes before decoding. Keep a small limit-injected helper for deterministic tests. +3. Add a core `decode_snapshot_document` boundary that deserializes only `document_type` and `schema_version`, explicitly routes `systemdiff.snapshot` v1, then constructs the current `Snapshot`. Return typed header/type/version/body errors. +4. Add standard RFC 3339 parsing in `Snapshot::validate` using the parsing-only surface of the maintained, Apache-2.0 OR MIT `time` crate. Require known UTC and preserve original wire text. +5. Add exact rustdoc and public-document semantics for all four `RegistryView` variants. Define `Registry32`/`Registry64` by the corresponding WOW64 selector, `Shared` by Microsoft's shared-key model, and `Native` as the sole view only where no WOW alternate view exists. +6. Add a required Registry startup kind plus a single optional RunOnce prefix-semantics enum. Run entries carry no RunOnce semantics; RunOnce entries carry exactly one of no documented prefix, deferred deletion, Safe Mode execution, or undocumented syntax. Validate the structured value against the complete raw value name. +7. Update synthetic fixtures and mechanical struct literals. Add tests for size ordering, header routing, UTC timestamp policy, stable RegistryView spellings, RunOnce semantics/round-trip/invariants, and full-name identity separation. +8. Update data-format, Collector, architecture/dependency, and durable project-state documentation without changing excluded subsystems. +9. Run format, Clippy, full workspace tests, existing CLI smoke commands, deterministic fixture checks, Markdown/diff checks, and independent reviewer analysis. Address High/Medium findings in scope. +10. Complete this plan and hand the validated implementation to the repository's normal branch/PR workflow. Track external integration and required-check status in the PR rather than treating unobserved CI as repository-local validation. + +## Affected files and modules + +Expected production and dependency surface: + +- `Cargo.toml`, `Cargo.lock` +- `crates/systemdiff-core/Cargo.toml` +- `crates/systemdiff-core/src/lib.rs` +- `crates/systemdiff-cli/Cargo.toml` +- `crates/systemdiff-cli/src/main.rs` + +Expected tests and draft fixtures: + +- `crates/systemdiff-core/tests/` +- `crates/systemdiff-risk/src/lib.rs` only for mechanical test-fixture construction +- `fixtures/snapshots/before-v1.json` +- `fixtures/snapshots/after-v1.json` + +Expected documentation/state: + +- `docs/architecture.md` +- `docs/data-format.md` +- `docs/collectors.md` +- `docs/threat-model.md` +- `.agent/PROJECT_STATE.md` +- this ExecPlan + +No production changes are expected in diff, report, risk, Windows, services, tasks, or desktop modules. + +## Test strategy + +- CLI size helper: below limit, exactly at limit, above limit. +- CLI ordering: a tiny invalid JSON file above an injected small limit returns `too large` before read/decode; the production reader remains bounded after metadata to limit growth races. +- Core document decoder: valid v1 fixture, unknown document type with an invalid body, unsupported version with an invalid body, supported header with invalid v1 body, and malformed/missing header. +- Timestamp validation: `Z`, `+00:00`, fractional seconds, non-zero offset, `-00:00`, malformed text, invalid calendar/time values, and empty input. +- RegistryView: exact serialized names, round-trip, unknown-name rejection, and fixture consistency. +- Registry startup: Run, RunOnce without a documented prefix, `!`, `*`, combined/repeated/marker-only forms as undocumented, raw/structured mismatch rejection, serde round-trip, and distinct `Foo`/`!Foo`/`*Foo` observation keys. +- Regression: existing fixture validation, deterministic shuffled Diff JSON, incomplete-coverage behavior, undecoded Registry hash comparison, Collector-version rejection, report tests, and all three CLI smoke paths. + +## Risks + +- A metadata-only size check has a growth race. Mitigation: retain metadata preflight but bound the actual read to `MAX + 1` and recheck before decoding. +- Header deserialization still scans the bounded JSON to skip body fields; it does not construct the full `Snapshot`, but it is not a streaming header parser. Documentation must describe this accurately. +- RFC 3339 parsers may normalize `-00:00` to numeric zero. Mitigation: use standard parsing plus an explicit accepted UTC designator policy. +- Adding required Registry fields changes unreleased draft-v1 fixtures. Mitigation: update all golden fixtures and reject missing/inconsistent evidence rather than silently defaulting it. +- Two independent RunOnce booleans could imply undocumented combinations. Mitigation: use one enum and retain uninterpreted raw names. +- `canonical_id` is Collector-owned, so core cannot prove that a future Collector includes the prefix. Mitigation: fixture/tests and docs fix the contract now; real canonicalization remains part of the Collector PR. + +## Rollback and compatibility + +The change is a focused, reversible draft-v1 update before v0.1. Reverting the PR restores prior fixtures and parsing behavior. Once v0.1 ships, these semantics become compatibility obligations and must not be redefined in place. + +Existing supported fixture diffs remain semantically deterministic. Unsupported document families/versions are rejected earlier and more clearly. No cross-version migration behavior is introduced. + +## Progress + +- [x] 2026-08-11: Confirmed clean synchronized `main`, green CI, active ruleset, and no equivalent issue. +- [x] 2026-08-11: Created focused GitHub issue #3. +- [x] 2026-08-11: Completed explorer, Microsoft Windows, and test-engineer read-only investigations. +- [x] 2026-08-11: Implemented and tested the bounded input and header-routing boundaries. +- [x] 2026-08-11: Implemented UTC timestamp and Registry schema semantics with deterministic tests. +- [x] 2026-08-11: Updated draft fixtures, public technical documentation, the threat model, and durable project state. +- [x] 2026-08-11: Completed local format, Clippy, workspace-test, CLI-smoke, diff, dependency-feature, and Markdown-link validation. +- [x] 2026-08-11: Completed independent read-only review with no High, Medium, or actionable Low findings. +- [x] 2026-08-11: Completed the implementation plan and prepared the reviewed change for the branch/PR workflow. + +## Discoveries + +- Microsoft documents `!` as deferring RunOnce value deletion until after the command runs, and `*` as allowing RunOnce execution in Safe Mode. It does not define combined/repeated prefixes or their ordering. +- HKLM `Software` is redirected on 64-bit Windows, while HKCU `Software` is shared on supported Windows versions; HKLM startup locations need explicit 32/64 passes and HKCU is collected once. +- Omitting WOW64 selectors on a redirected key binds the result to process bitness and is not stable evidence. +- RFC 3339 `-00:00` represents an unknown local offset, not a known UTC assertion. +- Header-first routing belongs in core so future CLI/desktop readers share version dispatch; filesystem size enforcement remains at the CLI boundary. + +## Decisions + +- Use a fixed 64 MiB input ceiling. It provides substantial headroom for targeted v0.1 evidence and future bounded task/Registry payloads while placing an auditable limit on parser amplification. A configurable policy is deferred until a demonstrated use case exists. +- Keep `captured_at` as the original wire `String`; validate rather than normalize evidence read from disk. A future writer will emit canonical `Z`. +- Use the `time` crate with only RFC 3339 parsing support. It is maintained, license-compatible, and avoids a custom parser or a larger time framework. +- Define `Native` only as the sole Registry view when no WOW alternate logical views exist. Redirected keys on WOW-enabled systems must use explicit `Registry32`/`Registry64` evidence. +- Add `RegistryStartupKind` (`run`/`run_once`) and a single `RunOncePrefixSemantics` value. Structured semantics enrich but never replace the complete `value_name`. + +## Final validation + +Local validation completed on Windows with the installed stable MSVC toolchain: + +- `cargo fmt --all --check`: passed after applying one formatting-only adjustment with `cargo fmt --all`. +- `cargo clippy --locked --workspace --all-targets -- -D warnings`: passed with no warnings. +- `cargo test --locked --workspace --all-targets`: passed, 39 tests, 0 failed, 0 ignored. +- `cargo run --locked -p systemdiff-cli -- diff fixtures/snapshots/before-v1.json fixtures/snapshots/after-v1.json`: passed; reported 2 added, 1 inconclusive, 1 modified, and 1 removed change. +- `cargo run --locked -p systemdiff-cli -- diff --json fixtures/snapshots/before-v1.json fixtures/snapshots/after-v1.json`: passed and emitted the deterministic v1 JSON Diff document. +- `cargo run --locked -p systemdiff-cli -- collectors`: passed and reported the three MVP Collectors as planned. +- `git diff --check`: passed. +- Relative Markdown links in modified project documents: passed. +- `cargo tree -p systemdiff-core -e features`: confirmed that SystemDiff enables only the `time` parsing feature. + +The independent reviewer confirmed the five adjudicated hardening items, dependency scope, documentation, draft-schema change, and explicit exclusions. It reported 0 High, 0 Medium, and 0 actionable Low findings. + +GitHub Actions had not run for the feature branch when this repository-local plan was completed. The PR records that external Windows and Ubuntu validation; this plan does not claim an unobserved CI result. diff --git a/Cargo.lock b/Cargo.lock index 156692e..842c97c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -98,6 +98,12 @@ version = "1.0.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "1d07550c9036bf2ae0c684c4297d503f838287c83c53686d05370d0e139ae570" +[[package]] +name = "deranged" +version = "0.5.8" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7cd812cc2bc1d69d4764bd80df88b4317eaef9e773c75226407d9bc0876b211c" + [[package]] name = "heck" version = "0.5.0" @@ -122,12 +128,24 @@ version = "2.8.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "cf8baf1c55e62ffcace7a9f06f4bd9cd3f0c4beb022d3b367256b91b87513d98" +[[package]] +name = "num-conv" +version = "0.2.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "521739c6d2bac4aa25192232afe6841231376b2b26d4d9fae5ecf8ca5772e441" + [[package]] name = "once_cell_polyfill" version = "1.70.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "384b8ab6d37215f3c5301a95a4accb5d64aa607f1fcb26a11b5303878451b4fe" +[[package]] +name = "powerfmt" +version = "0.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "439ee305def115ba05938db6eb1644ff94165c5ab5e9420d1c1bcedbba909391" + [[package]] name = "proc-macro2" version = "1.0.107" @@ -211,7 +229,6 @@ name = "systemdiff-cli" version = "0.0.0" dependencies = [ "clap", - "serde_json", "systemdiff-core", "systemdiff-diff", "systemdiff-report", @@ -224,6 +241,7 @@ version = "0.0.0" dependencies = [ "serde", "serde_json", + "time", ] [[package]] @@ -260,6 +278,35 @@ dependencies = [ "systemdiff-core", ] +[[package]] +name = "time" +version = "0.3.55" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "cdb87b95ec50ddfa440816d227a17b2ccbdda963a316a727fda0fc4334f7d134" +dependencies = [ + "deranged", + "num-conv", + "powerfmt", + "time-core", + "time-macros", +] + +[[package]] +name = "time-core" +version = "0.1.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9e1c906769ad99c88eaa54e728060edef082f8e358ff32030cb7c7d315e81109" + +[[package]] +name = "time-macros" +version = "0.2.32" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7e689342a48d2ea927c87ea50cabf8594854bf940e9310208848d680d668ed85" +dependencies = [ + "num-conv", + "time-core", +] + [[package]] name = "unicode-ident" version = "1.0.24" diff --git a/Cargo.toml b/Cargo.toml index 32e600d..6661f05 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -19,6 +19,7 @@ authors = ["SystemDiff contributors"] clap = { version = "4.5", features = ["derive"] } serde = { version = "1.0", features = ["derive"] } serde_json = "1.0" +time = { version = "0.3.55", default-features = false, features = ["parsing"] } systemdiff-core = { path = "crates/systemdiff-core" } systemdiff-diff = { path = "crates/systemdiff-diff" } systemdiff-report = { path = "crates/systemdiff-report" } diff --git a/crates/systemdiff-cli/Cargo.toml b/crates/systemdiff-cli/Cargo.toml index 7a857ed..2e9104e 100644 --- a/crates/systemdiff-cli/Cargo.toml +++ b/crates/systemdiff-cli/Cargo.toml @@ -12,7 +12,6 @@ path = "src/main.rs" [dependencies] clap.workspace = true -serde_json.workspace = true systemdiff-core.workspace = true systemdiff-diff.workspace = true systemdiff-report.workspace = true diff --git a/crates/systemdiff-cli/src/main.rs b/crates/systemdiff-cli/src/main.rs index 529415a..f30951b 100644 --- a/crates/systemdiff-cli/src/main.rs +++ b/crates/systemdiff-cli/src/main.rs @@ -3,15 +3,17 @@ use clap::{Parser, Subcommand}; use std::error::Error; use std::fmt; -use std::fs; -use std::io; +use std::fs::File; +use std::io::{self, Read}; use std::path::{Path, PathBuf}; use std::process::ExitCode; -use systemdiff_core::Snapshot; +use systemdiff_core::{Snapshot, decode_snapshot_document}; use systemdiff_diff::{DiffOptions, diff_snapshots}; use systemdiff_report::{write_json, write_terminal}; use systemdiff_windows::mvp_collector_plans; +const MAX_SNAPSHOT_INPUT_BYTES: u64 = 64 * 1024 * 1024; + #[derive(Debug, Parser)] #[command( name = "systemdiff", @@ -93,13 +95,44 @@ fn run(cli: Cli) -> Result<(), Box> { } fn load_snapshot(path: &Path) -> Result { - let bytes = fs::read(path).map_err(|error| { + load_snapshot_with_limit(path, MAX_SNAPSHOT_INPUT_BYTES) +} + +fn load_snapshot_with_limit(path: &Path, maximum_bytes: u64) -> Result { + let file = File::open(path).map_err(|error| { + CliError(format!( + "failed to open snapshot {}: {error}", + path.display() + )) + })?; + + let metadata = file.metadata().map_err(|error| { + CliError(format!( + "failed to inspect snapshot {}: {error}", + path.display() + )) + })?; + validate_snapshot_input_size(path, metadata.len(), maximum_bytes)?; + + let initial_capacity = usize::try_from(metadata.len()).map_err(|_| { CliError(format!( - "failed to read snapshot {}: {error}", + "snapshot file {} cannot be represented on this platform", path.display() )) })?; - serde_json::from_slice(&bytes).map_err(|error| { + let read_limit = maximum_bytes.saturating_add(1); + let mut bytes = Vec::with_capacity(initial_capacity); + file.take(read_limit) + .read_to_end(&mut bytes) + .map_err(|error| { + CliError(format!( + "failed to read snapshot {}: {error}", + path.display() + )) + })?; + validate_snapshot_input_size(path, bytes.len() as u64, maximum_bytes)?; + + decode_snapshot_document(&bytes).map_err(|error| { CliError(format!( "failed to parse snapshot {}: {error}", path.display() @@ -107,6 +140,20 @@ fn load_snapshot(path: &Path) -> Result { }) } +fn validate_snapshot_input_size( + path: &Path, + actual_bytes: u64, + maximum_bytes: u64, +) -> Result<(), CliError> { + if actual_bytes > maximum_bytes { + return Err(CliError(format!( + "snapshot file {} is too large: {actual_bytes} bytes; maximum supported size is {maximum_bytes} bytes", + path.display() + ))); + } + Ok(()) +} + #[derive(Debug)] struct CliError(String); @@ -121,6 +168,20 @@ impl Error for CliError {} #[cfg(test)] mod tests { use super::*; + use std::fs; + use std::sync::atomic::{AtomicU64, Ordering}; + + static TEMP_FILE_COUNTER: AtomicU64 = AtomicU64::new(0); + + fn write_temp_snapshot(contents: &[u8]) -> PathBuf { + let sequence = TEMP_FILE_COUNTER.fetch_add(1, Ordering::Relaxed); + let path = std::env::temp_dir().join(format!( + "systemdiff-cli-test-{}-{sequence}.json", + std::process::id() + )); + fs::write(&path, contents).expect("temporary snapshot must be written"); + path + } #[test] fn parses_json_diff_command() { @@ -142,4 +203,34 @@ mod tests { fn snapshot_command_is_not_advertised_before_collectors_exist() { assert!(Cli::try_parse_from(["systemdiff", "snapshot"]).is_err()); } + + #[test] + fn snapshot_input_size_below_limit_is_accepted() { + assert!(validate_snapshot_input_size(Path::new("snapshot.json"), 63, 64).is_ok()); + } + + #[test] + fn snapshot_input_size_exactly_at_limit_is_accepted() { + assert!(validate_snapshot_input_size(Path::new("snapshot.json"), 64, 64).is_ok()); + } + + #[test] + fn snapshot_input_size_above_limit_is_rejected() { + let error = validate_snapshot_input_size(Path::new("snapshot.json"), 65, 64) + .expect_err("an oversized snapshot must be rejected"); + + assert!(error.0.contains("is too large: 65 bytes")); + assert!(error.0.contains("maximum supported size is 64 bytes")); + } + + #[test] + fn oversized_input_is_rejected_before_snapshot_deserialization() { + let path = write_temp_snapshot(b"{}"); + let result = load_snapshot_with_limit(&path, 1); + fs::remove_file(&path).expect("temporary snapshot must be removed"); + + let error = result.expect_err("metadata above the limit must stop loading"); + assert!(error.0.contains("is too large")); + assert!(!error.0.contains("parse snapshot")); + } } diff --git a/crates/systemdiff-core/Cargo.toml b/crates/systemdiff-core/Cargo.toml index 3d9ebc7..857e49d 100644 --- a/crates/systemdiff-core/Cargo.toml +++ b/crates/systemdiff-core/Cargo.toml @@ -8,6 +8,5 @@ publish = false [dependencies] serde.workspace = true - -[dev-dependencies] serde_json.workspace = true +time.workspace = true diff --git a/crates/systemdiff-core/src/lib.rs b/crates/systemdiff-core/src/lib.rs index b0f0c9f..3afb9ff 100644 --- a/crates/systemdiff-core/src/lib.rs +++ b/crates/systemdiff-core/src/lib.rs @@ -4,11 +4,42 @@ use serde::{Deserialize, Serialize}; use std::collections::{BTreeMap, BTreeSet}; use std::error::Error; use std::fmt; +use time::format_description::well_known::Rfc3339; +use time::{OffsetDateTime, UtcOffset}; pub const SNAPSHOT_DOCUMENT_TYPE: &str = "systemdiff.snapshot"; pub const SNAPSHOT_SCHEMA_VERSION: u32 = 1; pub const REGISTRY_RAW_EVIDENCE_MAX_CAPTURE_BYTES: u64 = 4_096; +#[derive(Debug, Deserialize)] +struct SnapshotDocumentHeader { + document_type: String, + schema_version: u32, +} + +/// Routes a bounded JSON document to a supported Snapshot wire type. +/// +/// The caller owns transport-specific resource limits. This function inspects +/// `document_type` and `schema_version` before constructing the full v1 +/// [`Snapshot`]. +pub fn decode_snapshot_document(input: &[u8]) -> Result { + let header: SnapshotDocumentHeader = + serde_json::from_slice(input).map_err(SnapshotDocumentError::InvalidHeader)?; + + if header.document_type != SNAPSHOT_DOCUMENT_TYPE { + return Err(SnapshotDocumentError::UnexpectedDocumentType { + found: header.document_type, + }); + } + + match header.schema_version { + SNAPSHOT_SCHEMA_VERSION => { + serde_json::from_slice(input).map_err(SnapshotDocumentError::InvalidSnapshotV1) + } + found => Err(SnapshotDocumentError::UnsupportedSchemaVersion { found }), + } +} + #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] pub struct Snapshot { pub document_type: String, @@ -41,6 +72,13 @@ impl Snapshot { if self.captured_at.trim().is_empty() { return Err(SnapshotValidationError::EmptyField("captured_at")); } + let captured_at = OffsetDateTime::parse(&self.captured_at, &Rfc3339) + .map_err(|_| SnapshotValidationError::InvalidCapturedAt)?; + let has_known_utc_designator = + self.captured_at.ends_with('Z') || self.captured_at.ends_with("+00:00"); + if captured_at.offset() != UtcOffset::UTC || !has_known_utc_designator { + return Err(SnapshotValidationError::NonUtcCapturedAt); + } let mut enabled = BTreeSet::new(); for collector_id in &self.enabled_collectors { @@ -327,12 +365,38 @@ pub struct RegistryStartupEntry { pub registry_view: RegistryView, pub key_path: String, pub value_name: String, + pub startup_kind: RegistryStartupKind, + pub run_once_prefix: Option, pub value_type: u32, pub content_sha256: String, pub decoding: RegistryValueDecoding, pub raw_evidence: Option, } +/// Identifies which documented startup key produced an observation. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "snake_case")] +pub enum RegistryStartupKind { + Run, + RunOnce, +} + +/// Structured meaning derived from the complete RunOnce Registry value name. +/// +/// Microsoft documents `!` as deferring value deletion until after the +/// command runs and `*` as allowing execution in Safe Mode. Combined, +/// repeated, and marker-only prefixes remain [`Self::Undocumented`] because +/// their behavior is not documented. The original value name remains the +/// authoritative raw evidence and identity input. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "snake_case")] +pub enum RunOncePrefixSemantics { + NoDocumentedPrefix, + DeferDeletionUntilAfterRun, + RunInSafeMode, + Undocumented, +} + #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] #[serde(tag = "status", rename_all = "snake_case")] pub enum RegistryValueDecoding { @@ -363,6 +427,39 @@ pub struct RegistryRawEvidence { fn validate_registry_startup_entry( entry: &RegistryStartupEntry, ) -> Result<(), SnapshotValidationError> { + let key_kind = entry.key_path.rsplit('\\').next().and_then(|key_name| { + if key_name.eq_ignore_ascii_case("Run") { + Some(RegistryStartupKind::Run) + } else if key_name.eq_ignore_ascii_case("RunOnce") { + Some(RegistryStartupKind::RunOnce) + } else { + None + } + }); + if key_kind != Some(entry.startup_kind) { + return Err(SnapshotValidationError::InvalidRegistryEvidence { + field: "key_path/startup_kind", + }); + } + + match entry.startup_kind { + RegistryStartupKind::Run => { + if entry.run_once_prefix.is_some() { + return Err(SnapshotValidationError::InvalidRegistryEvidence { + field: "startup_kind/run_once_prefix", + }); + } + } + RegistryStartupKind::RunOnce => { + let expected = classify_run_once_prefix(&entry.value_name); + if entry.run_once_prefix != Some(expected) { + return Err(SnapshotValidationError::InvalidRegistryEvidence { + field: "value_name/run_once_prefix", + }); + } + } + } + if !is_lower_hex(&entry.content_sha256, 64) { return Err(SnapshotValidationError::InvalidRegistryEvidence { field: "content_sha256", @@ -401,6 +498,24 @@ fn validate_registry_startup_entry( Ok(()) } +fn classify_run_once_prefix(value_name: &str) -> RunOncePrefixSemantics { + if let Some(remainder) = value_name.strip_prefix('!') { + if remainder.is_empty() || remainder.starts_with('!') || remainder.starts_with('*') { + RunOncePrefixSemantics::Undocumented + } else { + RunOncePrefixSemantics::DeferDeletionUntilAfterRun + } + } else if let Some(remainder) = value_name.strip_prefix('*') { + if remainder.is_empty() || remainder.starts_with('!') || remainder.starts_with('*') { + RunOncePrefixSemantics::Undocumented + } else { + RunOncePrefixSemantics::RunInSafeMode + } + } else { + RunOncePrefixSemantics::NoDocumentedPrefix + } +} + fn decoded_value_matches_type(value_type: u32, value: &RegistryDecodedValue) -> bool { match value { RegistryDecodedValue::String { .. } => value_type == 1, @@ -425,12 +540,34 @@ pub enum RegistryHive { LocalMachine, } +/// SystemDiff's explicit evidence label for the Registry view used to collect a key. +/// +/// A Collector must choose a variant from the target key's documented Windows +/// semantics. It must never infer a stable view from the Collector process +/// bitness. #[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] -#[serde(rename_all = "snake_case")] pub enum RegistryView { + /// The key is documented by Microsoft as shared across WOW64 logical views + /// and is collected once. WOW64 view selectors do not create distinct data. + #[serde(rename = "shared")] Shared, + + /// The key has one system view because no WOW64 alternate logical views + /// exist for that key on the target Windows installation. + /// + /// This must not be used for a redirected key by omitting a WOW64 selector; + /// such a default would vary with Collector process bitness. + #[serde(rename = "native")] Native, + + /// The 32-bit logical Registry view selected explicitly with + /// `KEY_WOW64_32KEY` where alternate views exist. + #[serde(rename = "registry32")] Registry32, + + /// The 64-bit logical Registry view selected explicitly with + /// `KEY_WOW64_64KEY` where alternate views exist. + #[serde(rename = "registry64")] Registry64, } @@ -511,6 +648,45 @@ pub trait Collector { fn collect(&self, context: &CollectionContext) -> CollectionOutcome; } +#[derive(Debug)] +pub enum SnapshotDocumentError { + InvalidHeader(serde_json::Error), + UnexpectedDocumentType { found: String }, + UnsupportedSchemaVersion { found: u32 }, + InvalidSnapshotV1(serde_json::Error), +} + +impl fmt::Display for SnapshotDocumentError { + fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + Self::InvalidHeader(error) => { + write!( + formatter, + "failed to parse snapshot document header: {error}" + ) + } + Self::UnexpectedDocumentType { found } => { + write!(formatter, "unexpected snapshot document type: {found}") + } + Self::UnsupportedSchemaVersion { found } => { + write!(formatter, "unsupported snapshot schema version: {found}") + } + Self::InvalidSnapshotV1(error) => { + write!(formatter, "failed to parse snapshot schema v1: {error}") + } + } + } +} + +impl Error for SnapshotDocumentError { + fn source(&self) -> Option<&(dyn Error + 'static)> { + match self { + Self::InvalidHeader(error) | Self::InvalidSnapshotV1(error) => Some(error), + Self::UnexpectedDocumentType { .. } | Self::UnsupportedSchemaVersion { .. } => None, + } + } +} + #[derive(Debug, Clone, PartialEq, Eq)] pub enum SnapshotValidationError { UnexpectedDocumentType { @@ -520,6 +696,8 @@ pub enum SnapshotValidationError { found: u32, }, EmptyField(&'static str), + InvalidCapturedAt, + NonUtcCapturedAt, InvalidCollectorVersion { collector_id: String, }, @@ -569,6 +747,12 @@ impl fmt::Display for SnapshotValidationError { write!(formatter, "unsupported snapshot schema version: {found}") } Self::EmptyField(field) => write!(formatter, "required field is empty: {field}"), + Self::InvalidCapturedAt => { + formatter.write_str("captured_at is not a valid RFC 3339 timestamp") + } + Self::NonUtcCapturedAt => { + formatter.write_str("captured_at must use known UTC with Z or +00:00") + } Self::InvalidCollectorVersion { collector_id } => { write!(formatter, "collector {collector_id} has version 0") } diff --git a/crates/systemdiff-core/tests/registry_startup.rs b/crates/systemdiff-core/tests/registry_startup.rs new file mode 100644 index 0000000..7489930 --- /dev/null +++ b/crates/systemdiff-core/tests/registry_startup.rs @@ -0,0 +1,211 @@ +use std::collections::BTreeSet; +use systemdiff_core::{ + Artifact, RegistryStartupEntry, RegistryStartupKind, RegistryView, RunOncePrefixSemantics, + Snapshot, SnapshotValidationError, +}; + +fn before_snapshot() -> Snapshot { + serde_json::from_str(include_str!("../../../fixtures/snapshots/before-v1.json")) + .expect("the before fixture must deserialize") +} + +fn registry_entry(snapshot: &mut Snapshot) -> &mut RegistryStartupEntry { + snapshot + .observations + .iter_mut() + .find_map(|observation| match &mut observation.artifact { + Artifact::RegistryStartup(entry) => Some(entry), + _ => None, + }) + .expect("fixture must contain Registry evidence") +} + +#[test] +fn registry_view_serialized_names_are_stable_and_round_trip() { + let cases = [ + (RegistryView::Shared, "shared"), + (RegistryView::Native, "native"), + (RegistryView::Registry32, "registry32"), + (RegistryView::Registry64, "registry64"), + ]; + + for (view, name) in cases { + let json = serde_json::to_string(&view).expect("Registry view must serialize"); + assert_eq!(json, format!("\"{name}\"")); + let reparsed: RegistryView = + serde_json::from_str(&json).expect("Registry view must deserialize"); + assert_eq!(reparsed, view); + } + + assert!(serde_json::from_str::("\"process_default\"").is_err()); +} + +#[test] +fn registry_startup_semantic_names_are_stable() { + for (kind, name) in [ + (RegistryStartupKind::Run, "run"), + (RegistryStartupKind::RunOnce, "run_once"), + ] { + assert_eq!( + serde_json::to_string(&kind).expect("startup kind must serialize"), + format!("\"{name}\"") + ); + } + + for (semantics, name) in [ + ( + RunOncePrefixSemantics::NoDocumentedPrefix, + "no_documented_prefix", + ), + ( + RunOncePrefixSemantics::DeferDeletionUntilAfterRun, + "defer_deletion_until_after_run", + ), + (RunOncePrefixSemantics::RunInSafeMode, "run_in_safe_mode"), + (RunOncePrefixSemantics::Undocumented, "undocumented"), + ] { + assert_eq!( + serde_json::to_string(&semantics).expect("prefix semantics must serialize"), + format!("\"{name}\"") + ); + } +} + +#[test] +fn documented_run_once_prefixes_validate_and_round_trip() { + let cases = [ + ("Foo", RunOncePrefixSemantics::NoDocumentedPrefix), + ("!Foo", RunOncePrefixSemantics::DeferDeletionUntilAfterRun), + ("*Foo", RunOncePrefixSemantics::RunInSafeMode), + ]; + + for (value_name, semantics) in cases { + let mut snapshot = before_snapshot(); + let entry = registry_entry(&mut snapshot); + entry.key_path = "Software\\Microsoft\\Windows\\CurrentVersion\\RunOnce".to_owned(); + entry.value_name = value_name.to_owned(); + entry.startup_kind = RegistryStartupKind::RunOnce; + entry.run_once_prefix = Some(semantics); + + snapshot + .validate() + .unwrap_or_else(|error| panic!("{value_name} must validate: {error}")); + let json = serde_json::to_string(&snapshot).expect("Snapshot must serialize"); + let reparsed: Snapshot = serde_json::from_str(&json).expect("Snapshot must deserialize"); + assert_eq!(reparsed, snapshot); + } +} + +#[test] +fn undocumented_run_once_prefix_forms_remain_uninterpreted() { + for value_name in ["!*Foo", "*!Foo", "!!Foo", "**Foo", "!", "*"] { + let mut snapshot = before_snapshot(); + let entry = registry_entry(&mut snapshot); + entry.key_path = "Software\\Microsoft\\Windows\\CurrentVersion\\RunOnce".to_owned(); + entry.value_name = value_name.to_owned(); + entry.startup_kind = RegistryStartupKind::RunOnce; + entry.run_once_prefix = Some(RunOncePrefixSemantics::Undocumented); + + snapshot + .validate() + .unwrap_or_else(|error| panic!("{value_name} must remain valid raw evidence: {error}")); + assert_eq!( + registry_entry(&mut snapshot).value_name.as_str(), + value_name + ); + } +} + +#[test] +fn inconsistent_run_once_evidence_is_rejected() { + let cases = [ + ("!Foo", Some(RunOncePrefixSemantics::NoDocumentedPrefix)), + ( + "*Foo", + Some(RunOncePrefixSemantics::DeferDeletionUntilAfterRun), + ), + ("Foo", Some(RunOncePrefixSemantics::RunInSafeMode)), + ("Foo", None), + ]; + + for (value_name, semantics) in cases { + let mut snapshot = before_snapshot(); + let entry = registry_entry(&mut snapshot); + entry.key_path = "Software\\Microsoft\\Windows\\CurrentVersion\\RunOnce".to_owned(); + entry.value_name = value_name.to_owned(); + entry.startup_kind = RegistryStartupKind::RunOnce; + entry.run_once_prefix = semantics; + + assert!(matches!( + snapshot.validate(), + Err(SnapshotValidationError::InvalidRegistryEvidence { + field: "value_name/run_once_prefix" + }) + )); + } + + let mut run_snapshot = before_snapshot(); + registry_entry(&mut run_snapshot).run_once_prefix = + Some(RunOncePrefixSemantics::NoDocumentedPrefix); + assert!(matches!( + run_snapshot.validate(), + Err(SnapshotValidationError::InvalidRegistryEvidence { + field: "startup_kind/run_once_prefix" + }) + )); + + let mut mismatched_key = before_snapshot(); + registry_entry(&mut mismatched_key).startup_kind = RegistryStartupKind::RunOnce; + assert!(matches!( + mismatched_key.validate(), + Err(SnapshotValidationError::InvalidRegistryEvidence { + field: "key_path/startup_kind" + }) + )); +} + +#[test] +fn full_run_once_value_names_keep_distinct_observation_identities() { + let mut snapshot = before_snapshot(); + let template_index = snapshot + .observations + .iter() + .position(|observation| matches!(&observation.artifact, Artifact::RegistryStartup(_))) + .expect("fixture must contain Registry evidence"); + let template = snapshot.observations.remove(template_index); + + let cases = [ + ("Foo", RunOncePrefixSemantics::NoDocumentedPrefix), + ("!Foo", RunOncePrefixSemantics::DeferDeletionUntilAfterRun), + ("*Foo", RunOncePrefixSemantics::RunInSafeMode), + ]; + for (value_name, semantics) in cases { + let mut observation = template.clone(); + observation.canonical_id = format!( + "hkcu|shared|software\\microsoft\\windows\\currentversion\\runonce|{}", + value_name.to_ascii_lowercase() + ); + let Artifact::RegistryStartup(entry) = &mut observation.artifact else { + unreachable!("template must contain Registry evidence"); + }; + entry.key_path = "Software\\Microsoft\\Windows\\CurrentVersion\\RunOnce".to_owned(); + entry.value_name = value_name.to_owned(); + entry.startup_kind = RegistryStartupKind::RunOnce; + entry.run_once_prefix = Some(semantics); + snapshot.observations.push(observation); + } + + snapshot + .validate() + .expect("full prefixed names must remain distinct identities"); + let keys: BTreeSet<_> = snapshot + .observations + .iter() + .filter(|observation| matches!(&observation.artifact, Artifact::RegistryStartup(_))) + .map(|observation| observation.key()) + .collect(); + assert_eq!(keys.len(), 3); + assert!(keys.iter().any(|key| key.canonical_id.ends_with("|foo"))); + assert!(keys.iter().any(|key| key.canonical_id.ends_with("|!foo"))); + assert!(keys.iter().any(|key| key.canonical_id.ends_with("|*foo"))); +} diff --git a/crates/systemdiff-core/tests/snapshot_document.rs b/crates/systemdiff-core/tests/snapshot_document.rs new file mode 100644 index 0000000..8da067f --- /dev/null +++ b/crates/systemdiff-core/tests/snapshot_document.rs @@ -0,0 +1,57 @@ +use systemdiff_core::{SnapshotDocumentError, decode_snapshot_document}; + +#[test] +fn supported_snapshot_v1_is_routed_to_the_current_wire_type() { + let snapshot = + decode_snapshot_document(include_bytes!("../../../fixtures/snapshots/before-v1.json")) + .expect("the v1 fixture must be routed and decoded"); + + assert_eq!(snapshot.document_type, "systemdiff.snapshot"); + assert_eq!(snapshot.schema_version, 1); +} + +#[test] +fn unknown_document_type_is_rejected_before_snapshot_body_construction() { + let error = + decode_snapshot_document(br#"{"document_type":"systemdiff.diff","schema_version":1}"#) + .expect_err("another document family must not route to Snapshot v1"); + + assert!(matches!( + error, + SnapshotDocumentError::UnexpectedDocumentType { ref found } + if found == "systemdiff.diff" + )); +} + +#[test] +fn unsupported_schema_is_rejected_before_snapshot_body_construction() { + let error = + decode_snapshot_document(br#"{"document_type":"systemdiff.snapshot","schema_version":2}"#) + .expect_err("an unsupported Snapshot schema must not use the v1 wire type"); + + assert!(matches!( + error, + SnapshotDocumentError::UnsupportedSchemaVersion { found: 2 } + )); +} + +#[test] +fn supported_header_with_incomplete_body_reports_a_v1_body_error() { + let error = + decode_snapshot_document(br#"{"document_type":"systemdiff.snapshot","schema_version":1}"#) + .expect_err("the supported route must still require a complete v1 body"); + + assert!(matches!(error, SnapshotDocumentError::InvalidSnapshotV1(_))); +} + +#[test] +fn malformed_or_missing_header_is_rejected_as_a_header_error() { + for input in [ + br#"{"document_type":"systemdiff.snapshot"}"#.as_slice(), + br#"{"document_type": "systemdiff.snapshot""#.as_slice(), + ] { + let error = decode_snapshot_document(input) + .expect_err("a malformed document header must be rejected"); + assert!(matches!(error, SnapshotDocumentError::InvalidHeader(_))); + } +} diff --git a/crates/systemdiff-core/tests/snapshot_fixture.rs b/crates/systemdiff-core/tests/snapshot_fixture.rs index 89af583..f500529 100644 --- a/crates/systemdiff-core/tests/snapshot_fixture.rs +++ b/crates/systemdiff-core/tests/snapshot_fixture.rs @@ -1,7 +1,7 @@ use systemdiff_core::{ Artifact, CollectorStatus, REGISTRY_RAW_EVIDENCE_MAX_CAPTURE_BYTES, RegistryDecodedValue, - RegistryHive, RegistryRawEvidence, RegistryStartupEntry, RegistryValueDecoding, RegistryView, - Snapshot, SnapshotValidationError, + RegistryHive, RegistryRawEvidence, RegistryStartupEntry, RegistryStartupKind, + RegistryValueDecoding, RegistryView, Snapshot, SnapshotValidationError, }; fn before_snapshot() -> Snapshot { @@ -20,6 +20,59 @@ fn draft_v1_fixture_validates_and_round_trips() { assert_eq!(reparsed, snapshot); } +#[test] +fn captured_at_accepts_supported_utc_rfc3339_forms() { + for captured_at in [ + "2026-08-11T00:00:00Z", + "2026-08-11T00:00:00+00:00", + "2026-08-11T00:00:00.123456789Z", + ] { + let mut snapshot = before_snapshot(); + snapshot.captured_at = captured_at.to_owned(); + snapshot + .validate() + .unwrap_or_else(|error| panic!("{captured_at} must be accepted: {error}")); + } +} + +#[test] +fn captured_at_rejects_non_utc_and_unknown_offsets() { + for captured_at in ["2026-08-11T08:00:00+08:00", "2026-08-11T00:00:00-00:00"] { + let mut snapshot = before_snapshot(); + snapshot.captured_at = captured_at.to_owned(); + assert_eq!( + snapshot.validate(), + Err(SnapshotValidationError::NonUtcCapturedAt), + "{captured_at} must not be accepted as known UTC" + ); + } +} + +#[test] +fn captured_at_rejects_empty_malformed_and_invalid_dates() { + let cases = [ + ("", SnapshotValidationError::EmptyField("captured_at")), + ( + "2026-08-11T00:00:00", + SnapshotValidationError::InvalidCapturedAt, + ), + ( + "2026-02-30T00:00:00Z", + SnapshotValidationError::InvalidCapturedAt, + ), + ( + "2026-08-11T24:00:00Z", + SnapshotValidationError::InvalidCapturedAt, + ), + ]; + + for (captured_at, expected) in cases { + let mut snapshot = before_snapshot(); + snapshot.captured_at = captured_at.to_owned(); + assert_eq!(snapshot.validate(), Err(expected), "case: {captured_at}"); + } +} + #[test] fn registry_fixture_uses_typed_decoding_without_raw_evidence() { for fixture in [ @@ -32,6 +85,8 @@ fn registry_fixture_uses_typed_decoding_without_raw_evidence() { let Artifact::RegistryStartup(entry) = observation.artifact else { continue; }; + assert_eq!(entry.startup_kind, RegistryStartupKind::Run); + assert_eq!(entry.run_once_prefix, None); assert_eq!(entry.value_type, 1); assert_eq!(entry.content_sha256.len(), 64); assert!(entry.raw_evidence.is_none()); @@ -74,8 +129,10 @@ fn registry_decoding_round_trips_typed_values_and_compact_raw_evidence() { let entry = RegistryStartupEntry { hive: RegistryHive::CurrentUser, registry_view: RegistryView::Shared, - key_path: "Software\\Example".to_owned(), + key_path: "Software\\Microsoft\\Windows\\CurrentVersion\\Run".to_owned(), value_name: "Synthetic".to_owned(), + startup_kind: RegistryStartupKind::Run, + run_once_prefix: None, value_type, content_sha256: "0".repeat(64), decoding: RegistryValueDecoding::Decoded { @@ -93,8 +150,10 @@ fn registry_decoding_round_trips_typed_values_and_compact_raw_evidence() { let entry = RegistryStartupEntry { hive: RegistryHive::CurrentUser, registry_view: RegistryView::Shared, - key_path: "Software\\Example".to_owned(), + key_path: "Software\\Microsoft\\Windows\\CurrentVersion\\Run".to_owned(), value_name: "Synthetic".to_owned(), + startup_kind: RegistryStartupKind::Run, + run_once_prefix: None, value_type: 4, content_sha256: "e8a4b2ee7ede79a3afb332b5b6cc3d952a65fd8cffb897f5d18016577c33d7cc" .to_owned(), diff --git a/crates/systemdiff-risk/src/lib.rs b/crates/systemdiff-risk/src/lib.rs index e5d4f0f..36479f9 100644 --- a/crates/systemdiff-risk/src/lib.rs +++ b/crates/systemdiff-risk/src/lib.rs @@ -45,7 +45,7 @@ pub trait Rule { mod tests { use super::*; use systemdiff_core::{ - ArtifactKey, RegistryDecodedValue, RegistryHive, RegistryStartupEntry, + ArtifactKey, RegistryDecodedValue, RegistryHive, RegistryStartupEntry, RegistryStartupKind, RegistryValueDecoding, RegistryView, }; use systemdiff_diff::ChangeKind; @@ -94,8 +94,10 @@ mod tests { after: systemdiff_core::Artifact::RegistryStartup(RegistryStartupEntry { hive: RegistryHive::CurrentUser, registry_view: RegistryView::Shared, - key_path: "Software\\Example".to_owned(), + key_path: "Software\\Microsoft\\Windows\\CurrentVersion\\Run".to_owned(), value_name: "Synthetic".to_owned(), + startup_kind: RegistryStartupKind::Run, + run_once_prefix: None, value_type: 1, content_sha256: "04c9e304d22dd63d40474ebbb8ca4cb383a68b755876aced0b32ad9e54ec82bf" diff --git a/docs/architecture.md b/docs/architecture.md index 1795eb7..c8cf8fd 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -50,6 +50,8 @@ A Snapshot is an observed state at a point in time. It records document/schema v Snapshots do not claim to be atomic. Coverage and concurrent-change diagnostics are evidence needed to interpret a later diff. +Snapshot files are untrusted input. The CLI enforces a fixed 64 MiB file ceiling with metadata preflight and a bounded read before handing bytes to core. Core then inspects `document_type` and `schema_version` before constructing the supported v1 wire type. This is an intentionally small synchronous boundary, not a streaming parser or general resource-policy framework. + ### Collector A Collector has a stable ID, version, description, privilege expectations, and synchronous `collect` operation. Synchronous collection matches the blocking Win32/COM APIs in the MVP and avoids introducing an async runtime before concurrency is required. @@ -137,6 +139,7 @@ Important choices were checked on 2026-08-11. Re-evaluate them when introduced o | --- | --- | --- | | Rust/Cargo | Stable toolchain; Rust 2024 workspace resolver | Accepted for shared core, memory safety, native distribution, and strong test tooling | | `serde` / `serde_json` | Active; Apache-2.0 OR MIT; [official repository](https://github.com/serde-rs/serde) | Accepted for explicit versioned JSON wire types | +| `time` | Active; Apache-2.0 OR MIT; [official repository](https://github.com/time-rs/time) | Accepted in `systemdiff-core` with only the parsing feature for standards-based RFC 3339 validation; clock, local-offset, formatting, and serde features remain disabled | | `windows-rs` | Microsoft-maintained and active; Apache-2.0 OR MIT; [official repository](https://github.com/microsoft/windows-rs) | Planned for collectors; enable only required API features and prefer typed bindings | | `clap` | Active; Apache-2.0 OR MIT; [official repository](https://github.com/clap-rs/clap) | Accepted at the CLI boundary only | | Tauri 2 | Active; Apache-2.0 OR MIT; requires C++ Build Tools and WebView2 on Windows; [prerequisites](https://v2.tauri.app/start/prerequisites/) | Proposed for v0.2; excluded from v0.1 build until a security-focused spike | diff --git a/docs/collectors.md b/docs/collectors.md index 62bc47e..06fbba0 100644 --- a/docs/collectors.md +++ b/docs/collectors.md @@ -19,13 +19,21 @@ Implementation source: - `RegOpenKeyExW`, `RegQueryInfoKeyW`, and `RegEnumValueW`. - On 64-bit Windows, enumerate HKLM `Software` in explicit 64-bit and 32-bit views; do not address `Wow6432Node` directly. - On Windows 7 and later, HKCU `Software` is shared and is collected once. +- Emit `Registry32` only after explicitly selecting `KEY_WOW64_32KEY`, and emit `Registry64` only after explicitly selecting `KEY_WOW64_64KEY`. These labels describe logical Registry views, not processor-specific physical stores. +- Emit `Shared` only for keys Microsoft documents as shared. Emit `Native` only when the target key has one system view and no WOW64 alternate logical views. A Collector must never use `Native` as a shortcut for omitting a selector on a redirected key, because the resulting default depends on process bitness. - Preserve the numeric Registry type, a typed safe-decode outcome, original unexpanded `REG_EXPAND_SZ`, and original names/casing. Do not assume every value is UTF-16LE text. - Compute SHA-256 over the complete native value bytes before decoding or truncation so undecoded values still compare reliably. - Retain a raw prefix only when it adds concrete forensic value, encode it as lowercase hex, enforce the 4 KiB schema limit, and record truncation; ordinary decoded Run values do not need duplicated raw payloads by default. - Treat value order as unstable and bound any retry when a key changes during enumeration. - A missing Run key is a complete empty scope, not an error. -RunOnce `!` and `*` prefixes are recorded as derived evidence. Environment expansion, command-line parsing, executable discovery, hashing, and signature checks are later enrichment stages. +RunOnce prefix evidence follows Microsoft's documented value-name behavior: + +- By default, a RunOnce value is deleted before its command runs. A leading `!` defers deletion until after the command runs. +- Run and RunOnce keys are ignored in Safe Mode by default. A leading `*` makes a RunOnce value run in Safe Mode. +- Microsoft does not define combined, repeated, or marker-only prefix forms. Preserve those complete names as raw evidence and mark their structured interpretation `undocumented`; do not infer that both documented behaviors apply. + +The complete value name, including `!` or `*`, remains evidence and part of canonical identity. `Foo`, `!Foo`, and `*Foo` cannot collapse to the same observation. Environment expansion, command-line parsing, executable discovery, hashing, and signature checks are later enrichment stages. Official references: [Run and RunOnce](https://learn.microsoft.com/windows/win32/setupapi/run-and-runonce-registry-keys), [WOW64 affected keys](https://learn.microsoft.com/windows/win32/winprog64/shared-registry-keys), [alternate registry views](https://learn.microsoft.com/windows/win32/winprog64/accessing-an-alternate-registry-view), [RegEnumValueW](https://learn.microsoft.com/windows/win32/api/winreg/nf-winreg-regenumvaluew). diff --git a/docs/data-format.md b/docs/data-format.md index 6ff366e..a8469c3 100644 --- a/docs/data-format.md +++ b/docs/data-format.md @@ -21,12 +21,16 @@ A diff uses `systemdiff.diff` and its own schema version. Application versions a Readers inspect `document_type` and `schema_version` before deserializing the remaining body. Unknown major versions are rejected with a clear error. Additive top-level metadata may be tolerated, but an unknown typed artifact is never silently discarded. +The CLI accepts Snapshot files up to 64 MiB. It checks metadata before allocating the full input, performs a bounded read of at most the supported maximum plus one byte, and rechecks the actual byte count before JSON decoding. This fixed ceiling provides headroom for targeted v0.1 evidence while bounding parser amplification; it is not a streaming parser or a configurable resource-policy framework. + +After the bounded read, the core first deserializes a minimal header containing only `document_type` and `schema_version`. Only `systemdiff.snapshot` schema v1 is then routed to the current `Snapshot` wire type. This header pass still scans the bounded JSON to skip unrelated fields; it does not construct a generic JSON DOM or the full Snapshot body before routing. + ## Snapshot requirements A snapshot records: - document and schema version; -- SystemDiff version and UTC RFC 3339 capture time; +- SystemDiff version and a valid UTC RFC 3339 capture time; - Windows version/build/architecture when available, without requiring hostname or machine ID; - privilege/elevation state; - enabled Collector IDs; @@ -36,14 +40,36 @@ A snapshot records: Diagnostics include a stable code, collection stage, and optional Win32/HRESULT numeric value. Localized error messages are for humans only. +Readers accept known UTC expressed with `Z` or `+00:00` and reject non-zero offsets. RFC 3339 `-00:00` means that the local offset is unknown, so it is not accepted as a known UTC assertion. Snapshot readers preserve the original valid wire string; future SystemDiff-generated Snapshots will emit canonical `Z`. + ## Registry value evidence Registry artifacts preserve the value name and the numeric Windows Registry type code independently from interpretation. The draft typed decoding supports strings, unexpanded expandable strings, multi-strings, DWORDs, and QWORDs; binary, unknown, or malformed values can remain undecoded with an explicit `not_applicable`, `unsupported_type`, or `invalid_data` status. The type code remains authoritative, and decoded kinds are validated against it, so a future Collector never needs to coerce every value into UTF-16LE text. +Each startup artifact explicitly identifies whether it came from a `run` or `run_once` key. A Run entry has no RunOnce prefix semantics. A RunOnce entry records exactly one structured interpretation derived from the complete value name: + +- `no_documented_prefix` for an ordinary name; +- `defer_deletion_until_after_run` for a leading `!`; +- `run_in_safe_mode` for a leading `*`; +- `undocumented` for combined, repeated, or marker-only prefix forms that Microsoft does not define. + +The structured interpretation never replaces or strips the complete `value_name`. The full name, including any prefix, remains raw evidence and part of the Collector-owned canonical identity. `Foo`, `!Foo`, and `*Foo` therefore remain distinct observations. The draft schema rejects a `startup_kind` inconsistent with the final `Run`/`RunOnce` key-path component, a Run entry carrying RunOnce semantics, or a RunOnce interpretation inconsistent with its raw name. + Every Registry artifact includes lowercase SHA-256 of the complete native value bytes. This keeps two undecoded values distinguishable even when neither retains raw bytes, and keeps truncated prefixes from hiding a changed suffix. Optional raw evidence is limited to the first 4 KiB, records captured/original byte counts and truncation, and uses validated lowercase hex rather than JSON arrays of byte integers. Hex costs two JSON characters per byte but needs no ambiguous binary codec and remains substantially smaller than integer arrays. Hashes and raw prefixes are still sensitive: low-entropy values can be guessed, and Registry data may contain paths, usernames, commands, or secrets. Raw evidence is included only for a concrete forensic reason rather than duplicated for every decoded value, and both forms must be covered by redaction/share policy. +### Registry view labels + +`registry_view` records the Windows view actually used for acquisition: + +- `shared`: Microsoft documents the key as shared across WOW64 logical views; it is collected once. +- `registry32`: the 32-bit logical view selected explicitly with `KEY_WOW64_32KEY` where alternate views exist. +- `registry64`: the 64-bit logical view selected explicitly with `KEY_WOW64_64KEY` where alternate views exist. +- `native`: the sole view for a key on a Windows installation where that key has no WOW64 alternate logical views. + +`native` never means “omit a WOW64 selector and use the Collector process default” for a redirected key. That behavior changes with process bitness and is not stable evidence. + ## Observation identity The logical key is: @@ -54,6 +80,8 @@ collector_id + scope_id + artifact_kind + canonical_identity Collectors own canonicalization and version it through their Collector version. Raw casing and display values remain in evidence. Duplicate keys make the snapshot invalid for diffing. +For Registry startup entries, the complete raw value name participates in canonical identity. Prefixes are not stripped or normalized into a prefix-independent identity. + ## Diff semantics Output order is stable. A change contains a deterministic document-local opaque change ID, artifact key, and one of: @@ -68,6 +96,8 @@ Missing evidence becomes Added/Removed only when the relevant scope is complete If the same Collector ID has different Collector versions in the two snapshots, draft v1 rejects the diff instead of guessing compatibility. This is the current conservative default, not a permanent claim that cross-version comparison is impossible. A future version may explicitly register and test backward-compatible comparison semantics for particular Collector versions; no compatibility or migration framework exists yet. A Collector that exists on only one side remains a local coverage problem and produces Inconclusive absence where evidence is affected. +Draft v0.1 comparisons require Snapshot A and B to come from the same Windows installation and the same user/principal context. Cross-host and cross-user comparison is not supported, and the wire format does not introduce a persistent machine token or SID identity subsystem. + Change IDs are opaque references within a Diff document. They must not embed canonical paths, usernames, or other evidence because redaction cannot safely remove data that is duplicated into an identifier. ## Redaction metadata diff --git a/docs/threat-model.md b/docs/threat-model.md index 6d5f8ad..ecec8db 100644 --- a/docs/threat-model.md +++ b/docs/threat-model.md @@ -51,7 +51,7 @@ Open questions that may change rankings: ### Data flows and trust boundaries - Windows OS -> Windows collectors: registry bytes, service buffers, task COM/XML via local Win32/COM; current-token ACLs apply; safe wrappers must validate lengths, encodings, HRESULT/Win32 outcomes, and concurrent mutation. -- Snapshot files -> CLI/core parser: attacker-controlled local JSON via file I/O; document header, size/resource limits, typed schema, identity uniqueness, and count limits are required before expensive processing. +- Snapshot files -> CLI/core parser: attacker-controlled local JSON via file I/O; a 64 MiB bounded read and header-first schema route precede full Snapshot construction, while typed validation and identity uniqueness protect later processing. Finer object, string, nesting, and count limits remain future hardening. - Core evidence -> diff/rules: typed in-process values; compatibility, coverage, deterministic identity, and no evidence execution are the guarantees. - Diff/findings -> report files/terminal: privacy-sensitive local output; destination choice is user-controlled, and future sanitization must be explicit. - Future WebView -> Tauri core: typed local IPC; bundled origin, allowlisted commands, least-privilege capabilities, and input validation are required. @@ -135,7 +135,7 @@ flowchart LR | Threat ID | Threat source | Prerequisites | Threat action | Impact | Impacted assets | Existing controls (evidence) | Gaps | Recommended mitigations | Detection ideas | Likelihood | Impact severity | Priority | | --- | --- | --- | --- | --- | --- | --- | --- | --- | --- | --- | --- | --- | | TM-001 | Local changed software | Can race, deny, or influence a collected scope | Cause incomplete evidence to appear complete | False Added/Removed and incorrect findings | Diff and coverage integrity | Explicit statuses and no-false-removal rule (`docs/adr/0004-collector-failure-and-coverage.md`) | Collectors not implemented | Per-scope coverage, bounded retry, native diagnostics, complete->partial regression fixtures | Count coverage changes and surface prominent report warnings | High | High | High | -| TM-002 | Snapshot provider | Can supply arbitrary local JSON | Exhaust memory/CPU or exploit parser assumptions | CLI denial of service; misleading evidence | Availability and diff integrity | Typed/versioned schema and duplicate rejection (`docs/data-format.md`) | No input limits yet | File-size, object-count, string-size and nesting limits; validate header before body; fuzz parsers later | Structured parse failure codes; resource-limit tests | Medium | Medium | Medium | +| TM-002 | Snapshot provider | Can supply arbitrary local JSON | Exhaust memory/CPU or exploit parser assumptions | CLI denial of service; misleading evidence | Availability and diff integrity | 64 MiB bounded file read, header-first version routing, typed schema, and duplicate rejection (`docs/data-format.md`) | No object-count, string-size, nesting, or parser-fuzz limits yet | Add finer structural limits only from measured need; fuzz parsers later | Structured parse failure codes; resource-limit tests | Medium | Medium | Medium | | TM-003 | Windows data/API edge case | Collector handles raw buffers/COM values | Trigger unsafe length, lifetime, or encoding bug | Crash, memory corruption, corrupted evidence | Token, evidence, availability | Unsafe isolated to Windows crate (`docs/architecture.md`) | Native adapters absent | Minimal audited unsafe blocks, `windows-rs`, two-call buffer patterns, RAII handles/COM, fuzz pure decoders | Windows crash fixtures and sanitizer/fuzz jobs where practical | Medium | High | High | | TM-004 | User/workflow mistake | Real report is shared publicly | Publish sensitive raw evidence | Lasting privacy disclosure | Snapshot/report confidentiality | Sensitive-by-default and redaction metadata (`docs/product-principles.md`) | Sanitizer absent | Blocking share warning in UI, documented manual review, policy-versioned pure sanitizer, synthetic issue fixtures | Scan project issues for accidental reports; sanitizer golden tests | High | High | High | | TM-005 | Future compromised WebView | Desktop exposes broad command/capability | Invoke native execution/write or read excess data | Privilege misuse and boundary violation | Token, host integrity, evidence confidentiality | Proposed narrow IPC (`docs/adr/0003-desktop-stack.md`) | Desktop not yet threat-tested | Bundled content, restrictive CSP, explicit commands, no generic shell/fs/http plugins, command authorization tests | Log command IDs without sensitive payloads; capability review in CI | Low pre-v0.2 | High | Medium | diff --git a/fixtures/snapshots/after-v1.json b/fixtures/snapshots/after-v1.json index e98c3a1..0216cac 100644 --- a/fixtures/snapshots/after-v1.json +++ b/fixtures/snapshots/after-v1.json @@ -76,6 +76,8 @@ "registry_view": "shared", "key_path": "Software\\Microsoft\\Windows\\CurrentVersion\\Run", "value_name": "ExampleApp", + "startup_kind": "run", + "run_once_prefix": null, "value_type": 1, "content_sha256": "e3f7174dd4ae12dc6be7e6d17d598e035cd03bf994008d3e8866c15183640f35", "decoding": { @@ -101,6 +103,8 @@ "registry_view": "shared", "key_path": "Software\\Microsoft\\Windows\\CurrentVersion\\Run", "value_name": "ExampleUpdater", + "startup_kind": "run", + "run_once_prefix": null, "value_type": 1, "content_sha256": "667b845250ce31564390d74904e9fd04f1a03f8dd99a6429a9bfda8fbd4efb2e", "decoding": { diff --git a/fixtures/snapshots/before-v1.json b/fixtures/snapshots/before-v1.json index 184f470..d9cf9a2 100644 --- a/fixtures/snapshots/before-v1.json +++ b/fixtures/snapshots/before-v1.json @@ -76,6 +76,8 @@ "registry_view": "shared", "key_path": "Software\\Microsoft\\Windows\\CurrentVersion\\Run", "value_name": "ExampleApp", + "startup_kind": "run", + "run_once_prefix": null, "value_type": 1, "content_sha256": "b27cce45267b6100cdd3267ec6dcdf2023e6846bb3f0f66162ab179678f0727c", "decoding": {