From 4e24dcb68a49037ee64c087514eca4e0524f7ebe Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Thu, 6 Aug 2026 21:10:49 -0400 Subject: [PATCH 1/4] Add failing tests for run-spec redaction corruption MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The entropy redactor rewrites NAME= assignment pairs to a bare REDACTED, and the worker rehydrates its executable RunSpec from the projection folded from redacted stored events. Together these broke Daytona snapshot builds for any run definition whose inline Dockerfile pins a git SHA: the spec came back as `ARG REDACTED`, the build died on the unset variable under `set -eu`, and the environment's snapshot identity silently changed. Pin the intended contracts with red tests: - fabro-redact: an assignment whose value alone is below the entropy threshold survives redaction (pure hex cannot exceed 4.0 bits; only the name+value charset merge crosses 4.5), and a genuinely high-entropy value is redacted without destroying the key name. - fabro-workflow: the spec that load_from_store rehydrates round-trips byte-identical through the store, including content that looks like a secret — event redaction must not reach execution. Co-Authored-By: Claude Fable 5 --- .../fabro-workflow/src/pipeline/persist.rs | 44 +++++++++++++++++++ lib/foundation/fabro-redact/src/lib.rs | 21 +++++++++ 2 files changed, 65 insertions(+) diff --git a/lib/components/fabro-workflow/src/pipeline/persist.rs b/lib/components/fabro-workflow/src/pipeline/persist.rs index cc12c3cba..846f9b853 100644 --- a/lib/components/fabro-workflow/src/pipeline/persist.rs +++ b/lib/components/fabro-workflow/src/pipeline/persist.rs @@ -272,6 +272,50 @@ mod tests { assert!(loaded.diagnostics().is_empty()); } + #[tokio::test] + async fn load_from_store_preserves_high_entropy_dockerfile_content() { + // The spec the worker executes must survive the store byte-identical. + // Event redaction is a storage/display concern; when it reaches the + // spec that `load_from_store` rehydrates, the sandbox builds a + // corrupted Dockerfile: `ARG NAME=` pairs come back as + // `ARG REDACTED`, the build's `set -eu` step fails on the unset + // variable, and the environment's snapshot identity silently changes. + let temp = tempfile::tempdir().unwrap(); + let run_dir = temp.path().join("run"); + std::fs::create_dir_all(&run_dir).unwrap(); + let (graph, source) = graph_and_source(); + + // Two shapes that must both survive: the hex pins that triggered the + // production failure, and a token high-entropy enough that any + // detector will keep flagging it in stored events. The second keeps + // this test red until execution stops reading redacted content, + // independent of how the entropy heuristic evolves. + let dockerfile = "FROM buildpack-deps:noble\n\ + ARG DOCKER_INSTALL_COMMIT=5ce20f2eef3615d08fea941eda5a109e949e8ebf\n\ + ARG DOCKER_INSTALL_SHA256=b991f2806186f7287bb9e53362060c382e906d154599b2fb0982f34246bacfd4\n\ + ENV CACHE_SALT=xK9mZ2vL8nQ5rT1wY4bC7dF0gH3jE6p\n\ + RUN install-docker \"${DOCKER_INSTALL_COMMIT}\" \"${DOCKER_INSTALL_SHA256}\"\n"; + + let mut record = sample_record(different_graph()); + record.graph = graph; + record.settings.run.environment.image.dockerfile = Some( + fabro_types::settings::run::DockerfileSource::Inline(dockerfile.to_string()), + ); + + let run_store = seeded_store(&record, Some(&source)).await; + let loaded = load_from_store(&run_store.clone().into(), &run_dir) + .await + .unwrap(); + + assert_eq!( + loaded.run_spec().settings.run.environment.image.dockerfile, + Some(fabro_types::settings::run::DockerfileSource::Inline( + dockerfile.to_string() + )), + "the executable run spec must round-trip through the store unredacted" + ); + } + #[test] fn persist_returns_error_on_io_failure() { let temp = tempfile::tempdir().unwrap(); diff --git a/lib/foundation/fabro-redact/src/lib.rs b/lib/foundation/fabro-redact/src/lib.rs index 8ed562e53..4a7efa45b 100644 --- a/lib/foundation/fabro-redact/src/lib.rs +++ b/lib/foundation/fabro-redact/src/lib.rs @@ -111,6 +111,27 @@ mod tests { assert_eq!(result, "key=REDACTED"); } + #[test] + fn redact_string_keeps_assignment_with_low_entropy_value() { + // A pinned git SHA is pure hex, so the value alone can never exceed + // 4.0 bits of entropy. Only the merged NAME=value token crosses the + // 4.5-bit threshold, because the uppercase name widens the charset. + // Measuring the name together with the value redacts innocuous + // pins; the pair must survive. + let input = "ARG DOCKER_INSTALL_COMMIT=5ce20f2eef3615d08fea941eda5a109e949e8ebf"; + assert_eq!(redact_string(input), input); + } + + #[test] + fn redact_string_keeps_assignment_key_for_high_entropy_value() { + // The value alone is above the entropy threshold, so it is + // redacted either way — but the name says which setting was + // redacted and must survive, as the gitleaks layer already + // does for `key=REDACTED`. + let result = redact_string("BUILD_STAMP=xK9mZ2vL8nQ5rT1wY4bC7dF0gH3jE6p"); + assert_eq!(result, "BUILD_STAMP=REDACTED"); + } + #[test] fn redact_string_overlapping_detections_produce_single_redacted() { // A high-entropy string that also matches a gitleaks pattern From 3421c4f06fb77af09cc33b33bb57cbfbd226c752 Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Thu, 6 Aug 2026 21:44:46 -0400 Subject: [PATCH 2/4] Keep the executable run spec out of reach of event redaction MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two root-cause fixes for the sandbox failure where an inline Dockerfile came back from the store as `ARG REDACTED` and the Daytona snapshot build died on the unset variable. Entropy redaction measures values, not assignment pairs. The detector matched `NAME=value` as one token, so an uppercase name merged its charset into a pure-hex value (which alone can never exceed 4.0 bits) and pushed the pair over the 4.5-bit threshold — then replaced the whole pair, destroying the name. `find_entropy_regions` now strips an identifier-shaped `NAME=` prefix before measuring and redacts only the value, matching the gitleaks layer's `key=REDACTED` shape. Execution no longer reads redacted content. Every stored event passes through the redaction sink, and `load_from_store` rehydrated the worker's RunSpec from the projection folded from those events — so a redactor false positive silently rewrote the spec the sandbox builds from (and changed its snapshot identity). The creation path now writes the exact spec bytes to the content-addressed blob store and records `spec_blob` on run.created; `load_from_store` loads the spec from the blob, keeping the event stream authoritative for run identity, provenance, and event-recorded blob ids. Retry and fork carry the source run's `spec_blob` forward, so derived runs stop inheriting the redacted copy. Runs created before the blob existed fall back to the folded spec. The projection and every API surface keep serving the redacted fold; blobs were already stored unredacted (the workflow bundle carries the same bytes), so this adds no new exposure at rest. Co-Authored-By: Claude Fable 5 --- docs/public/api-reference/fabro-api.yaml | 2 + lib/apps/fabro-cli/src/commands/run/attach.rs | 1 + lib/apps/fabro-cli/tests/it/cmd/attach.rs | 1 + lib/apps/fabro-cli/tests/it/support/mod.rs | 1 + lib/apps/fabro-server/src/run_files.rs | 1 + .../fabro-server/src/server/handler/events.rs | 1 + .../fabro-server/src/server/handler/pair.rs | 1 + .../src/server/handler/sessions.rs | 1 + lib/apps/fabro-server/src/server/tests.rs | 8 ++ .../fabro-server/tests/it/api/run_files.rs | 1 + lib/components/fabro-dump/src/lib.rs | 1 + lib/components/fabro-store/src/run_state.rs | 4 + .../fabro-store/src/run_summary_store.rs | 1 + lib/components/fabro-store/src/slate/mod.rs | 1 + .../tests/serializable_projection.rs | 1 + .../fabro-workflow/src/billing_rollup.rs | 1 + .../fabro-workflow/src/event/convert.rs | 3 + .../fabro-workflow/src/event/events.rs | 2 + .../fabro-workflow/src/event/sink.rs | 1 + lib/components/fabro-workflow/src/git.rs | 1 + .../fabro-workflow/src/handler/agent.rs | 1 + .../fabro-workflow/src/handler/command.rs | 2 + .../fabro-workflow/src/handler/parallel.rs | 1 + .../fabro-workflow/src/handler/prompt.rs | 1 + .../fabro-workflow/src/lifecycle/git.rs | 1 + .../fabro-workflow/src/operations/archive.rs | 1 + .../fabro-workflow/src/operations/create.rs | 8 ++ .../fabro-workflow/src/operations/fork.rs | 4 + .../fabro-workflow/src/operations/retry.rs | 5 + .../fabro-workflow/src/operations/timeline.rs | 1 + .../src/pipeline/execute/tests.rs | 2 + .../fabro-workflow/src/pipeline/finalize.rs | 2 + .../fabro-workflow/src/pipeline/initialize.rs | 2 + .../fabro-workflow/src/pipeline/persist.rs | 109 +++++++++++++++--- .../src/pipeline/pull_request.rs | 9 ++ .../fabro-workflow/src/run_lookup.rs | 2 + .../fabro-workflow/src/run_metadata.rs | 1 + .../fabro-workflow/src/runtime_store.rs | 2 + .../fabro-workflow/src/stage_execution.rs | 1 + .../fabro-workflow/src/test_support.rs | 1 + .../tests/run_projection_round_trip.rs | 1 + lib/foundation/fabro-redact/src/entropy.rs | 33 +++++- lib/foundation/fabro-redact/src/jsonl.rs | 4 +- lib/foundation/fabro-test/src/lib.rs | 4 + lib/foundation/fabro-types/src/run.rs | 5 + .../fabro-types/src/run_event/run.rs | 5 + .../fabro-types/src/run_projection.rs | 3 + .../fabro-types/tests/run_event_serde.rs | 2 + .../fabro-types/tests/run_spec_methods.rs | 1 + .../fabro-types/tests/run_spec_serde.rs | 1 + 50 files changed, 229 insertions(+), 20 deletions(-) diff --git a/docs/public/api-reference/fabro-api.yaml b/docs/public/api-reference/fabro-api.yaml index 261badbbb..1875a9662 100644 --- a/docs/public/api-reference/fabro-api.yaml +++ b/docs/public/api-reference/fabro-api.yaml @@ -11304,6 +11304,8 @@ components: type: ["string", "null"] definition_blob: type: ["string", "null"] + spec_blob: + type: ["string", "null"] git: oneOf: - $ref: "#/components/schemas/GitContext" diff --git a/lib/apps/fabro-cli/src/commands/run/attach.rs b/lib/apps/fabro-cli/src/commands/run/attach.rs index cd356ffa0..7c1cf93b7 100644 --- a/lib/apps/fabro-cli/src/commands/run/attach.rs +++ b/lib/apps/fabro-cli/src/commands/run/attach.rs @@ -849,6 +849,7 @@ mod tests { provenance: test_support::test_run_provenance(), manifest_blob: None, definition_blob: None, + spec_blob: None, git: None, fork_source_ref: None, }; diff --git a/lib/apps/fabro-cli/tests/it/cmd/attach.rs b/lib/apps/fabro-cli/tests/it/cmd/attach.rs index 813bf7424..cc00e14a2 100644 --- a/lib/apps/fabro-cli/tests/it/cmd/attach.rs +++ b/lib/apps/fabro-cli/tests/it/cmd/attach.rs @@ -1024,6 +1024,7 @@ fn attach_json_errors_without_prompting_for_human_input() { } }, "source_directory": "[TEMP_DIR]", + "spec_blob": "[BLOB_ID]", "title": "Wait for approval", "web_url": "http://localhost:3000/runs/[ULID]", "workflow_slug": "human-gate", diff --git a/lib/apps/fabro-cli/tests/it/support/mod.rs b/lib/apps/fabro-cli/tests/it/support/mod.rs index 2f3557044..57e4a98e5 100644 --- a/lib/apps/fabro-cli/tests/it/support/mod.rs +++ b/lib/apps/fabro-cli/tests/it/support/mod.rs @@ -53,6 +53,7 @@ pub(crate) fn run_projection_json(run_id: &str, status: &serde_json::Value) -> s provenance: test_support::test_run_provenance(), manifest_blob: None, definition_blob: None, + spec_blob: None, git: None, fork_source_ref: None, }; diff --git a/lib/apps/fabro-server/src/run_files.rs b/lib/apps/fabro-server/src/run_files.rs index 920460f79..2343933b6 100644 --- a/lib/apps/fabro-server/src/run_files.rs +++ b/lib/apps/fabro-server/src/run_files.rs @@ -2387,6 +2387,7 @@ index 1111111..2222222 160000 provenance: test_support::test_run_provenance(), manifest_blob: None, definition_blob: None, + spec_blob: None, git: None, fork_source_ref: None, }, diff --git a/lib/apps/fabro-server/src/server/handler/events.rs b/lib/apps/fabro-server/src/server/handler/events.rs index 1bfb7f889..5fd655f1d 100644 --- a/lib/apps/fabro-server/src/server/handler/events.rs +++ b/lib/apps/fabro-server/src/server/handler/events.rs @@ -627,6 +627,7 @@ mod stage_events_tests { automation: None, provenance: test_support::test_run_provenance(), manifest_blob: None, + spec_blob: None, git: None, fork_source_ref: None, retried_from: None, diff --git a/lib/apps/fabro-server/src/server/handler/pair.rs b/lib/apps/fabro-server/src/server/handler/pair.rs index ec31d2e54..6e244bcac 100644 --- a/lib/apps/fabro-server/src/server/handler/pair.rs +++ b/lib/apps/fabro-server/src/server/handler/pair.rs @@ -1027,6 +1027,7 @@ mod tests { automation: None, provenance: test_support::test_run_provenance(), manifest_blob: None, + spec_blob: None, git: None, fork_source_ref: None, retried_from: None, diff --git a/lib/apps/fabro-server/src/server/handler/sessions.rs b/lib/apps/fabro-server/src/server/handler/sessions.rs index c2ee94b32..e9bacdfff 100644 --- a/lib/apps/fabro-server/src/server/handler/sessions.rs +++ b/lib/apps/fabro-server/src/server/handler/sessions.rs @@ -1923,6 +1923,7 @@ reasoning = false provenance: test_support::test_run_provenance(), manifest_blob: None, definition_blob: None, + spec_blob: None, git: None, fork_source_ref: None, }; diff --git a/lib/apps/fabro-server/src/server/tests.rs b/lib/apps/fabro-server/src/server/tests.rs index a05443a00..4278eeec5 100644 --- a/lib/apps/fabro-server/src/server/tests.rs +++ b/lib/apps/fabro-server/src/server/tests.rs @@ -4650,6 +4650,7 @@ async fn append_default_run_created(run_store: &fabro_store::RunDatabase, run_id automation: None, provenance: test_support::test_run_provenance(), manifest_blob: None, + spec_blob: None, git: None, fork_source_ref: None, retried_from: None, @@ -4701,6 +4702,7 @@ async fn create_slack_notification_run( automation: None, provenance: test_support::test_run_provenance(), manifest_blob: None, + spec_blob: None, git: None, fork_source_ref: None, retried_from: None, @@ -5774,6 +5776,7 @@ async fn list_run_stages_distinguishes_visits() { automation: None, provenance: test_support::test_run_provenance(), manifest_blob: None, + spec_blob: None, git: None, fork_source_ref: None, retried_from: None, @@ -5910,6 +5913,7 @@ async fn list_run_stages_exposes_execution_identity_for_resumed_stage() { automation: None, provenance: test_support::test_run_provenance(), manifest_blob: None, + spec_blob: None, git: None, fork_source_ref: None, retried_from: None, @@ -7097,6 +7101,7 @@ async fn create_completed_run_ready_for_pull_request( provenance: test_support::test_run_provenance(), manifest_blob: None, definition_blob: None, + spec_blob: None, fork_source_ref: None, }; @@ -7113,6 +7118,7 @@ async fn create_completed_run_ready_for_pull_request( automation: None, provenance: run_spec.provenance.clone(), manifest_blob: None, + spec_blob: None, git, fork_source_ref: None, retried_from: None, @@ -14082,6 +14088,7 @@ async fn create_preserved_local_sandbox_run(state: &Arc, run_id: RunId automation: None, provenance: test_support::test_run_provenance(), manifest_blob: None, + spec_blob: None, git: None, fork_source_ref: None, retried_from: None, @@ -14831,6 +14838,7 @@ async fn delete_run_retry_after_missing_provider_resource_removes_metadata() { automation: None, provenance: test_support::test_run_provenance(), manifest_blob: None, + spec_blob: None, git: None, fork_source_ref: None, retried_from: None, diff --git a/lib/apps/fabro-server/tests/it/api/run_files.rs b/lib/apps/fabro-server/tests/it/api/run_files.rs index 6d286cbab..307c4a574 100644 --- a/lib/apps/fabro-server/tests/it/api/run_files.rs +++ b/lib/apps/fabro-server/tests/it/api/run_files.rs @@ -68,6 +68,7 @@ async fn append_completed_run_with_final_patch( automation: None, provenance: test_support::test_run_provenance(), manifest_blob: None, + spec_blob: None, git: None, fork_source_ref: None, retried_from: None, diff --git a/lib/components/fabro-dump/src/lib.rs b/lib/components/fabro-dump/src/lib.rs index 1b3a10a4c..39c73e1d8 100644 --- a/lib/components/fabro-dump/src/lib.rs +++ b/lib/components/fabro-dump/src/lib.rs @@ -501,6 +501,7 @@ mod tests { provenance: test_support::test_run_provenance(), manifest_blob: None, definition_blob: None, + spec_blob: None, fork_source_ref: None, } } diff --git a/lib/components/fabro-store/src/run_state.rs b/lib/components/fabro-store/src/run_state.rs index 5e4ab5e37..dedcb0ef5 100644 --- a/lib/components/fabro-store/src/run_state.rs +++ b/lib/components/fabro-store/src/run_state.rs @@ -1046,6 +1046,7 @@ fn projection_from_created(event: &EventEnvelope) -> Result { provenance: props.provenance.clone(), manifest_blob: props.manifest_blob, definition_blob: None, + spec_blob: props.spec_blob, git: props.git.clone(), fork_source_ref: props.fork_source_ref.clone(), }; @@ -2292,6 +2293,7 @@ mod tests { provenance: test_support::test_run_provenance(), manifest_blob: None, definition_blob: None, + spec_blob: None, git: None, fork_source_ref: None, } @@ -4098,6 +4100,7 @@ mod tests { provenance: test_support::test_run_provenance(), manifest_blob: None, definition_blob: None, + spec_blob: None, fork_source_ref: None, }; @@ -4124,6 +4127,7 @@ mod tests { provenance: test_support::test_run_provenance(), manifest_blob: None, definition_blob: None, + spec_blob: None, fork_source_ref: None, }; diff --git a/lib/components/fabro-store/src/run_summary_store.rs b/lib/components/fabro-store/src/run_summary_store.rs index 5db1a49b1..dcbec847e 100644 --- a/lib/components/fabro-store/src/run_summary_store.rs +++ b/lib/components/fabro-store/src/run_summary_store.rs @@ -601,6 +601,7 @@ mod tests { provenance: test_support::test_run_provenance(), manifest_blob: None, definition_blob: None, + spec_blob: None, git: None, fork_source_ref: None, }, diff --git a/lib/components/fabro-store/src/slate/mod.rs b/lib/components/fabro-store/src/slate/mod.rs index 5d9ae3bd9..04be6ba1f 100644 --- a/lib/components/fabro-store/src/slate/mod.rs +++ b/lib/components/fabro-store/src/slate/mod.rs @@ -602,6 +602,7 @@ mod tests { provenance: test_support::test_run_provenance(), manifest_blob: None, definition_blob: None, + spec_blob: None, git: Some(fabro_types::GitContext { origin_url: "https://github.com/fabro-sh/fabro".to_string(), branch: "main".to_string(), diff --git a/lib/components/fabro-store/tests/serializable_projection.rs b/lib/components/fabro-store/tests/serializable_projection.rs index ef0ed067b..4e03e1782 100644 --- a/lib/components/fabro-store/tests/serializable_projection.rs +++ b/lib/components/fabro-store/tests/serializable_projection.rs @@ -25,6 +25,7 @@ fn sample_run_spec() -> RunSpec { provenance: test_support::test_run_provenance(), manifest_blob: None, definition_blob: None, + spec_blob: None, git: Some(fabro_types::GitContext { origin_url: "https://github.com/fabro-sh/fabro.git".to_string(), branch: "main".to_string(), diff --git a/lib/components/fabro-workflow/src/billing_rollup.rs b/lib/components/fabro-workflow/src/billing_rollup.rs index 986b541d4..ed26de465 100644 --- a/lib/components/fabro-workflow/src/billing_rollup.rs +++ b/lib/components/fabro-workflow/src/billing_rollup.rs @@ -322,6 +322,7 @@ mod tests { provenance: test_support::test_run_provenance(), manifest_blob: None, definition_blob: None, + spec_blob: None, git: None, fork_source_ref: None, } diff --git a/lib/components/fabro-workflow/src/event/convert.rs b/lib/components/fabro-workflow/src/event/convert.rs index 6403d8933..ef255d5b8 100644 --- a/lib/components/fabro-workflow/src/event/convert.rs +++ b/lib/components/fabro-workflow/src/event/convert.rs @@ -36,6 +36,7 @@ fn event_body_from_event(event: &Event) -> EventBody { automation, provenance, manifest_blob, + spec_blob, git, fork_source_ref, retried_from, @@ -54,6 +55,7 @@ fn event_body_from_event(event: &Event) -> EventBody { automation: automation.clone(), provenance: provenance.clone(), manifest_blob: *manifest_blob, + spec_blob: *spec_blob, git: git.clone(), fork_source_ref: fork_source_ref.clone(), retried_from: *retried_from, @@ -2669,6 +2671,7 @@ mod tests { automation: Some(automation.clone()), provenance, manifest_blob: None, + spec_blob: None, git: None, fork_source_ref: None, retried_from: None, diff --git a/lib/components/fabro-workflow/src/event/events.rs b/lib/components/fabro-workflow/src/event/events.rs index f36d01a16..f465a5cfe 100644 --- a/lib/components/fabro-workflow/src/event/events.rs +++ b/lib/components/fabro-workflow/src/event/events.rs @@ -41,6 +41,8 @@ pub enum Event { #[serde(default, skip_serializing_if = "Option::is_none")] manifest_blob: Option, #[serde(default, skip_serializing_if = "Option::is_none")] + spec_blob: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] git: Option, #[serde(default, skip_serializing_if = "Option::is_none")] fork_source_ref: Option, diff --git a/lib/components/fabro-workflow/src/event/sink.rs b/lib/components/fabro-workflow/src/event/sink.rs index f6abb1724..150c69f72 100644 --- a/lib/components/fabro-workflow/src/event/sink.rs +++ b/lib/components/fabro-workflow/src/event/sink.rs @@ -290,6 +290,7 @@ mod tests { automation: None, provenance: test_support::test_run_provenance(), manifest_blob: None, + spec_blob: None, git: None, fork_source_ref: None, retried_from: None, diff --git a/lib/components/fabro-workflow/src/git.rs b/lib/components/fabro-workflow/src/git.rs index de144b6a1..e7ca4feda 100644 --- a/lib/components/fabro-workflow/src/git.rs +++ b/lib/components/fabro-workflow/src/git.rs @@ -364,6 +364,7 @@ mod tests { automation: None, provenance: test_support::test_run_provenance(), manifest_blob: None, + spec_blob: None, git: None, fork_source_ref: None, retried_from: None, diff --git a/lib/components/fabro-workflow/src/handler/agent.rs b/lib/components/fabro-workflow/src/handler/agent.rs index 48b234a8b..99d20431b 100644 --- a/lib/components/fabro-workflow/src/handler/agent.rs +++ b/lib/components/fabro-workflow/src/handler/agent.rs @@ -501,6 +501,7 @@ mod tests { automation: None, provenance: test_support::test_run_provenance(), manifest_blob: None, + spec_blob: None, git: None, fork_source_ref: None, retried_from: None, diff --git a/lib/components/fabro-workflow/src/handler/command.rs b/lib/components/fabro-workflow/src/handler/command.rs index 5dfc41e46..60f2b9a51 100644 --- a/lib/components/fabro-workflow/src/handler/command.rs +++ b/lib/components/fabro-workflow/src/handler/command.rs @@ -374,6 +374,7 @@ mod tests { provenance: test_support::test_run_provenance(), manifest_blob: None, definition_blob: None, + spec_blob: None, git: None, fork_source_ref: None, }, @@ -471,6 +472,7 @@ mod tests { automation: None, provenance: test_support::test_run_provenance(), manifest_blob: None, + spec_blob: None, git: None, fork_source_ref: None, retried_from: None, diff --git a/lib/components/fabro-workflow/src/handler/parallel.rs b/lib/components/fabro-workflow/src/handler/parallel.rs index 62fc59630..44323be72 100644 --- a/lib/components/fabro-workflow/src/handler/parallel.rs +++ b/lib/components/fabro-workflow/src/handler/parallel.rs @@ -956,6 +956,7 @@ mod tests { automation: None, provenance: test_support::test_run_provenance(), manifest_blob: None, + spec_blob: None, git: None, fork_source_ref: None, retried_from: None, diff --git a/lib/components/fabro-workflow/src/handler/prompt.rs b/lib/components/fabro-workflow/src/handler/prompt.rs index d586ca6b7..630332878 100644 --- a/lib/components/fabro-workflow/src/handler/prompt.rs +++ b/lib/components/fabro-workflow/src/handler/prompt.rs @@ -279,6 +279,7 @@ mod tests { automation: None, provenance: test_support::test_run_provenance(), manifest_blob: None, + spec_blob: None, git: None, fork_source_ref: None, retried_from: None, diff --git a/lib/components/fabro-workflow/src/lifecycle/git.rs b/lib/components/fabro-workflow/src/lifecycle/git.rs index fce72da6e..6fa277b31 100644 --- a/lib/components/fabro-workflow/src/lifecycle/git.rs +++ b/lib/components/fabro-workflow/src/lifecycle/git.rs @@ -750,6 +750,7 @@ mod tests { automation: None, provenance: test_support::test_run_provenance(), manifest_blob: None, + spec_blob: None, git: None, fork_source_ref: None, retried_from: None, diff --git a/lib/components/fabro-workflow/src/operations/archive.rs b/lib/components/fabro-workflow/src/operations/archive.rs index c44ff0006..92d9124cf 100644 --- a/lib/components/fabro-workflow/src/operations/archive.rs +++ b/lib/components/fabro-workflow/src/operations/archive.rs @@ -227,6 +227,7 @@ mod tests { automation: None, provenance: test_support::test_run_provenance(), manifest_blob: None, + spec_blob: None, git: None, fork_source_ref: None, retried_from: None, diff --git a/lib/components/fabro-workflow/src/operations/create.rs b/lib/components/fabro-workflow/src/operations/create.rs index 59f424e2d..3f23acd5b 100644 --- a/lib/components/fabro-workflow/src/operations/create.rs +++ b/lib/components/fabro-workflow/src/operations/create.rs @@ -470,6 +470,7 @@ pub async fn persist_create_run( provenance, manifest_blob: None, definition_blob: None, + spec_blob: None, git, fork_source_ref, }; @@ -528,6 +529,12 @@ async fn persist_created_run( } None => None, }; + // The spec on the run.created event is subject to secret redaction in + // stored copies; the blob keeps the exact bytes execution needs. + let spec_blob = { + let bytes = serde_json::to_vec(record).map_err(|err| Error::engine(err.to_string()))?; + Some(run_store.write_blob(&bytes).await.map_err(store_error)?) + }; let title = explicit_title.unwrap_or_else(|| fabro_types::infer_run_title(record.graph.goal())); let stored = to_run_event_at( @@ -554,6 +561,7 @@ async fn persist_created_run( automation: record.automation.clone(), provenance: record.provenance.clone(), manifest_blob, + spec_blob, git: record.git.clone(), fork_source_ref: record.fork_source_ref.clone(), retried_from: None, diff --git a/lib/components/fabro-workflow/src/operations/fork.rs b/lib/components/fabro-workflow/src/operations/fork.rs index 7e5e65516..3f728211d 100644 --- a/lib/components/fabro-workflow/src/operations/fork.rs +++ b/lib/components/fabro-workflow/src/operations/fork.rs @@ -162,6 +162,9 @@ async fn persist_forked_run( automation: spec.automation.clone(), provenance: spec.provenance.clone(), manifest_blob: spec.manifest_blob, + // Content-addressed, so the forked run reads the source run's + // unredacted spec bytes through the same id. + spec_blob: spec.spec_blob, git: spec.git.clone(), fork_source_ref: spec.fork_source_ref.clone(), retried_from: None, @@ -381,6 +384,7 @@ mod tests { automation: None, provenance: test_support::test_run_provenance(), manifest_blob: None, + spec_blob: None, git: Some(fabro_types::GitContext { origin_url: "https://github.com/example/repo.git".to_string(), branch: "main".to_string(), diff --git a/lib/components/fabro-workflow/src/operations/retry.rs b/lib/components/fabro-workflow/src/operations/retry.rs index 27a9df68a..49b5861f4 100644 --- a/lib/components/fabro-workflow/src/operations/retry.rs +++ b/lib/components/fabro-workflow/src/operations/retry.rs @@ -54,6 +54,7 @@ pub async fn retry_run( provenance: _, manifest_blob, definition_blob, + spec_blob, git, fork_source_ref, } = source.spec; @@ -78,6 +79,9 @@ pub async fn retry_run( automation, provenance: input.provenance.clone(), manifest_blob, + // Blobs are content-addressed, so the retried run reads the source + // run's unredacted spec bytes through the same id. + spec_blob, git, fork_source_ref, retried_from: Some(source_run_id), @@ -185,6 +189,7 @@ mod tests { automation: None, provenance: provenance("source-user"), manifest_blob, + spec_blob: None, git: Some(git_context()), fork_source_ref, retried_from: None, diff --git a/lib/components/fabro-workflow/src/operations/timeline.rs b/lib/components/fabro-workflow/src/operations/timeline.rs index b96d10aa4..83319745e 100644 --- a/lib/components/fabro-workflow/src/operations/timeline.rs +++ b/lib/components/fabro-workflow/src/operations/timeline.rs @@ -252,6 +252,7 @@ mod tests { provenance: test_support::test_run_provenance(), manifest_blob: None, definition_blob: None, + spec_blob: None, git: None, fork_source_ref: None, }, diff --git a/lib/components/fabro-workflow/src/pipeline/execute/tests.rs b/lib/components/fabro-workflow/src/pipeline/execute/tests.rs index 118767be8..e69d19f29 100644 --- a/lib/components/fabro-workflow/src/pipeline/execute/tests.rs +++ b/lib/components/fabro-workflow/src/pipeline/execute/tests.rs @@ -173,6 +173,7 @@ fn persisted_workflow(graph: Graph, source: String, run_dir: &Path, run_id: RunI provenance: test_support::test_run_provenance(), manifest_blob: None, definition_blob: None, + spec_blob: None, fork_source_ref: None, }, ) @@ -218,6 +219,7 @@ async fn seed_created_and_starting( automation: None, provenance: test_support::test_run_provenance(), manifest_blob: None, + spec_blob: None, git: run_options.pre_run_git.clone(), fork_source_ref: run_options.fork_source_ref.clone(), retried_from: None, diff --git a/lib/components/fabro-workflow/src/pipeline/finalize.rs b/lib/components/fabro-workflow/src/pipeline/finalize.rs index 8c497175d..74b1e9579 100644 --- a/lib/components/fabro-workflow/src/pipeline/finalize.rs +++ b/lib/components/fabro-workflow/src/pipeline/finalize.rs @@ -788,6 +788,7 @@ mod tests { automation: None, provenance: test_support::test_run_provenance(), manifest_blob: None, + spec_blob: None, git: None, fork_source_ref: None, retried_from: None, @@ -906,6 +907,7 @@ mod tests { provenance: test_support::test_run_provenance(), manifest_blob: None, definition_blob: None, + spec_blob: None, git: None, fork_source_ref: None, }, diff --git a/lib/components/fabro-workflow/src/pipeline/initialize.rs b/lib/components/fabro-workflow/src/pipeline/initialize.rs index 5a7dd6069..49c32de19 100644 --- a/lib/components/fabro-workflow/src/pipeline/initialize.rs +++ b/lib/components/fabro-workflow/src/pipeline/initialize.rs @@ -870,6 +870,7 @@ mod tests { provenance: test_support::test_run_provenance(), manifest_blob: None, definition_blob: None, + spec_blob: None, fork_source_ref, }, ) @@ -1053,6 +1054,7 @@ mod tests { automation: None, provenance: test_support::test_run_provenance(), manifest_blob: None, + spec_blob: None, git: None, fork_source_ref: run_options.fork_source_ref.clone(), retried_from: None, diff --git a/lib/components/fabro-workflow/src/pipeline/persist.rs b/lib/components/fabro-workflow/src/pipeline/persist.rs index 846f9b853..8c224c700 100644 --- a/lib/components/fabro-workflow/src/pipeline/persist.rs +++ b/lib/components/fabro-workflow/src/pipeline/persist.rs @@ -2,6 +2,7 @@ use std::path::Path; use super::types::{PersistOptions, Persisted, Validated}; use crate::error::Error; +use crate::records::RunSpec; use crate::runtime_store::RunStoreHandle; /// PERSIST phase: create the run directory and return durable metadata for @@ -37,7 +38,7 @@ pub(crate) async fn load_from_store( .state() .await .map_err(|err| Error::engine(err.to_string()))?; - let run_spec = state.spec; + let run_spec = executable_run_spec(run_store, state.spec).await?; let graph = run_spec.graph.clone(); let source = run_spec.graph_source.clone().unwrap_or_default(); @@ -50,6 +51,40 @@ pub(crate) async fn load_from_store( )) } +/// Replace the event-folded spec content with the exact bytes from the spec +/// blob. Stored events pass through secret redaction, so the folded spec is +/// display data; the blob written at creation is what execution must see. +/// Runs created before the blob existed fall back to the folded spec. +async fn executable_run_spec( + run_store: &RunStoreHandle, + folded: RunSpec, +) -> Result { + let Some(blob_id) = folded.spec_blob else { + return Ok(folded); + }; + let bytes = run_store + .read_blob(&blob_id) + .await + .map_err(|err| Error::engine(err.to_string()))? + .ok_or_else(|| { + Error::engine(format!( + "run spec blob is missing from the run store: {blob_id}" + )) + })?; + let mut spec: RunSpec = + serde_json::from_slice(&bytes).map_err(|err| Error::Parse(err.to_string()))?; + // The event stream stays authoritative for run identity, for provenance + // (a retry rewrites it), for blob ids recorded on events after the spec + // blob was written, and for a graph source the blob does not carry. + spec.run_id = folded.run_id; + spec.provenance = folded.provenance; + spec.manifest_blob = folded.manifest_blob; + spec.definition_blob = folded.definition_blob; + spec.spec_blob = folded.spec_blob; + spec.graph_source = spec.graph_source.or(folded.graph_source); + Ok(spec) +} + #[cfg(test)] #[expect(clippy::disallowed_methods, reason = "tests stage pipeline fixtures")] mod tests { @@ -150,30 +185,52 @@ mod tests { provenance: test_support::test_run_provenance(), manifest_blob: None, definition_blob: None, + spec_blob: None, fork_source_ref: None, } } async fn seeded_store(record: &RunSpec, source: Option<&str>) -> RunDatabase { + seeded_store_with(record, source, true).await + } + + async fn seeded_store_with( + record: &RunSpec, + source: Option<&str>, + write_spec_blob: bool, + ) -> RunDatabase { let store = memory_store(); let run_store = store.create_run(&record.run_id).await.unwrap(); + // Mirror the production producer: the unredacted spec rides a blob + // and the redacted event carries its id. + let spec_blob = if write_spec_blob { + Some( + run_store + .write_blob(&serde_json::to_vec(record).unwrap()) + .await + .unwrap(), + ) + } else { + None + }; append_event(&run_store, &record.run_id, &Event::RunCreated { - run_id: record.run_id, - title: None, - settings: serde_json::to_value(&record.settings).unwrap(), - graph: serde_json::to_value(&record.graph).unwrap(), - workflow_source: source.map(ToOwned::to_owned), - labels: record.labels.clone().into_iter().collect(), + run_id: record.run_id, + title: None, + settings: serde_json::to_value(&record.settings).unwrap(), + graph: serde_json::to_value(&record.graph).unwrap(), + workflow_source: source.map(ToOwned::to_owned), + labels: record.labels.clone().into_iter().collect(), source_directory: record.source_directory.clone(), - workflow_slug: record.workflow_slug.clone(), - automation: record.automation.clone(), - provenance: record.provenance.clone(), - manifest_blob: None, - git: record.git.clone(), - fork_source_ref: record.fork_source_ref.clone(), - retried_from: None, - parent_id: None, - web_url: None, + workflow_slug: record.workflow_slug.clone(), + automation: record.automation.clone(), + provenance: record.provenance.clone(), + manifest_blob: None, + spec_blob, + git: record.git.clone(), + fork_source_ref: record.fork_source_ref.clone(), + retried_from: None, + parent_id: None, + web_url: None, }) .await .unwrap(); @@ -316,6 +373,26 @@ mod tests { ); } + #[tokio::test] + async fn load_from_store_falls_back_to_folded_spec_without_spec_blob() { + // Runs created before the spec blob existed carry no spec_blob on + // run.created; the folded spec is their only copy. + let temp = tempfile::tempdir().unwrap(); + let run_dir = temp.path().join("run"); + std::fs::create_dir_all(&run_dir).unwrap(); + let (graph, source) = graph_and_source(); + let mut record = sample_record(different_graph()); + record.graph = graph; + + let run_store = seeded_store_with(&record, Some(&source), false).await; + let loaded = load_from_store(&run_store.clone().into(), &run_dir) + .await + .unwrap(); + + assert_eq!(loaded.run_spec().settings, record.settings); + assert_eq!(loaded.run_spec().spec_blob, None); + } + #[test] fn persist_returns_error_on_io_failure() { let temp = tempfile::tempdir().unwrap(); diff --git a/lib/components/fabro-workflow/src/pipeline/pull_request.rs b/lib/components/fabro-workflow/src/pipeline/pull_request.rs index 1ab76a24b..f3f7e4a78 100644 --- a/lib/components/fabro-workflow/src/pipeline/pull_request.rs +++ b/lib/components/fabro-workflow/src/pipeline/pull_request.rs @@ -830,6 +830,7 @@ mod tests { provenance: test_support::test_run_provenance(), manifest_blob: None, definition_blob: None, + spec_blob: None, git: None, fork_source_ref: None, }, @@ -1112,6 +1113,7 @@ mod tests { provenance: test_support::test_run_provenance(), manifest_blob: None, definition_blob: None, + spec_blob: None, fork_source_ref: None, }; append_event(&run_store, &fixtures::RUN_1, &Event::RunCreated { @@ -1126,6 +1128,7 @@ mod tests { automation: None, provenance: run_spec.provenance.clone(), manifest_blob: None, + spec_blob: None, git: run_spec.git.clone(), fork_source_ref: None, retried_from: None, @@ -1179,6 +1182,7 @@ mod tests { provenance: test_support::test_run_provenance(), manifest_blob: None, definition_blob: None, + spec_blob: None, fork_source_ref: None, }; append_event(&run_store, &fixtures::RUN_1, &Event::RunCreated { @@ -1193,6 +1197,7 @@ mod tests { automation: None, provenance: run_spec.provenance.clone(), manifest_blob: None, + spec_blob: None, git: run_spec.git.clone(), fork_source_ref: None, retried_from: None, @@ -1596,6 +1601,7 @@ mod tests { provenance: test_support::test_run_provenance(), manifest_blob: None, definition_blob: None, + spec_blob: None, fork_source_ref: None, }; append_event(&run_store, &fixtures::RUN_1, &Event::RunCreated { @@ -1610,6 +1616,7 @@ mod tests { automation: None, provenance: test_support::test_run_provenance(), manifest_blob: None, + spec_blob: None, git: None, fork_source_ref: None, retried_from: None, @@ -1813,6 +1820,7 @@ mod tests { provenance: test_support::test_run_provenance(), manifest_blob: None, definition_blob: None, + spec_blob: None, fork_source_ref: None, }; append_event(&run_store, &fixtures::RUN_1, &Event::RunCreated { @@ -1827,6 +1835,7 @@ mod tests { automation: None, provenance: test_support::test_run_provenance(), manifest_blob: None, + spec_blob: None, git: None, fork_source_ref: None, retried_from: None, diff --git a/lib/components/fabro-workflow/src/run_lookup.rs b/lib/components/fabro-workflow/src/run_lookup.rs index 99e9825fe..e70caf216 100644 --- a/lib/components/fabro-workflow/src/run_lookup.rs +++ b/lib/components/fabro-workflow/src/run_lookup.rs @@ -487,6 +487,7 @@ mod tests { provenance: test_support::test_run_provenance(), manifest_blob: None, definition_blob: None, + spec_blob: None, fork_source_ref: None, } } @@ -512,6 +513,7 @@ mod tests { automation: None, provenance: run_spec.provenance.clone(), manifest_blob: None, + spec_blob: None, git: run_spec.git.clone(), fork_source_ref: run_spec.fork_source_ref.clone(), retried_from: None, diff --git a/lib/components/fabro-workflow/src/run_metadata.rs b/lib/components/fabro-workflow/src/run_metadata.rs index 74be989df..754bb5dfa 100644 --- a/lib/components/fabro-workflow/src/run_metadata.rs +++ b/lib/components/fabro-workflow/src/run_metadata.rs @@ -641,6 +641,7 @@ mod tests { provenance: test_support::test_run_provenance(), manifest_blob: None, definition_blob: None, + spec_blob: None, fork_source_ref: None, }, chrono::Utc::now(), diff --git a/lib/components/fabro-workflow/src/runtime_store.rs b/lib/components/fabro-workflow/src/runtime_store.rs index c376c47e7..8f43ef647 100644 --- a/lib/components/fabro-workflow/src/runtime_store.rs +++ b/lib/components/fabro-workflow/src/runtime_store.rs @@ -151,6 +151,7 @@ mod tests { provenance: test_support::test_run_provenance(), manifest_blob: None, definition_blob: None, + spec_blob: None, fork_source_ref: None, } } @@ -169,6 +170,7 @@ mod tests { automation: None, provenance: test_support::test_run_provenance(), manifest_blob: None, + spec_blob: None, git: None, fork_source_ref: None, retried_from: None, diff --git a/lib/components/fabro-workflow/src/stage_execution.rs b/lib/components/fabro-workflow/src/stage_execution.rs index c05a3547e..ec755127f 100644 --- a/lib/components/fabro-workflow/src/stage_execution.rs +++ b/lib/components/fabro-workflow/src/stage_execution.rs @@ -209,6 +209,7 @@ mod tests { provenance: test_support::test_run_provenance(), manifest_blob: None, definition_blob: None, + spec_blob: None, git: None, fork_source_ref: None, }; diff --git a/lib/components/fabro-workflow/src/test_support.rs b/lib/components/fabro-workflow/src/test_support.rs index 19390762d..c7407b7b9 100644 --- a/lib/components/fabro-workflow/src/test_support.rs +++ b/lib/components/fabro-workflow/src/test_support.rs @@ -204,6 +204,7 @@ async fn initialized( }, }, manifest_blob: None, + spec_blob: None, git: run_options.pre_run_git.clone(), fork_source_ref: run_options.fork_source_ref.clone(), retried_from: None, diff --git a/lib/foundation/fabro-api/tests/run_projection_round_trip.rs b/lib/foundation/fabro-api/tests/run_projection_round_trip.rs index 77d28178b..cbe1c6239 100644 --- a/lib/foundation/fabro-api/tests/run_projection_round_trip.rs +++ b/lib/foundation/fabro-api/tests/run_projection_round_trip.rs @@ -140,6 +140,7 @@ fn run_spec_json() -> serde_json::Value { provenance: test_support::test_run_provenance(), manifest_blob: None, definition_blob: None, + spec_blob: None, git: None, fork_source_ref: None, }) diff --git a/lib/foundation/fabro-redact/src/entropy.rs b/lib/foundation/fabro-redact/src/entropy.rs index dc744cf39..884aa3713 100644 --- a/lib/foundation/fabro-redact/src/entropy.rs +++ b/lib/foundation/fabro-redact/src/entropy.rs @@ -36,6 +36,12 @@ pub(super) fn shannon_entropy(s: &str) -> f64 { /// Returns regions where tokens match `[A-Za-z0-9+_=-]{10,}` and have /// Shannon entropy above the threshold (4.5 bits). Protects against /// consuming characters from JSON escape sequences. +/// +/// An assignment token (`NAME=value`) is measured and redacted by its +/// value alone. Measuring the pair merges the name's charset into the +/// value's and pushes innocuous values (a pure-hex git SHA can never +/// exceed 4.0 bits by itself) over the threshold, and redacting the +/// pair destroys the name that says what was redacted. pub(super) fn find_entropy_regions(s: &str) -> Vec { let mut regions = Vec::new(); for m in SECRET_PATTERN.find_iter(s) { @@ -58,6 +64,10 @@ pub(super) fn find_entropy_regions(s: &str) -> Vec { } } + if let Some(offset) = assignment_value_offset(&s[start..end]) { + start += offset; + } + if shannon_entropy(&s[start..end]) > ENTROPY_THRESHOLD { regions.push(Region { start, end }); } @@ -65,6 +75,24 @@ pub(super) fn find_entropy_regions(s: &str) -> Vec { regions } +/// For an assignment token (`NAME=value` with an identifier-shaped name), +/// return the byte offset where the value begins. Entropy above the 4.5-bit +/// threshold needs at least 23 distinct characters, so a value too short to +/// qualify simply measures under the threshold; no length guard is needed. +fn assignment_value_offset(token: &str) -> Option { + let eq = token.find('=')?; + let name = &token[..eq]; + let mut chars = name.chars(); + let first = chars.next()?; + if !(first.is_ascii_alphabetic() || first == '_') { + return None; + } + if !chars.all(|c| c.is_ascii_alphanumeric() || c == '_') { + return None; + } + Some(eq + 1) +} + #[cfg(test)] mod tests { use super::*; @@ -98,11 +126,12 @@ mod tests { #[test] fn regions_finds_high_entropy_token() { - // `=` is in the regex pattern, so "key=xK9..." matches as one token + // "key=xK9..." matches as one token, but only the value is + // measured and flagged; the name survives redaction. let input = "key=xK9mZ2vL8nQ5rT1wY4bC7dF0gH3jE6p"; let regions = find_entropy_regions(input); assert_eq!(regions.len(), 1); - assert_eq!(regions[0].start, 0); + assert_eq!(regions[0].start, "key=".len()); assert_eq!(regions[0].end, input.len()); } diff --git a/lib/foundation/fabro-redact/src/jsonl.rs b/lib/foundation/fabro-redact/src/jsonl.rs index e466f7002..f0f50f6fa 100644 --- a/lib/foundation/fabro-redact/src/jsonl.rs +++ b/lib/foundation/fabro-redact/src/jsonl.rs @@ -209,7 +209,7 @@ mod tests { let redacted = redact_json_value(input); assert_eq!(redacted["name"], "fabro-01KQR3V9D4VPFFWMNTVH09J48G"); - assert_eq!(redacted["content"], "REDACTED"); + assert_eq!(redacted["content"], "token=REDACTED"); } #[test] @@ -262,7 +262,7 @@ mod tests { let redacted = redact_json_value(input); - assert_eq!(redacted["content"], "REDACTED"); + assert_eq!(redacted["content"], "key=REDACTED"); assert_eq!(redacted["session_id"], HIGH_ENTROPY_SECRET); } diff --git a/lib/foundation/fabro-test/src/lib.rs b/lib/foundation/fabro-test/src/lib.rs index 74a5f6727..eec273d32 100644 --- a/lib/foundation/fabro-test/src/lib.rs +++ b/lib/foundation/fabro-test/src/lib.rs @@ -1963,6 +1963,10 @@ pub fn json_snapshot_filters(mut filters: Vec<(String, String)>) -> Vec<(String, r#""definition_blob":\s*"[0-9a-f]{64}""#.to_string(), r#""definition_blob": "[BLOB_ID]""#.to_string(), )); + filters.push(( + r#""spec_blob":\s*"[0-9a-f]{64}""#.to_string(), + r#""spec_blob": "[BLOB_ID]""#.to_string(), + )); filters.push(( r#""run_dir":\s*"\[STORAGE_DIR\]/scratch/\d{8}-\[ULID\]""#.to_string(), r#""run_dir": "[RUN_DIR]""#.to_string(), diff --git a/lib/foundation/fabro-types/src/run.rs b/lib/foundation/fabro-types/src/run.rs index cf0fbe1ff..0c79bb23d 100644 --- a/lib/foundation/fabro-types/src/run.rs +++ b/lib/foundation/fabro-types/src/run.rs @@ -76,6 +76,11 @@ pub struct RunSpec { pub manifest_blob: Option, #[serde(default, skip_serializing_if = "Option::is_none")] pub definition_blob: Option, + /// Unredacted copy of this spec in the blob store. Stored events pass + /// through secret redaction, so the spec folded from them is display + /// data; execution must load the spec from this blob. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub spec_blob: Option, #[serde(default, skip_serializing_if = "Option::is_none")] pub git: Option, #[serde(default, skip_serializing_if = "Option::is_none")] diff --git a/lib/foundation/fabro-types/src/run_event/run.rs b/lib/foundation/fabro-types/src/run_event/run.rs index d070d8aef..b8c7fa34a 100644 --- a/lib/foundation/fabro-types/src/run_event/run.rs +++ b/lib/foundation/fabro-types/src/run_event/run.rs @@ -28,6 +28,11 @@ pub struct RunCreatedProps { pub provenance: RunProvenance, #[serde(default, skip_serializing_if = "Option::is_none")] pub manifest_blob: Option, + /// Unredacted copy of the run spec in the blob store. The settings and + /// graph on this event are redacted at the sink; execution loads the + /// spec from this blob instead. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub spec_blob: Option, #[serde(default, skip_serializing_if = "Option::is_none")] pub git: Option, #[serde(default, skip_serializing_if = "Option::is_none")] diff --git a/lib/foundation/fabro-types/src/run_projection.rs b/lib/foundation/fabro-types/src/run_projection.rs index 3a6be70d4..3616aa33c 100644 --- a/lib/foundation/fabro-types/src/run_projection.rs +++ b/lib/foundation/fabro-types/src/run_projection.rs @@ -1087,6 +1087,7 @@ mod title_tests { provenance: test_support::test_run_provenance(), manifest_blob: None, definition_blob: None, + spec_blob: None, git: None, fork_source_ref: None, }; @@ -1161,6 +1162,7 @@ mod iter_stages_tests { provenance: test_support::test_run_provenance(), manifest_blob: None, definition_blob: None, + spec_blob: None, git: None, fork_source_ref: None, }, @@ -1365,6 +1367,7 @@ mod live_timing_tests { provenance: test_support::test_run_provenance(), manifest_blob: None, definition_blob: None, + spec_blob: None, git: None, fork_source_ref: None, }, diff --git a/lib/foundation/fabro-types/tests/run_event_serde.rs b/lib/foundation/fabro-types/tests/run_event_serde.rs index 633abab04..f287a6acc 100644 --- a/lib/foundation/fabro-types/tests/run_event_serde.rs +++ b/lib/foundation/fabro-types/tests/run_event_serde.rs @@ -32,6 +32,7 @@ fn run_created_props_round_trip_templated_settings() { }), provenance: test_run_provenance(), manifest_blob: None, + spec_blob: None, git: Some(GitContext { origin_url: "https://github.com/fabro-sh/fabro.git".to_string(), branch: "main".to_string(), @@ -93,6 +94,7 @@ fn run_created_props_omits_web_url_when_absent() { automation: None, provenance: test_run_provenance(), manifest_blob: None, + spec_blob: None, git: None, fork_source_ref: None, retried_from: None, diff --git a/lib/foundation/fabro-types/tests/run_spec_methods.rs b/lib/foundation/fabro-types/tests/run_spec_methods.rs index f6f76fecf..50dff20b0 100644 --- a/lib/foundation/fabro-types/tests/run_spec_methods.rs +++ b/lib/foundation/fabro-types/tests/run_spec_methods.rs @@ -31,6 +31,7 @@ fn sample_run_spec() -> RunSpec { provenance: test_run_provenance(), manifest_blob: None, definition_blob: None, + spec_blob: None, git: Some(GitContext { origin_url: "https://github.com/fabro-sh/fabro.git".to_string(), branch: "main".to_string(), diff --git a/lib/foundation/fabro-types/tests/run_spec_serde.rs b/lib/foundation/fabro-types/tests/run_spec_serde.rs index 97529bc93..ec7a7ef40 100644 --- a/lib/foundation/fabro-types/tests/run_spec_serde.rs +++ b/lib/foundation/fabro-types/tests/run_spec_serde.rs @@ -31,6 +31,7 @@ fn run_spec_round_trips_templated_settings() { provenance: test_run_provenance(), manifest_blob: None, definition_blob: None, + spec_blob: None, git: Some(GitContext { origin_url: "https://github.com/fabro-sh/fabro.git".to_string(), branch: "main".to_string(), From 2b095612c8eb315d35f80ef4ac6eb728b22ddd3a Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Thu, 20 Aug 2026 20:35:52 -0400 Subject: [PATCH 3/4] Address run spec persistence review findings --- .../fabro-workflow/src/operations/create.rs | 54 +++++++++-------- .../fabro-workflow/src/pipeline/persist.rs | 58 +++++++++++++------ lib/foundation/fabro-redact/src/entropy.rs | 17 ++++++ lib/foundation/fabro-test/src/lib.rs | 18 ++---- 4 files changed, 95 insertions(+), 52 deletions(-) diff --git a/lib/components/fabro-workflow/src/operations/create.rs b/lib/components/fabro-workflow/src/operations/create.rs index 3f23acd5b..a872f0186 100644 --- a/lib/components/fabro-workflow/src/operations/create.rs +++ b/lib/components/fabro-workflow/src/operations/create.rs @@ -13,10 +13,11 @@ use std::sync::Arc; use fabro_config::Storage; use fabro_graphviz::graph::{AttrValue, Graph}; use fabro_model::{Catalog, ProviderId}; -use fabro_store::Database; +use fabro_store::{Database, RunDatabase}; use fabro_template::TemplateContext; use fabro_types::{ - AutomationRef, ForkSourceRef, GitContext, ManifestPath, RunId, RunProvenance, WorkflowSettings, + AutomationRef, ForkSourceRef, GitContext, ManifestPath, RunBlobId, RunId, RunProvenance, + WorkflowSettings, }; use fabro_util::json::normalize_json_value; use tokio::task::spawn_blocking; @@ -517,24 +518,17 @@ async fn persist_created_run( .create_run(&record.run_id) .await .map_err(|err| Error::engine_with_source("failed to create run store", err))?; - let manifest_blob = match submitted_manifest_bytes { - Some(bytes) => Some(run_store.write_blob(bytes).await.map_err(store_error)?), - None => None, - }; - let definition_blob = match accepted_definition { - Some(definition) => { - let bytes = - serde_json::to_vec(definition).map_err(|err| Error::engine(err.to_string()))?; - Some(run_store.write_blob(&bytes).await.map_err(store_error)?) - } - None => None, - }; - // The spec on the run.created event is subject to secret redaction in - // stored copies; the blob keeps the exact bytes execution needs. - let spec_blob = { - let bytes = serde_json::to_vec(record).map_err(|err| Error::engine(err.to_string()))?; - Some(run_store.write_blob(&bytes).await.map_err(store_error)?) - }; + let definition_bytes = accepted_definition + .map(serde_json::to_vec) + .transpose() + .map_err(|err| Error::engine_with_source("failed to serialize run definition", err))?; + let spec_bytes = serde_json::to_vec(record) + .map_err(|err| Error::engine_with_source("failed to serialize run spec", err))?; + let (manifest_blob, definition_blob, spec_blob) = tokio::try_join!( + write_optional_blob(&run_store, submitted_manifest_bytes), + write_optional_blob(&run_store, definition_bytes.as_deref()), + async { run_store.write_blob(&spec_bytes).await.map_err(store_error) }, + )?; let title = explicit_title.unwrap_or_else(|| fabro_types::infer_run_title(record.graph.goal())); let stored = to_run_event_at( @@ -561,7 +555,7 @@ async fn persist_created_run( automation: record.automation.clone(), provenance: record.provenance.clone(), manifest_blob, - spec_blob, + spec_blob: Some(spec_blob), git: record.git.clone(), fork_source_ref: record.fork_source_ref.clone(), retried_from: None, @@ -588,8 +582,22 @@ async fn persist_created_run( .map_err(store_error) } -fn store_error(err: impl std::fmt::Display) -> Error { - Error::engine(err.to_string()) +async fn write_optional_blob( + run_store: &RunDatabase, + bytes: Option<&[u8]>, +) -> Result, Error> { + match bytes { + Some(bytes) => run_store + .write_blob(bytes) + .await + .map(Some) + .map_err(store_error), + None => Ok(None), + } +} + +fn store_error(err: impl Into) -> Error { + Error::engine_with_source("run store operation failed", err) } /// Parse, transform, and validate `dot_source`. diff --git a/lib/components/fabro-workflow/src/pipeline/persist.rs b/lib/components/fabro-workflow/src/pipeline/persist.rs index 8c224c700..303241ff5 100644 --- a/lib/components/fabro-workflow/src/pipeline/persist.rs +++ b/lib/components/fabro-workflow/src/pipeline/persist.rs @@ -65,22 +65,23 @@ async fn executable_run_spec( let bytes = run_store .read_blob(&blob_id) .await - .map_err(|err| Error::engine(err.to_string()))? + .map_err(|err| Error::engine_with_anyhow("failed to read run spec blob", err))? .ok_or_else(|| { Error::engine(format!( "run spec blob is missing from the run store: {blob_id}" )) })?; - let mut spec: RunSpec = - serde_json::from_slice(&bytes).map_err(|err| Error::Parse(err.to_string()))?; - // The event stream stays authoritative for run identity, for provenance - // (a retry rewrites it), for blob ids recorded on events after the spec - // blob was written, and for a graph source the blob does not carry. + let mut spec: RunSpec = serde_json::from_slice(&bytes) + .map_err(|err| Error::engine_with_source("run spec blob was not valid JSON", err))?; + // The event stream stays authoritative for run identity, provenance, and + // blob ids. Prefer the unredacted graph source from the blob, with the + // folded source as a compatibility fallback. spec.run_id = folded.run_id; spec.provenance = folded.provenance; spec.manifest_blob = folded.manifest_blob; spec.definition_blob = folded.definition_blob; spec.spec_blob = folded.spec_blob; + spec.fork_source_ref = folded.fork_source_ref; spec.graph_source = spec.graph_source.or(folded.graph_source); Ok(spec) } @@ -191,27 +192,24 @@ mod tests { } async fn seeded_store(record: &RunSpec, source: Option<&str>) -> RunDatabase { - seeded_store_with(record, source, true).await + seeded_store_with(record, source, Some(record)).await } async fn seeded_store_with( record: &RunSpec, source: Option<&str>, - write_spec_blob: bool, + blob_record: Option<&RunSpec>, ) -> RunDatabase { let store = memory_store(); let run_store = store.create_run(&record.run_id).await.unwrap(); - // Mirror the production producer: the unredacted spec rides a blob - // and the redacted event carries its id. - let spec_blob = if write_spec_blob { - Some( + let spec_blob = match blob_record { + Some(blob_record) => Some( run_store - .write_blob(&serde_json::to_vec(record).unwrap()) + .write_blob(&serde_json::to_vec(blob_record).unwrap()) .await .unwrap(), - ) - } else { - None + ), + None => None, }; append_event(&run_store, &record.run_id, &Event::RunCreated { run_id: record.run_id, @@ -384,7 +382,7 @@ mod tests { let mut record = sample_record(different_graph()); record.graph = graph; - let run_store = seeded_store_with(&record, Some(&source), false).await; + let run_store = seeded_store_with(&record, Some(&source), None).await; let loaded = load_from_store(&run_store.clone().into(), &run_dir) .await .unwrap(); @@ -393,6 +391,32 @@ mod tests { assert_eq!(loaded.run_spec().spec_blob, None); } + #[tokio::test] + async fn load_from_store_uses_fork_reference_from_event_fold() { + let temp = tempfile::tempdir().unwrap(); + let run_dir = temp.path().join("run"); + std::fs::create_dir_all(&run_dir).unwrap(); + let (graph, source) = graph_and_source(); + let source_record = sample_record(graph.clone()); + let mut fork_record = source_record.clone(); + fork_record.run_id = fixtures::RUN_7; + fork_record.fork_source_ref = Some(fabro_types::ForkSourceRef { + source_run_id: source_record.run_id, + checkpoint_sha: "checkpoint-sha".to_string(), + }); + + let run_store = seeded_store_with(&fork_record, Some(&source), Some(&source_record)).await; + let loaded = load_from_store(&run_store.clone().into(), &run_dir) + .await + .unwrap(); + + assert_eq!(loaded.run_spec().run_id, fork_record.run_id); + assert_eq!( + loaded.run_spec().fork_source_ref, + fork_record.fork_source_ref + ); + } + #[test] fn persist_returns_error_on_io_failure() { let temp = tempfile::tempdir().unwrap(); diff --git a/lib/foundation/fabro-redact/src/entropy.rs b/lib/foundation/fabro-redact/src/entropy.rs index 884aa3713..ec6029c3c 100644 --- a/lib/foundation/fabro-redact/src/entropy.rs +++ b/lib/foundation/fabro-redact/src/entropy.rs @@ -81,6 +81,10 @@ pub(super) fn find_entropy_regions(s: &str) -> Vec { /// qualify simply measures under the threshold; no length guard is needed. fn assignment_value_offset(token: &str) -> Option { let eq = token.find('=')?; + let value = &token[eq + 1..]; + if value.is_empty() || value.starts_with('=') { + return None; + } let name = &token[..eq]; let mut chars = name.chars(); let first = chars.next()?; @@ -135,6 +139,19 @@ mod tests { assert_eq!(regions[0].end, input.len()); } + #[test] + fn regions_find_padded_base64_tokens() { + for input in [ + "WxFhjC5EAnh30M0JIe0Wa58Xb1BYf8kedTTdKUbbd9Y=", + "AbCdEfGhIjKlMnOpQrStUvWxYz0123456789ABCDEF==", + ] { + assert_eq!(find_entropy_regions(input), vec![Region { + start: 0, + end: input.len(), + }]); + } + } + #[test] fn regions_empty_for_json_escape_sequence() { // "controller.go\nmodel.go" — the regex could match across the \n boundary diff --git a/lib/foundation/fabro-test/src/lib.rs b/lib/foundation/fabro-test/src/lib.rs index eec273d32..5c53f9ba2 100644 --- a/lib/foundation/fabro-test/src/lib.rs +++ b/lib/foundation/fabro-test/src/lib.rs @@ -1955,18 +1955,12 @@ pub fn json_snapshot_filters(mut filters: Vec<(String, String)>) -> Vec<(String, r#""id": "[EVENT_ID]""#.to_string(), )); filters = json_elapsed_ms_snapshot_filters(filters); - filters.push(( - r#""manifest_blob":\s*"[0-9a-f]{64}""#.to_string(), - r#""manifest_blob": "[BLOB_ID]""#.to_string(), - )); - filters.push(( - r#""definition_blob":\s*"[0-9a-f]{64}""#.to_string(), - r#""definition_blob": "[BLOB_ID]""#.to_string(), - )); - filters.push(( - r#""spec_blob":\s*"[0-9a-f]{64}""#.to_string(), - r#""spec_blob": "[BLOB_ID]""#.to_string(), - )); + for field in ["manifest_blob", "definition_blob", "spec_blob"] { + filters.push(( + format!(r#""{field}":\s*"[0-9a-f]{{64}}""#), + format!(r#""{field}": "[BLOB_ID]""#), + )); + } filters.push(( r#""run_dir":\s*"\[STORAGE_DIR\]/scratch/\d{8}-\[ULID\]""#.to_string(), r#""run_dir": "[RUN_DIR]""#.to_string(), From a64b65b88c1b6d33e65116cc28d428cc7e1c1951 Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Thu, 20 Aug 2026 20:50:41 -0400 Subject: [PATCH 4/4] Update blob hash CLI snapshot --- lib/apps/fabro-cli/tests/it/cmd/attach.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/apps/fabro-cli/tests/it/cmd/attach.rs b/lib/apps/fabro-cli/tests/it/cmd/attach.rs index b2b3904ee..9f388c14b 100644 --- a/lib/apps/fabro-cli/tests/it/cmd/attach.rs +++ b/lib/apps/fabro-cli/tests/it/cmd/attach.rs @@ -1012,7 +1012,7 @@ fn attach_json_errors_without_prompting_for_human_input() { } }, "source_directory": "[TEMP_DIR]", - "spec_blob": "[BLOB_ID]", + "spec_blob": "[BLOB_HASH]", "title": "Wait for approval", "web_url": "http://localhost:3000/runs/[ULID]", "workflow_slug": "human-gate",