From 145baac393ef71a9f6337c76a5418ac56c61b5e0 Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Sat, 5 Sep 2026 12:37:58 +0000 Subject: [PATCH] Permit safe cross-run output declarations --- docs/internal/artifacts.md | 6 ++ docs/operations.md | 6 ++ docs/roadmap/implementation.md | 2 +- internal/app/operator_artifact_rendering.go | 51 +++++++----- internal/app/operator_artifacts_list.go | 7 +- internal/app/operator_helpers_test.go | 85 +++++++++++++++----- internal/app/operator_status.go | 8 +- internal/app/restore_commit_test.go | 60 ++++++++++++++ internal/app/restore_plan.go | 16 ++-- internal/app/restore_plan_test.go | 2 +- internal/artifacts/remote_commit.go | 16 ++++ internal/artifacts/remote_commit_test.go | 70 ++++++++++++++++ internal/artifacts/remote_lock.go | 17 +--- internal/artifacts/remote_object_key.go | 60 ++++++++++++++ internal/previouscache/previouscache.go | 13 +-- internal/previouscache/previouscache_test.go | 25 +++++- 16 files changed, 363 insertions(+), 81 deletions(-) create mode 100644 internal/artifacts/remote_object_key.go diff --git a/docs/internal/artifacts.md b/docs/internal/artifacts.md index d424f01..923ac3c 100644 --- a/docs/internal/artifacts.md +++ b/docs/internal/artifacts.md @@ -249,6 +249,12 @@ New publication creates run-scoped immutable objects, including mutable selector is `current/commit-pointer.json`; readers verify its selected commit and declared object generations/checksums. +A `published_output` declaration may identify its commit's run or an earlier +immutable run in the same campaign/session namespace. The key parser validates +that namespace, rejects mutable and malformed keys, and derives the originating +run from the exact declaration. All other commit artifacts remain confined to +the commit's own run prefix. + Campaign, session, and Narratio run IDs are validated as portable opaque segments at configuration and artifact boundaries before they can be used in a workspace or S3 namespace. Previous-artifact destinations remain typed, diff --git a/docs/operations.md b/docs/operations.md index cf6ae61..29d3c98 100644 --- a/docs/operations.md +++ b/docs/operations.md @@ -387,6 +387,12 @@ commit by version, checksum, size, and storage generation; committed artifacts are also checksum- and generation-bound. Readers accept this contract now and strictly reject mismatched or unknown data. +A selected commit may carry a `published_output` declaration from an earlier +run in the same campaign/session. Restore, previous-session caching, status, +and artifact inspection use that declaration's exact immutable key and stable +source; they do not reconstruct a current-run key or scan the session prefix. +Other artifact types remain confined to the selected commit's own run. + When the pointer is absent, remote state is not published; republish the session to establish a committed snapshot. Remote commit state does not carry local `current_pointer_written` metadata. diff --git a/docs/roadmap/implementation.md b/docs/roadmap/implementation.md index f303cce..e1b0695 100644 --- a/docs/roadmap/implementation.md +++ b/docs/roadmap/implementation.md @@ -140,7 +140,7 @@ and removal. Run `go test ./internal/artifacts ./internal/adapters/storage` and ## Stage 3 — Permit Safe Cross-Run Declarations -**Status:** Pending +**Status:** Completed ### Goal diff --git a/internal/app/operator_artifact_rendering.go b/internal/app/operator_artifact_rendering.go index 47a4bbd..62d68c7 100644 --- a/internal/app/operator_artifact_rendering.go +++ b/internal/app/operator_artifact_rendering.go @@ -127,9 +127,8 @@ func writePublishedOutputLine(out io.Writer, rule config.PublishOutputRule, cata fmt.Fprintf(out, "- %s\n", strings.Join(parts, " ")) } -func remotePublishedOutputAvailability(ctx context.Context, cfg *config.Config, store storage.ObjectStore, catalog *artifacts.ArtifactCatalog) map[string]string { +func unresolvedRemotePublishedOutputAvailability(cfg *config.Config, catalog *artifacts.ArtifactCatalog, state string) map[string]string { out := map[string]string{} - sessionPrefix := artifacts.S3SessionPrefix(cfg.Pipeline.Storage.S3.RootPrefix, cfg.Session.Campaign, cfg.Session.SessionID) for _, rule := range cfg.Pipeline.Publish.Outputs { source := strings.TrimSpace(rule.Source) dest, _, err := helperPublishedOutputDest(rule, catalog) @@ -137,14 +136,7 @@ func remotePublishedOutputAvailability(ctx context.Context, cfg *config.Config, out[publishedOutputRemoteStateKey(source, "")] = "remote=error" continue } - key := artifacts.S3PublishedOutputKey(sessionPrefix, dest) - if exists, err := store.Exists(ctx, key); err == nil && exists { - out[publishedOutputRemoteStateKey(source, dest)] = "remote=published" - } else if err != nil { - out[publishedOutputRemoteStateKey(source, dest)] = "remote=error" - } else { - out[publishedOutputRemoteStateKey(source, dest)] = "remote=missing" - } + out[publishedOutputRemoteStateKey(source, dest)] = state } return out } @@ -157,10 +149,9 @@ func remotePublishedOutputAvailabilityForCurrent( current *RemoteCurrentState, ) map[string]string { if current == nil || current.Commit == nil { - return remotePublishedOutputAvailability(ctx, cfg, store, catalog) + return unresolvedRemotePublishedOutputAvailability(cfg, catalog, "remote=missing") } out := map[string]string{} - runPrefix := artifacts.S3RunPrefix(current.SessionPrefix, current.RunID) for _, rule := range cfg.Pipeline.Publish.Outputs { source := strings.TrimSpace(rule.Source) dest, _, err := helperPublishedOutputDest(rule, catalog) @@ -168,26 +159,44 @@ func remotePublishedOutputAvailabilityForCurrent( out[publishedOutputRemoteStateKey(source, "")] = "remote=error" continue } - key := artifacts.S3RunRelativeDestinationKey(runPrefix, dest) - if committedPublishedOutput(current.Commit, key) { - out[publishedOutputRemoteStateKey(source, dest)] = "remote=published" - } else { + declaration, ok := committedPublishedOutput(current.Commit, source) + if !ok { out[publishedOutputRemoteStateKey(source, dest)] = "remote=missing" + continue } + identity, err := artifacts.ParseImmutableRunObjectKey(current.SessionPrefix, declaration.DestinationKey) + if err != nil { + out[publishedOutputRemoteStateKey(source, dest)] = "remote=error" + continue + } + exists, err := store.Exists(ctx, declaration.DestinationKey) + if err != nil { + out[publishedOutputRemoteStateKey(source, dest)] = "remote=error" + continue + } + if !exists { + out[publishedOutputRemoteStateKey(source, dest)] = "remote=missing" + continue + } + state := "remote=published" + if identity.RunID != current.RunID { + state += " origin_run=" + identity.RunID + } + out[publishedOutputRemoteStateKey(source, dest)] = state } return out } -func committedPublishedOutput(commit *artifacts.RemoteCommitManifest, key string) bool { +func committedPublishedOutput(commit *artifacts.RemoteCommitManifest, source string) (artifacts.RemoteArtifact, bool) { if commit == nil { - return false + return artifacts.RemoteArtifact{}, false } for _, artifact := range commit.Artifacts { - if artifact.Type == artifacts.RemoteArtifactTypePublishedOutput && artifact.DestinationKey == key { - return true + if artifact.Type == artifacts.RemoteArtifactTypePublishedOutput && artifact.Source == source { + return artifact, true } } - return false + return artifacts.RemoteArtifact{}, false } func helperPublishedOutputDest(rule config.PublishOutputRule, catalog *artifacts.ArtifactCatalog) (string, bool, error) { diff --git a/internal/app/operator_artifacts_list.go b/internal/app/operator_artifacts_list.go index 5ebd502..2a47bdc 100644 --- a/internal/app/operator_artifacts_list.go +++ b/internal/app/operator_artifacts_list.go @@ -33,7 +33,12 @@ func ArtifactsList(ctx context.Context, args []string, out io.Writer) error { } publishedRemoteState := map[string]string{} if remote && store != nil { - publishedRemoteState = remotePublishedOutputAvailability(ctx, cfg, store, catalog) + current := inspectRemoteCurrentState(ctx, cfg, store) + if current.Err != nil { + publishedRemoteState = unresolvedRemotePublishedOutputAvailability(cfg, catalog, "remote=error") + } else { + publishedRemoteState = remotePublishedOutputAvailabilityForCurrent(ctx, cfg, store, catalog, current.State) + } } writeArtifactList(out, cfg, catalog, locks, publishedRemoteState) return nil diff --git a/internal/app/operator_helpers_test.go b/internal/app/operator_helpers_test.go index 3f6e3da..3cf97de 100644 --- a/internal/app/operator_helpers_test.go +++ b/internal/app/operator_helpers_test.go @@ -7,6 +7,7 @@ import ( "fmt" "os" "path/filepath" + "sort" "strings" "sync" "testing" @@ -914,6 +915,55 @@ func addSessionTemplateToCampaign(t *testing.T, campaignPath, templateFile strin } } +func seedOperatorCommittedOutputs(t *testing.T, fake *storage.FakeBackend, outputs map[string]string) { + t.Helper() + const runID = "20260519T010203Z-a1b2c3d4" + sessionPrefix := artifacts.S3SessionPrefix("dnd", "sample-campaign", "2026-05-03") + manifestKey := artifacts.S3RunSessionManifestKey(sessionPrefix, runID) + fake.SeedObject(storage.FakeObject{Key: manifestKey, Data: restoreManifestJSON(t, "2026-05-03", "sample-campaign")}) + declarations := []artifacts.RemoteArtifact{ + remoteRestoreArtifact(fake, artifacts.RemoteArtifactTypeSessionManifest, "session.manifest", manifestKey), + } + sources := make([]string, 0, len(outputs)) + for source := range outputs { + sources = append(sources, source) + } + sort.Strings(sources) + for _, source := range sources { + key := artifacts.S3RunRelativeDestinationKey(artifacts.S3RunPrefix(sessionPrefix, runID), outputs[source]) + fake.SeedObject(storage.FakeObject{Key: key, Data: []byte(`{"available":true}`)}) + declarations = append(declarations, remoteRestoreArtifact(fake, artifacts.RemoteArtifactTypePublishedOutput, source, key)) + } + commit := artifacts.RemoteCommitManifest{ + FormatVersion: artifacts.RemoteCommitFormatVersion, + Campaign: "sample-campaign", + SessionID: "2026-05-03", + RunID: runID, + Artifacts: declarations, + } + commitData, err := artifacts.EncodeRemoteCommitManifest(commit) + if err != nil { + t.Fatalf("encode operator remote commit: %v", err) + } + commitKey := artifacts.S3RunCommitKey(sessionPrefix, runID) + fake.SeedObject(storage.FakeObject{Key: commitKey, Data: commitData}) + commitObject := fake.Objects[commitKey] + pointerData, err := artifacts.EncodeCurrentCommitPointer(artifacts.CurrentCommitPointer{ + FormatVersion: artifacts.RemoteCommitFormatVersion, + Campaign: "sample-campaign", + SessionID: "2026-05-03", + RunID: runID, + CommitKey: commitKey, + CommitSHA256: restoreCommitSHA256(commitData), + CommitSize: int64(len(commitData)), + CommitGeneration: commitObject.ETag, + }) + if err != nil { + t.Fatalf("encode operator current pointer: %v", err) + } + fake.SeedObject(storage.FakeObject{Key: artifacts.S3CurrentCommitPointerKey(sessionPrefix), Data: pointerData}) +} + func TestExecuteArtifactsListRemoteReportsPublishedAvailability(t *testing.T) { workspaceRoot := t.TempDir() pipelinePath, campaignPath, sessionPath := writeValidConfigFiles(t, workspaceRoot) @@ -924,11 +974,9 @@ func TestExecuteArtifactsListRemoteReportsPublishedAvailability(t *testing.T) { required: true `) fake := &storage.FakeBackend{} - trimmedKey := artifacts.S3PublishedOutputKey( - artifacts.S3SessionPrefix("dnd", "sample-campaign", "2026-05-03"), - "transcripts/final.trimmed.json", - ) - fake.SeedObject(storage.FakeObject{Key: trimmedKey, Data: []byte(`{"segments":[]}`)}) + seedOperatorCommittedOutputs(t, fake, map[string]string{ + "narratio.transcript.final_trimmed": "transcripts/final.trimmed.json", + }) var storeInitCalls int restoreAppConfigTestGlobals(t, fake, &storeInitCalls, []string{sessionPath}) @@ -961,11 +1009,9 @@ func TestExecuteArtifactsListReportsExtractionLifecycleWithoutPayload(t *testing `) lanePath := writeOperatorExtractionManifest(t, workspaceRoot) fake := &storage.FakeBackend{} - publishedKey := artifacts.S3PublishedOutputKey( - artifacts.S3SessionPrefix("dnd", "sample-campaign", "2026-05-03"), - "artifacts/encounters.json", - ) - fake.SeedObject(storage.FakeObject{Key: publishedKey, Data: []byte(`{"secret":"DO_NOT_PRINT"}`)}) + seedOperatorCommittedOutputs(t, fake, map[string]string{ + "narratio.extraction.encounters": "artifacts/encounters.json", + }) var storeInitCalls int restoreAppConfigTestGlobals(t, fake, &storeInitCalls, []string{sessionPath}) @@ -1027,9 +1073,10 @@ func TestExecuteArtifactsListRemoteUsesPublishOutputDestinations(t *testing.T) { required: true `) fake := &storage.FakeBackend{} - sessionPrefix := artifacts.S3SessionPrefix("dnd", "sample-campaign", "2026-05-03") - fake.SeedObject(storage.FakeObject{Key: artifacts.S3PublishedOutputKey(sessionPrefix, "transcripts/full.json"), Data: []byte(`{"segments":[]}`)}) - fake.SeedObject(storage.FakeObject{Key: artifacts.S3PublishedOutputKey(sessionPrefix, "transcripts/bounds.json"), Data: []byte(`{}`)}) + seedOperatorCommittedOutputs(t, fake, map[string]string{ + "narratio.transcript.final": "transcripts/full.json", + "narratio.bounds.session": "transcripts/bounds.json", + }) var storeInitCalls int restoreAppConfigTestGlobals(t, fake, &storeInitCalls, []string{sessionPath}) @@ -1078,15 +1125,11 @@ func TestExecuteStatusReportsRemoteArtifactCatalog(t *testing.T) { `) fake := &storage.FakeBackend{} sessionPrefix := artifacts.S3SessionPrefix("dnd", "sample-campaign", "2026-05-03") - manifestKey := artifacts.S3RunSessionManifestKey(sessionPrefix, "20260519T010203Z-a1b2c3d4") - runIDKey := artifacts.S3CurrentCommitPointerKey(sessionPrefix) - trimmedKey := artifacts.S3PublishedOutputKey(sessionPrefix, "transcripts/final.trimmed.json") - fullKey := artifacts.S3PublishedOutputKey(sessionPrefix, "transcripts/full.json") lockKey := artifacts.S3SessionLocksKey(sessionPrefix) - fake.SeedObject(storage.FakeObject{Key: runIDKey, Data: []byte("20260519T010203Z-a1b2c3d4\n")}) - fake.SeedObject(storage.FakeObject{Key: manifestKey, Data: restoreManifestJSON(t, "2026-05-03", "sample-campaign")}) - fake.SeedObject(storage.FakeObject{Key: trimmedKey, Data: []byte(`{"segments":[]}`)}) - fake.SeedObject(storage.FakeObject{Key: fullKey, Data: []byte(`{"segments":[]}`)}) + seedOperatorCommittedOutputs(t, fake, map[string]string{ + "narratio.transcript.final_trimmed": "transcripts/final.trimmed.json", + "narratio.transcript.final": "transcripts/full.json", + }) fake.SeedObject(storage.FakeObject{Key: lockKey, Data: []byte("locks:\n - source: narratio.transcript.final_trimmed\n reason: remote review\n")}) var storeInitCalls int restoreAppConfigTestGlobals(t, fake, &storeInitCalls, []string{sessionPath}) diff --git a/internal/app/operator_status.go b/internal/app/operator_status.go index 99af7a1..999e094 100644 --- a/internal/app/operator_status.go +++ b/internal/app/operator_status.go @@ -58,11 +58,13 @@ func Status(ctx context.Context, args []string, out io.Writer) error { store, storeErr := objectStoreIfConfigured(ctx, cfg) var remoteCurrent *RemoteCurrentState + var remoteCurrentErr error if storeErr != nil { fmt.Fprintf(out, "Remote publish: unavailable: %v\n", storeErr) } else if store != nil { current := inspectRemoteCurrentState(ctx, cfg, store) if current.Err != nil { + remoteCurrentErr = current.Err fmt.Fprintf(out, "Remote publish: missing or unavailable: %v\n", current.Err) } else { remoteCurrent = current.State @@ -97,7 +99,11 @@ func Status(ctx context.Context, args []string, out io.Writer) error { } publishedRemoteState := map[string]string{} if store != nil { - publishedRemoteState = remotePublishedOutputAvailabilityForCurrent(ctx, cfg, store, catalog, remoteCurrent) + if remoteCurrentErr != nil { + publishedRemoteState = unresolvedRemotePublishedOutputAvailability(cfg, catalog, "remote=error") + } else { + publishedRemoteState = remotePublishedOutputAvailabilityForCurrent(ctx, cfg, store, catalog, remoteCurrent) + } } fmt.Fprintln(out, "Remote outputs:") writeArtifactList(out, cfg, catalog, catalogLocks, publishedRemoteState) diff --git a/internal/app/restore_commit_test.go b/internal/app/restore_commit_test.go index 41dd071..713af3a 100644 --- a/internal/app/restore_commit_test.go +++ b/internal/app/restore_commit_test.go @@ -43,6 +43,38 @@ func TestCommittedRestorePlanUsesOnlyDeclaredObjects(t *testing.T) { } } +func TestCommittedRestoreDownloadsCarriedOutputFromDeclaredRun(t *testing.T) { + cfg := restorePlanConfig(t) + fake := &storage.FakeBackend{} + current := seedCommittedRestoreSnapshot(t, cfg, fake, "20260519T010203Z-a1b2c3d4", map[string][]byte{ + "transcripts/full.json": []byte("carried\n"), + }) + declaration := ¤t.Commit.Artifacts[1] + oldKey := artifacts.S3RunRelativeDestinationKey(artifacts.S3RunPrefix(current.SessionPrefix, "20260512T010203Z-11223344"), "transcripts/full.json") + fake.SeedObject(storage.FakeObject{Key: oldKey, Data: []byte("carried\n")}) + oldObject := fake.Objects[oldKey] + declaration.DestinationKey = oldKey + declaration.Generation = oldObject.ETag + if err := current.Commit.ValidateForSessionPrefix(current.SessionPrefix); err != nil { + t.Fatalf("carried commit validation: %v", err) + } + + plan, err := buildRestorePlan(context.Background(), cfg, current, fake, RestorePlanOptions{}) + if err != nil { + t.Fatalf("buildRestorePlan() error = %v", err) + } + action := restoreActionForPath(t, plan, "transcripts/full.json") + if action.RemoteKey != oldKey { + t.Fatalf("restore remote key = %q, want exact carried key %q", action.RemoteKey, oldKey) + } + if _, err := executeRestorePlan(context.Background(), cfg, current, plan, nil, fake); err != nil { + t.Fatalf("executeRestorePlan() error = %v", err) + } + if got := fakeDownloadCount(fake, oldKey); got != 1 { + t.Fatalf("carried object downloads = %d, want one", got) + } +} + func TestCommittedRestoreReusesVerifiedManifestCandidate(t *testing.T) { cfg := restorePlanConfig(t) fake := &storage.FakeBackend{} @@ -92,6 +124,7 @@ func TestCommittedStatusReportsOnlyDeclaredPublishedOutputs(t *testing.T) { current := seedCommittedRestoreSnapshot(t, cfg, fake, "20260519T010203Z-a1b2c3d4", map[string][]byte{ "transcripts/full.json": []byte("declared\n"), }) + current.Commit.Artifacts[1].Source = "narratio.transcript.final_trimmed" fake.SeedObject(storage.FakeObject{Key: current.SessionPrefix + "transcripts/full.json", Data: []byte("mutable stale copy\n")}) availability := remotePublishedOutputAvailabilityForCurrent(context.Background(), cfg, fake, nil, current) @@ -101,6 +134,33 @@ func TestCommittedStatusReportsOnlyDeclaredPublishedOutputs(t *testing.T) { } } +func TestCommittedStatusReportsCarriedOutputOriginAndChecksExactKey(t *testing.T) { + cfg := restorePlanConfig(t) + cfg.Pipeline.Publish = &config.PublishConfig{Outputs: []config.PublishOutputRule{{ + Source: "narratio.transcript.final_trimmed", + Dest: "transcripts/full.json", + }}} + fake := &storage.FakeBackend{} + current := seedCommittedRestoreSnapshot(t, cfg, fake, "20260519T010203Z-a1b2c3d4", map[string][]byte{ + "transcripts/full.json": []byte("carried\n"), + }) + const originRun = "20260512T010203Z-11223344" + oldKey := artifacts.S3RunRelativeDestinationKey(artifacts.S3RunPrefix(current.SessionPrefix, originRun), "transcripts/full.json") + fake.SeedObject(storage.FakeObject{Key: oldKey, Data: []byte("carried\n")}) + current.Commit.Artifacts[1].Source = "narratio.transcript.final_trimmed" + current.Commit.Artifacts[1].DestinationKey = oldKey + + availability := remotePublishedOutputAvailabilityForCurrent(context.Background(), cfg, fake, nil, current) + key := publishedOutputRemoteStateKey("narratio.transcript.final_trimmed", "transcripts/full.json") + want := "remote=published origin_run=" + originRun + if availability[key] != want { + t.Fatalf("availability = %#v, want %q", availability, want) + } + if len(fake.ExistsKeys) == 0 || fake.ExistsKeys[len(fake.ExistsKeys)-1] != oldKey { + t.Fatalf("exists keys = %#v, want final exact declaration key %q", fake.ExistsKeys, oldKey) + } +} + func TestCommittedRestoreKeepsSelectedSnapshotWhenPointerChanges(t *testing.T) { cfg := restorePlanConfig(t) fake := &storage.FakeBackend{} diff --git a/internal/app/restore_plan.go b/internal/app/restore_plan.go index 92b6190..fded5bb 100644 --- a/internal/app/restore_plan.go +++ b/internal/app/restore_plan.go @@ -148,13 +148,9 @@ func buildCommittedRestoreActions( if current == nil || current.Commit == nil { return nil, fmt.Errorf("committed remote current state is required") } - runPrefix := artifacts.S3RunPrefix(current.SessionPrefix, current.RunID) - if runPrefix == "" { - return nil, fmt.Errorf("committed run prefix is required") - } actions := make([]RestoreAction, 0, len(current.Commit.Artifacts)) for _, artifact := range current.Commit.Artifacts { - rel, include, err := restoreLocalRelativePathForCommittedArtifact(runPrefix, artifact, opts.IncludeAudio) + rel, include, err := restoreLocalRelativePathForCommittedArtifact(current.SessionPrefix, artifact, opts.IncludeAudio) if err != nil { return nil, fmt.Errorf("map committed object %q: %w", artifact.DestinationKey, err) } @@ -179,18 +175,18 @@ func buildCommittedRestoreActions( return actions, nil } -func restoreLocalRelativePathForCommittedArtifact(runPrefix string, artifact artifacts.RemoteArtifact, includeAudio bool) (string, bool, error) { +func restoreLocalRelativePathForCommittedArtifact(sessionPrefix string, artifact artifacts.RemoteArtifact, includeAudio bool) (string, bool, error) { if artifact.Type == artifacts.RemoteArtifactTypeSessionManifest { return config.PathManifestFile, true, nil } if artifact.Type != artifacts.RemoteArtifactTypePublishedOutput { return "", false, nil } - key := normalizeRemoteKey(artifact.DestinationKey) - if !strings.HasPrefix(key, runPrefix) { - return "", false, fmt.Errorf("object is outside committed run prefix %q", runPrefix) + identity, err := artifacts.ParseImmutableRunObjectKey(sessionPrefix, artifact.DestinationKey) + if err != nil { + return "", false, err } - rel := strings.TrimPrefix(key, runPrefix) + rel := strings.TrimPrefix(identity.RelativePath, "published/") cleanRel, err := pathsafe.NormalizeRelativeDestination(rel) if err != nil { return "", false, fmt.Errorf("committed destination is unsafe: %w", err) diff --git a/internal/app/restore_plan_test.go b/internal/app/restore_plan_test.go index 21dc4ec..b36138b 100644 --- a/internal/app/restore_plan_test.go +++ b/internal/app/restore_plan_test.go @@ -345,7 +345,7 @@ func TestRestorePlanTraversalUnsafeKeyFails(t *testing.T) { if err == nil { t.Fatal("expected error, got nil") } - if !strings.Contains(err.Error(), "committed destination is unsafe") { + if !strings.Contains(err.Error(), "clean relative object key") { t.Fatalf("error = %v, want traversal safety failure", err) } } diff --git a/internal/artifacts/remote_commit.go b/internal/artifacts/remote_commit.go index f515474..5bb43db 100644 --- a/internal/artifacts/remote_commit.go +++ b/internal/artifacts/remote_commit.go @@ -114,7 +114,23 @@ func (m RemoteCommitManifest) ValidateForSessionPrefix(sessionPrefix string) err if runPrefix == "" { return fmt.Errorf("remote commit run prefix is required") } + publishedDestinations := make(map[string]struct{}) for _, artifact := range m.Artifacts { + if artifact.Type == RemoteArtifactTypePublishedOutput { + identity, err := ParseImmutableRunObjectKey(sessionPrefix, artifact.DestinationKey) + if err != nil { + return fmt.Errorf("remote commit published output destination %q: %w", artifact.DestinationKey, err) + } + effectiveDestination := strings.TrimPrefix(identity.RelativePath, "published/") + if effectiveDestination == "" { + return fmt.Errorf("remote commit published output destination %q is empty", artifact.DestinationKey) + } + if _, duplicate := publishedDestinations[effectiveDestination]; duplicate { + return fmt.Errorf("remote commit declares duplicate published output destination %q", effectiveDestination) + } + publishedDestinations[effectiveDestination] = struct{}{} + continue + } if !strings.HasPrefix(artifact.DestinationKey, runPrefix) { return fmt.Errorf("remote commit artifact destination %q is outside immutable run prefix %q", artifact.DestinationKey, runPrefix) } diff --git a/internal/artifacts/remote_commit_test.go b/internal/artifacts/remote_commit_test.go index f67c25b..b14eab8 100644 --- a/internal/artifacts/remote_commit_test.go +++ b/internal/artifacts/remote_commit_test.go @@ -4,6 +4,7 @@ import ( "bytes" "context" "errors" + "fmt" "strings" "testing" @@ -46,6 +47,75 @@ func TestRemoteCommitDecodersRejectUnknownFields(t *testing.T) { } } +func TestParseImmutableRunObjectKeyAcceptsSameSessionRuns(t *testing.T) { + prefix := testCurrentSessionPrefix() + for _, runID := range []string{"20260519T010203Z-a1b2c3d4", "20260401T010203Z-11223344"} { + key := S3RunRelativeDestinationKey(S3RunPrefix(prefix, runID), "published/artifacts/recap.md") + identity, err := ParseImmutableRunObjectKey(prefix, key) + if err != nil { + t.Fatalf("ParseImmutableRunObjectKey(%q) error = %v", runID, err) + } + if identity.RunID != runID || identity.RelativePath != "published/artifacts/recap.md" { + t.Fatalf("ParseImmutableRunObjectKey(%q) = %#v", runID, identity) + } + } +} + +func TestParseImmutableRunObjectKeyRejectsUnsafeOrMutableKeys(t *testing.T) { + prefix := testCurrentSessionPrefix() + for _, key := range []string{ + prefix + "current/commit-pointer.json", + prefix + "locks/source.lock", + prefix + "runs//published/recap.md", + prefix + "runs/20260519T010203Z-a1b2c3d4", + prefix + "runs/20260519T010203Z-a1b2c3d4/published/../recap.md", + "dnd/campaigns/other/sessions/2026-05-03/runs/20260519T010203Z-a1b2c3d4/published/recap.md", + } { + if _, err := ParseImmutableRunObjectKey(prefix, key); err == nil { + t.Fatalf("ParseImmutableRunObjectKey(%q) error = nil, want rejection", key) + } + } +} + +func TestRemoteCommitAllowsPublishedOutputFromEarlierSameSessionRunOnly(t *testing.T) { + commit := testRemoteCommit(t, "20260519T010203Z-a1b2c3d4") + prefix := testCurrentSessionPrefix() + published := RemoteArtifact{ + Type: RemoteArtifactTypePublishedOutput, Source: "narratio.artifact.recap", + DestinationKey: S3RunRelativeDestinationKey(S3RunPrefix(prefix, "20260401T010203Z-11223344"), "published/artifacts/recap.md"), + SHA256: strings.Repeat("a", 64), Size: 1, Generation: "older-generation", + } + commit.Artifacts = append(commit.Artifacts, published) + if err := commit.ValidateForSessionPrefix(prefix); err != nil { + t.Fatalf("ValidateForSessionPrefix() error = %v", err) + } + + commit.Artifacts[1].DestinationKey = "dnd/campaigns/other/sessions/2026-05-03/runs/20260401T010203Z-11223344/published/artifacts/recap.md" + if err := commit.ValidateForSessionPrefix(prefix); err == nil { + t.Fatal("ValidateForSessionPrefix() cross-session error = nil") + } + commit.Artifacts[1].DestinationKey = published.DestinationKey + commit.Artifacts[1].Type = RemoteArtifactTypeRunFile + if err := commit.ValidateForSessionPrefix(prefix); err == nil { + t.Fatal("ValidateForSessionPrefix() earlier non-published object error = nil") + } +} + +func TestRemoteCommitRejectsEffectivePublishedDestinationCollisionAcrossRuns(t *testing.T) { + commit := testRemoteCommit(t, "20260519T010203Z-a1b2c3d4") + prefix := testCurrentSessionPrefix() + for index, runID := range []string{commit.RunID, "20260401T010203Z-11223344"} { + commit.Artifacts = append(commit.Artifacts, RemoteArtifact{ + Type: RemoteArtifactTypePublishedOutput, Source: fmt.Sprintf("narratio.artifact.recap_%d", index), + DestinationKey: S3RunRelativeDestinationKey(S3RunPrefix(prefix, runID), "published/artifacts/recap.md"), + SHA256: strings.Repeat("a", 64), Size: 1, Generation: fmt.Sprintf("generation-%d", index), + }) + } + if err := commit.ValidateForSessionPrefix(prefix); err == nil || !strings.Contains(err.Error(), "duplicate published output destination") { + t.Fatalf("ValidateForSessionPrefix() error = %v, want effective destination collision", err) + } +} + func TestRemoteCommitDecoderAcceptsHistoricalRunManifestArtifact(t *testing.T) { data := []byte(`{"format_version":1,"campaign":"sample-campaign","session_id":"2026-05-03","run_id":"20260519T010203Z-a1b2c3d4","artifacts":[{"type":"run_manifest","source":"run:manifest.json","destination_key":"sample-campaign/2026-05-03/runs/20260519T010203Z-a1b2c3d4/manifest.json","sha256":"0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef","size":1,"generation":"historical"}]}`) diff --git a/internal/artifacts/remote_lock.go b/internal/artifacts/remote_lock.go index 621a267..fa3aaa0 100644 --- a/internal/artifacts/remote_lock.go +++ b/internal/artifacts/remote_lock.go @@ -16,7 +16,6 @@ import ( "gitea.maximumdirect.net/eric/narratio/internal/adapters/storage" "gitea.maximumdirect.net/eric/narratio/internal/artifactpolicy" - "gitea.maximumdirect.net/eric/narratio/internal/config" ) const ( @@ -287,21 +286,7 @@ func remoteLocksPrefix(sessionPrefix string) (string, error) { } func validateRemoteLockSessionPrefix(sessionPrefix string) error { - if sessionPrefix == "" || strings.TrimSpace(sessionPrefix) != sessionPrefix || strings.Contains(sessionPrefix, `\`) || !strings.HasSuffix(sessionPrefix, "/") { - return fmt.Errorf("session prefix %q must be a canonical relative object prefix", sessionPrefix) - } - withoutSlash := strings.TrimSuffix(sessionPrefix, "/") - if err := validateRemoteObjectKey(withoutSlash); err != nil { - return fmt.Errorf("session prefix %q: %w", sessionPrefix, err) - } - parts := strings.Split(withoutSlash, "/") - if len(parts) < 4 || parts[len(parts)-4] != config.S3CampaignsSegment || parts[len(parts)-2] != config.S3SessionsSegment { - return fmt.Errorf("session prefix %q does not identify a campaign session", sessionPrefix) - } - if err := ValidateSessionIdentity(parts[len(parts)-3], parts[len(parts)-1]); err != nil { - return fmt.Errorf("session prefix %q: %w", sessionPrefix, err) - } - return nil + return validateRemoteSessionPrefix(sessionPrefix) } func remoteLockDigest(source string) string { diff --git a/internal/artifacts/remote_object_key.go b/internal/artifacts/remote_object_key.go new file mode 100644 index 0000000..9a2a7b5 --- /dev/null +++ b/internal/artifacts/remote_object_key.go @@ -0,0 +1,60 @@ +package artifacts + +import ( + "fmt" + "strings" + + "gitea.maximumdirect.net/eric/narratio/internal/config" +) + +// ImmutableRunObjectKey identifies one clean object beneath a session's +// immutable run namespace. +type ImmutableRunObjectKey struct { + RunID string + RelativePath string +} + +// ParseImmutableRunObjectKey proves that key belongs to one immutable run in +// the expected campaign/session namespace and returns its run and relative path. +func ParseImmutableRunObjectKey(sessionPrefix, key string) (ImmutableRunObjectKey, error) { + if err := validateRemoteSessionPrefix(sessionPrefix); err != nil { + return ImmutableRunObjectKey{}, err + } + if err := validateRemoteObjectKey(key); err != nil { + return ImmutableRunObjectKey{}, fmt.Errorf("immutable object key %q: %w", key, err) + } + runsPrefix := sessionPrefix + config.S3RunsSegment + "/" + remainder, ok := strings.CutPrefix(key, runsPrefix) + if !ok { + return ImmutableRunObjectKey{}, fmt.Errorf("immutable object key %q is outside session run prefix %q", key, runsPrefix) + } + runID, relativePath, ok := strings.Cut(remainder, "/") + if !ok || runID == "" { + return ImmutableRunObjectKey{}, fmt.Errorf("immutable object key %q has an empty run identity", key) + } + if err := ValidateRunIdentity(runID); err != nil { + return ImmutableRunObjectKey{}, fmt.Errorf("immutable object key %q: %w", key, err) + } + if err := validateRemoteObjectKey(relativePath); err != nil { + return ImmutableRunObjectKey{}, fmt.Errorf("immutable object key %q has invalid run-relative path: %w", key, err) + } + return ImmutableRunObjectKey{RunID: runID, RelativePath: relativePath}, nil +} + +func validateRemoteSessionPrefix(sessionPrefix string) error { + if sessionPrefix == "" || strings.TrimSpace(sessionPrefix) != sessionPrefix || strings.Contains(sessionPrefix, `\`) || !strings.HasSuffix(sessionPrefix, "/") { + return fmt.Errorf("session prefix %q must be a canonical relative object prefix", sessionPrefix) + } + withoutSlash := strings.TrimSuffix(sessionPrefix, "/") + if err := validateRemoteObjectKey(withoutSlash); err != nil { + return fmt.Errorf("session prefix %q: %w", sessionPrefix, err) + } + parts := strings.Split(withoutSlash, "/") + if len(parts) < 4 || parts[len(parts)-4] != config.S3CampaignsSegment || parts[len(parts)-2] != config.S3SessionsSegment { + return fmt.Errorf("session prefix %q does not identify a campaign session", sessionPrefix) + } + if err := ValidateSessionIdentity(parts[len(parts)-3], parts[len(parts)-1]); err != nil { + return fmt.Errorf("session prefix %q: %w", sessionPrefix, err) + } + return nil +} diff --git a/internal/previouscache/previouscache.go b/internal/previouscache/previouscache.go index 4a5d4a9..1751a5c 100644 --- a/internal/previouscache/previouscache.go +++ b/internal/previouscache/previouscache.go @@ -155,7 +155,7 @@ func Resolve( result.SkippedMissing = append(result.SkippedMissing, requirement.Name) continue } - selectedRel, err := publishedOutputRelativePath(previousSessionPrefix, current.RunID, selectedArtifact.DestinationKey) + selectedRel, err := publishedOutputRelativePath(previousSessionPrefix, selectedArtifact.DestinationKey) if err != nil { return nil, fmt.Errorf("resolve declared previous-session artifact %q: %w", requirement.Name, err) } @@ -209,11 +209,14 @@ func committedPublishedArtifactBySource(commit *artifacts.RemoteCommitManifest, return artifacts.RemoteArtifact{}, false } -func publishedOutputRelativePath(sessionPrefix, runID, destinationKey string) (string, error) { - prefix := artifacts.S3RunRelativeDestinationKey(artifacts.S3RunPrefix(sessionPrefix, runID), "published") + "/" - rel, ok := strings.CutPrefix(strings.TrimSpace(destinationKey), prefix) +func publishedOutputRelativePath(sessionPrefix, destinationKey string) (string, error) { + identity, err := artifacts.ParseImmutableRunObjectKey(sessionPrefix, destinationKey) + if err != nil { + return "", err + } + rel, ok := strings.CutPrefix(identity.RelativePath, "published/") if !ok || strings.TrimSpace(rel) == "" { - return "", fmt.Errorf("destination %q is outside selected run published-output prefix %q", destinationKey, prefix) + return "", fmt.Errorf("destination %q is outside its immutable run published-output prefix", destinationKey) } normalized, err := pathsafe.NormalizeRelativeDestination(rel) if err != nil { diff --git a/internal/previouscache/previouscache_test.go b/internal/previouscache/previouscache_test.go index a03dc50..7afc504 100644 --- a/internal/previouscache/previouscache_test.go +++ b/internal/previouscache/previouscache_test.go @@ -42,6 +42,23 @@ func TestResolveUsesExactCommittedDeclaration(t *testing.T) { } } +func TestResolveUsesCarriedDeclarationFromEarlierRun(t *testing.T) { + cfg, paths := previousCacheTestConfig(t) + store := &storage.FakeBackend{} + declaration := committedPublishedArtifactForTest("narratio.artifact.session_recap", "artifacts/session_recap.md", []byte("# recap\n"), "recap-generation") + declaration.DestinationKey = strings.Replace(declaration.DestinationKey, "/runs/previous-run/", "/runs/origin-run/", 1) + exactKey := seedCommittedPreviousState(t, store, cfg, previousManifestWithOutput(t, cfg, "artifacts/session_recap.md", nil), []artifacts.RemoteArtifact{declaration}) + + plan, err := Resolve(context.Background(), cfg, paths, []artifacts.PreviousArtifactRequirement{{Name: "session_recap", Required: true}}, store) + if err != nil { + t.Fatalf("Resolve() error = %v", err) + } + record := findRecord(plan.Records, InputKindArtifact) + if record == nil || record.RemoteKey != exactKey { + t.Fatalf("artifact record = %#v, want exact carried key %q", record, exactKey) + } +} + func TestResolveDerivesLocalPathFromCommittedDestination(t *testing.T) { cfg, paths := previousCacheTestConfig(t) cfg.Pipeline.Scriptorium.Artifacts["session_recap"] = config.ScriptoriumArtifactConfig{Enabled: true, OutputPath: "configured/ignored.md"} @@ -63,12 +80,12 @@ func TestResolveDerivesLocalPathFromCommittedDestination(t *testing.T) { func TestPublishedOutputRelativePathRejectsUnsafeDestination(t *testing.T) { sessionPrefix := artifacts.S3SessionPrefix("dnd", "sample-campaign", "2026-04-26") - _, err := publishedOutputRelativePath(sessionPrefix, "previous-run", artifacts.S3RunPrefix(sessionPrefix, "previous-run")+"artifacts/session_recap.md") - if err == nil || !strings.Contains(err.Error(), "outside selected run published-output prefix") { + _, err := publishedOutputRelativePath(sessionPrefix, artifacts.S3RunPrefix(sessionPrefix, "previous-run")+"artifacts/session_recap.md") + if err == nil || !strings.Contains(err.Error(), "outside its immutable run published-output prefix") { t.Fatalf("publishedOutputRelativePath() error = %v, want prefix rejection", err) } - _, err = publishedOutputRelativePath(sessionPrefix, "previous-run", artifacts.S3RunPrefix(sessionPrefix, "previous-run")+"published/../session_recap.md") - if err == nil || !strings.Contains(err.Error(), "published output destination") { + _, err = publishedOutputRelativePath(sessionPrefix, artifacts.S3RunPrefix(sessionPrefix, "previous-run")+"published/../session_recap.md") + if err == nil || !strings.Contains(err.Error(), "clean relative object key") { t.Fatalf("publishedOutputRelativePath() error = %v, want unsafe destination rejection", err) } }