From ba858cc9978078abc79a87e2f655a7da23ded491 Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Tue, 1 Sep 2026 17:51:37 +0000 Subject: [PATCH] Migrate ordinary I/O from confined file helpers --- docs/roadmap/implementation.md | 2 +- internal/adapters/audita/subprocess.go | 2 +- internal/adapters/notarius/subprocess.go | 2 +- internal/adapters/scriptorium/subprocess.go | 2 +- internal/adapters/seriatim/subprocess.go | 2 +- internal/adapters/subprocess/diagnostics.go | 2 +- internal/adapters/subprocess/run_test.go | 8 +- internal/app/secrets_env.go | 40 +++-- internal/artifacts/analyze_evidence.go | 4 +- internal/artifacts/analyze_evidence_test.go | 26 +-- internal/artifacts/artifact_resolver.go | 2 +- internal/artifacts/extraction_evidence.go | 2 +- internal/artifacts/extraction_input.go | 2 +- internal/artifacts/local.go | 5 +- internal/artifacts/prepared_input.go | 9 +- internal/artifacts/prepared_input_test.go | 16 -- internal/audio/cache_identity.go | 8 +- internal/config/party_resolution.go | 2 +- internal/config/pipeline_composition.go | 5 +- internal/config/pipeline_composition_test.go | 14 +- internal/config/pipeline_profiles_test.go | 2 +- internal/contracts/bounds.go | 2 +- internal/fileops/confined_open_test.go | 148 ------------------ internal/fileops/confined_read.go | 78 --------- internal/fileops/read_confined.go | 121 -------------- internal/fileops/read_confined_test.go | 65 -------- internal/stage/analyze_failure_test.go | 6 +- internal/stage/analyze_input_identity.go | 8 +- internal/stage/analyze_input_identity_test.go | 15 -- internal/stage/external_result.go | 2 +- internal/stage/extract.go | 2 +- internal/stage/extract_references.go | 4 +- internal/stage/publish.go | 16 +- internal/stage/publish_test.go | 19 +-- 34 files changed, 96 insertions(+), 547 deletions(-) delete mode 100644 internal/fileops/confined_open_test.go delete mode 100644 internal/fileops/confined_read.go delete mode 100644 internal/fileops/read_confined.go delete mode 100644 internal/fileops/read_confined_test.go diff --git a/docs/roadmap/implementation.md b/docs/roadmap/implementation.md index 2e55030..e63a365 100644 --- a/docs/roadmap/implementation.md +++ b/docs/roadmap/implementation.md @@ -237,7 +237,7 @@ replacement. ## Stage 2 — Migrate Ordinary I/O And Isolate Credential Handling -**Status: Pending** +**Status: Completed** ### Goal diff --git a/internal/adapters/audita/subprocess.go b/internal/adapters/audita/subprocess.go index 96d3c0f..e2e7f37 100644 --- a/internal/adapters/audita/subprocess.go +++ b/internal/adapters/audita/subprocess.go @@ -425,7 +425,7 @@ func validateJSONFile(path string) error { } func readAuditaResult(path string, limit int64, category string) ([]byte, error) { - data, err := fileops.ReadRegularFile(path, limit) + data, err := fileops.ReadFileBounded(path, limit) if err != nil { return nil, fmt.Errorf("audita %s result exceeds or cannot be read within %d-byte limit: %w", category, limit, err) } diff --git a/internal/adapters/notarius/subprocess.go b/internal/adapters/notarius/subprocess.go index f9231bd..941a462 100644 --- a/internal/adapters/notarius/subprocess.go +++ b/internal/adapters/notarius/subprocess.go @@ -671,7 +671,7 @@ func sumWarningOccurrences(values []WarningSummary) (int, error) { } func decodeBoundedJSON(path string, limit int64, destination any) error { - data, err := fileops.ReadRegularFile(path, limit) + data, err := fileops.ReadFileBounded(path, limit) if err != nil { return fmt.Errorf("notarius JSON result exceeds or cannot be read within %d-byte limit: %w", limit, err) } diff --git a/internal/adapters/scriptorium/subprocess.go b/internal/adapters/scriptorium/subprocess.go index 21c0e76..d85e0cd 100644 --- a/internal/adapters/scriptorium/subprocess.go +++ b/internal/adapters/scriptorium/subprocess.go @@ -346,7 +346,7 @@ func writeInvocationConfig(path string, payload invocationPayload) error { } func validateNonEmptyOutput(path string) error { - data, err := fileops.ReadRegularFile(path, MaxOutputFileBytes) + data, err := fileops.ReadFileBounded(path, MaxOutputFileBytes) if err != nil { return fmt.Errorf("scriptorium artifact output exceeds or cannot be read within %d-byte limit: %w", MaxOutputFileBytes, err) } diff --git a/internal/adapters/seriatim/subprocess.go b/internal/adapters/seriatim/subprocess.go index 06b347e..c7ce014 100644 --- a/internal/adapters/seriatim/subprocess.go +++ b/internal/adapters/seriatim/subprocess.go @@ -692,7 +692,7 @@ func validateNonEmptyTextFile(path string) error { } func readSeriatimResult(path, category string) ([]byte, error) { - data, err := fileops.ReadRegularFile(path, MaxOutputFileBytes) + data, err := fileops.ReadFileBounded(path, MaxOutputFileBytes) if err != nil { return nil, fmt.Errorf("seriatim %s exceeds or cannot be read within %d-byte limit: %w", category, MaxOutputFileBytes, err) } diff --git a/internal/adapters/subprocess/diagnostics.go b/internal/adapters/subprocess/diagnostics.go index a56eccc..eba5c91 100644 --- a/internal/adapters/subprocess/diagnostics.go +++ b/internal/adapters/subprocess/diagnostics.go @@ -255,7 +255,7 @@ func openDiagnosticFile(path string) (*os.File, error) { if err := fileops.EnsureWorkspaceDirectory(filepath.Dir(path)); err != nil { return nil, fmt.Errorf("create log directory for %q: %w", path, err) } - file, err := fileops.OpenFileConfined(path, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, fileops.WorkspaceFileMode) + file, err := os.OpenFile(path, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, fileops.WorkspaceFileMode) if err != nil { return nil, fmt.Errorf("open log file %q: %w", path, err) } diff --git a/internal/adapters/subprocess/run_test.go b/internal/adapters/subprocess/run_test.go index 4fcfb64..325673d 100644 --- a/internal/adapters/subprocess/run_test.go +++ b/internal/adapters/subprocess/run_test.go @@ -163,15 +163,15 @@ func TestRunRejectsSymlinkDiagnosticWithoutTruncatingTarget(t *testing.T) { StdoutLogPath: stdoutPath, StderrLogPath: filepath.Join(dir, "stderr.log"), }) - if err == nil || !strings.Contains(err.Error(), "symbolic link") { - t.Fatalf("Run() error = %v, want symbolic-link rejection", err) + if err != nil { + t.Fatalf("Run() error = %v", err) } data, readErr := os.ReadFile(targetPath) if readErr != nil { t.Fatalf("ReadFile(target) error = %v", readErr) } - if string(data) != original { - t.Fatalf("target content = %q, want %q", data, original) + if string(data) == original { + t.Fatal("diagnostic target was not replaced") } } diff --git a/internal/app/secrets_env.go b/internal/app/secrets_env.go index 7338e01..84fd240 100644 --- a/internal/app/secrets_env.go +++ b/internal/app/secrets_env.go @@ -49,7 +49,14 @@ func loadSecretsFromConfig(cfg *config.Config, logger *slog.Logger) (*secretsLoa } resolvedDir = filepath.Clean(resolvedDir) - if err := fileops.ValidateConfinedDirectory(resolvedDir, validateSecretDirectory); err != nil { + directoryInfo, err := os.Lstat(resolvedDir) + if err != nil { + return nil, fmt.Errorf("API-key loader: validate secrets env_dir: %w", err) + } + if directoryInfo.Mode()&os.ModeSymlink != 0 { + return nil, fmt.Errorf("API-key loader: secrets env_dir must not be a symbolic link") + } + if err := validateSecretDirectory(directoryInfo); err != nil { return nil, fmt.Errorf("API-key loader: validate secrets env_dir: %w", err) } @@ -57,6 +64,13 @@ func loadSecretsFromConfig(cfg *config.Config, logger *slog.Logger) (*secretsLoa if err != nil { return nil, fmt.Errorf("API-key loader: read secrets env_dir %q: %w", resolvedDir, err) } + directoryInfo, err = os.Lstat(resolvedDir) + if err != nil || directoryInfo.Mode()&os.ModeSymlink != 0 { + if err == nil { + err = fmt.Errorf("symbolic links are not allowed") + } + return nil, fmt.Errorf("API-key loader: validate secrets env_dir: %w", err) + } stats := &secretsLoadStats{Dir: resolvedDir} for _, entry := range entries { @@ -107,15 +121,23 @@ func validateSecretFile(info os.FileInfo) error { } func readAPIKeyFile(directory, name string) (string, error) { - content, err := fileops.ReadRegularFileUnderRoot( - directory, - name, - APIKeyFileByteLimit, - validateSecretDirectory, - validateSecretFile, - ) + path := filepath.Join(directory, name) + info, err := os.Lstat(path) + if err == nil && info.Mode()&os.ModeSymlink != 0 { + return "", fmt.Errorf("API-key loader: read secret file %q with %d-byte limit: symbolic links are not allowed", name, APIKeyFileByteLimit) + } + if err == nil { + err = validateSecretFile(info) + } + if err == nil { + content, readErr := fileops.ReadFileBounded(path, APIKeyFileByteLimit) + if readErr == nil { + return strings.TrimRight(string(content), "\r\n"), nil + } + err = readErr + } if err != nil { return "", fmt.Errorf("API-key loader: read secret file %q with %d-byte limit: %w", name, APIKeyFileByteLimit, err) } - return strings.TrimRight(string(content), "\r\n"), nil + return "", fmt.Errorf("API-key loader: read secret file %q with %d-byte limit", name, APIKeyFileByteLimit) } diff --git a/internal/artifacts/analyze_evidence.go b/internal/artifacts/analyze_evidence.go index 860b2cf..c0e4623 100644 --- a/internal/artifacts/analyze_evidence.go +++ b/internal/artifacts/analyze_evidence.go @@ -5,11 +5,11 @@ import ( "encoding/hex" "fmt" "io" + "os" "path/filepath" "strings" "gitea.maximumdirect.net/eric/narratio/internal/artifactmodel" - "gitea.maximumdirect.net/eric/narratio/internal/fileops" "gitea.maximumdirect.net/eric/narratio/internal/manifest" "gitea.maximumdirect.net/eric/narratio/internal/pathsafe" ) @@ -85,7 +85,7 @@ func InspectAnalyzeEvidence( return nonCurrent("configured artifact manifest identity is incompatible; regenerate the artifact") } - file, err := fileops.OpenConfinedRegularFile(paths.Root, configuredPath) + file, err := os.Open(filepath.Join(paths.Root, filepath.FromSlash(configuredPath))) if err != nil { return nonCurrent("configured artifact output is missing or unsafe; regenerate the artifact") } diff --git a/internal/artifacts/analyze_evidence_test.go b/internal/artifacts/analyze_evidence_test.go index 6efbf34..e7940bb 100644 --- a/internal/artifacts/analyze_evidence_test.go +++ b/internal/artifacts/analyze_evidence_test.go @@ -104,30 +104,6 @@ func TestInspectAnalyzeEvidenceRejectsMissingAndUnsafeFiles(t *testing.T) { t.Fatal(err) } }}, - {name: "symlink leaf", alter: func(t *testing.T, paths SessionPaths, outputPath string) { - if err := os.Remove(outputPath); err != nil { - t.Fatal(err) - } - target := filepath.Join(paths.Root, "target.md") - if err := os.WriteFile(target, []byte("recap\n"), 0o644); err != nil { - t.Fatal(err) - } - if err := os.Symlink(target, outputPath); err != nil { - t.Fatal(err) - } - }}, - {name: "symlink ancestor", alter: func(t *testing.T, paths SessionPaths, outputPath string) { - if err := os.RemoveAll(paths.ArtifactsDir); err != nil { - t.Fatal(err) - } - outside := t.TempDir() - if err := os.WriteFile(filepath.Join(outside, "session_recap.md"), []byte("recap\n"), 0o644); err != nil { - t.Fatal(err) - } - if err := os.Symlink(outside, paths.ArtifactsDir); err != nil { - t.Fatal(err) - } - }}, } for _, test := range tests { t.Run(test.name, func(t *testing.T) { @@ -136,7 +112,7 @@ func TestInspectAnalyzeEvidenceRejectsMissingAndUnsafeFiles(t *testing.T) { outputPath := filepath.Join(paths.ArtifactsDir, "session_recap.md") test.alter(t, paths, outputPath) got := InspectAnalyzeEvidence(paths, m, "session_recap", ConfiguredArtifactDefinition{OutputPath: "artifacts/session_recap.md"}) - if got.State != AnalyzeEvidenceNonCurrent || !strings.Contains(got.Reason, "missing or unsafe") { + if got.State != AnalyzeEvidenceNonCurrent { t.Fatalf("evidence = %#v", got) } }) diff --git a/internal/artifacts/artifact_resolver.go b/internal/artifacts/artifact_resolver.go index 80dccc1..be7d9a8 100644 --- a/internal/artifacts/artifact_resolver.go +++ b/internal/artifacts/artifact_resolver.go @@ -497,7 +497,7 @@ func validateNonEmptyContent(path string) error { } func readResolvedArtifact(path, category string) ([]byte, error) { - data, err := fileops.ReadRegularFile(path, MaxResolvedArtifactBytes) + data, err := fileops.ReadFileBounded(path, MaxResolvedArtifactBytes) if err != nil { return nil, fmt.Errorf("resolved %s exceeds or cannot be read within %d-byte limit: %w", category, MaxResolvedArtifactBytes, err) } diff --git a/internal/artifacts/extraction_evidence.go b/internal/artifacts/extraction_evidence.go index 85d0b0b..3bbaf5f 100644 --- a/internal/artifacts/extraction_evidence.go +++ b/internal/artifacts/extraction_evidence.go @@ -144,7 +144,7 @@ func inspectExtractionPayload(root, path, checksum string) (ExtractionEvidenceSt if actual != checksum { return ExtractionEvidenceObsolete, "extract output checksum does not match durable bytes", 0 } - body, err := fileops.ReadRegularFile(path, MaxExtractionPayloadBytes) + body, err := fileops.ReadFileBounded(path, MaxExtractionPayloadBytes) if err != nil || !json.Valid(body) { return ExtractionEvidenceObsolete, "extract output is not valid JSON", 0 } diff --git a/internal/artifacts/extraction_input.go b/internal/artifacts/extraction_input.go index d728a43..2dd001d 100644 --- a/internal/artifacts/extraction_input.go +++ b/internal/artifacts/extraction_input.go @@ -34,7 +34,7 @@ func ResolveExtractionInputIdentity(paths SessionPaths, m *manifest.Manifest) (E if err != nil { return ExtractionInputIdentity{}, fmt.Errorf("resolve trimmed transcript path: %w", err) } - bytes, err := fileops.ReadRegularFile(path, MaxResolvedArtifactBytes) + bytes, err := fileops.ReadFileBounded(path, MaxResolvedArtifactBytes) if err != nil { return ExtractionInputIdentity{}, fmt.Errorf("read trimmed transcript: %w", err) } diff --git a/internal/artifacts/local.go b/internal/artifacts/local.go index 38f99f3..5b13940 100644 --- a/internal/artifacts/local.go +++ b/internal/artifacts/local.go @@ -207,7 +207,10 @@ func (s *LocalStore) AcquireSessionLockForContext(ctx context.Context, campaign, } func (s *LocalStore) acquireSessionLockForPaths(paths SessionPaths) (*LockHandle, error) { - f, err := fileops.OpenFileConfined(paths.LockPath, os.O_CREATE|os.O_RDWR, fileops.WorkspaceFileMode) + if err := fileops.EnsureWorkspaceDirectory(filepath.Dir(paths.LockPath)); err != nil { + return nil, fmt.Errorf("create lock directory %q: %w", filepath.Dir(paths.LockPath), err) + } + f, err := os.OpenFile(paths.LockPath, os.O_CREATE|os.O_RDWR, fileops.WorkspaceFileMode) if err != nil { return nil, fmt.Errorf("acquire lock %q: %w", paths.LockPath, err) } diff --git a/internal/artifacts/prepared_input.go b/internal/artifacts/prepared_input.go index ab166d0..1776efd 100644 --- a/internal/artifacts/prepared_input.go +++ b/internal/artifacts/prepared_input.go @@ -6,11 +6,11 @@ import ( "errors" "fmt" "io" + "os" "path/filepath" "strings" "gitea.maximumdirect.net/eric/narratio/internal/artifactpolicy" - "gitea.maximumdirect.net/eric/narratio/internal/fileops" "gitea.maximumdirect.net/eric/narratio/internal/manifest" ) @@ -107,11 +107,16 @@ func ResolvePreparedInput(paths SessionPaths, m *manifest.Manifest, sourceID str return PreparedInputIdentity{}, fmt.Errorf("prepared input source %q manifest checksum is required", descriptor.SourceID) } - file, err := fileops.OpenConfinedRegularFile(rootPath, relativePath) + file, err := os.Open(filepath.Join(rootPath, filepath.FromSlash(relativePath))) if err != nil { return PreparedInputIdentity{}, fmt.Errorf("open prepared input source %q: %w", descriptor.SourceID, err) } digest := sha256.New() + info, statErr := file.Stat() + if statErr != nil || !info.Mode().IsRegular() { + _ = file.Close() + return PreparedInputIdentity{}, fmt.Errorf("prepared input source %q is not a regular file", descriptor.SourceID) + } size, readErr := io.Copy(digest, file) closeErr := file.Close() if readErr != nil { diff --git a/internal/artifacts/prepared_input_test.go b/internal/artifacts/prepared_input_test.go index b40b72a..5b270ca 100644 --- a/internal/artifacts/prepared_input_test.go +++ b/internal/artifacts/prepared_input_test.go @@ -130,22 +130,6 @@ func TestResolvePreparedInputRejectsInvalidCanonicalFile(t *testing.T) { }, wantErr: "open prepared input source", }, - { - name: "symlink", - mutate: func(t *testing.T, path string) { - outside := filepath.Join(t.TempDir(), "outside.json") - if err := os.WriteFile(outside, []byte("outside\n"), 0o644); err != nil { - t.Fatalf("WriteFile(outside) error = %v", err) - } - if err := os.Remove(path); err != nil { - t.Fatalf("Remove() error = %v", err) - } - if err := os.Symlink(outside, path); err != nil { - t.Fatalf("Symlink() error = %v", err) - } - }, - wantErr: "not a regular file", - }, { name: "directory", mutate: func(t *testing.T, path string) { diff --git a/internal/audio/cache_identity.go b/internal/audio/cache_identity.go index 0655ee3..512e4e8 100644 --- a/internal/audio/cache_identity.go +++ b/internal/audio/cache_identity.go @@ -57,7 +57,7 @@ func cacheIdentityEligible(req S3MaterializeRequest) bool { } func loadAudioCacheIdentity(path string) (audioCacheIdentity, bool, error) { - data, err := fileops.ReadRegularFile(path, 64*1024) + data, err := fileops.ReadFileBounded(path, 64*1024) if os.IsNotExist(err) { return audioCacheIdentity{}, false, nil } @@ -99,11 +99,7 @@ func materializeVerifiedCachedAudio(ctx context.Context, req S3MaterializeReques return "", false, nil } - relativePath, err := cacheRelativePath(req.CacheRoot, cachePath) - if err != nil { - return "", false, err - } - source, err := fileops.OpenConfinedRegularFile(req.CacheRoot, relativePath) + source, err := os.Open(cachePath) if err != nil { return "", false, nil } diff --git a/internal/config/party_resolution.go b/internal/config/party_resolution.go index 8d019d4..108c410 100644 --- a/internal/config/party_resolution.go +++ b/internal/config/party_resolution.go @@ -24,7 +24,7 @@ func resolvePartyInput(input ResolvedInputFile) (ResolvedParty, error) { path = filepath.Join(filepath.Dir(configPath), path) } path = filepath.Clean(path) - raw, err := fileops.ReadRegularFile(path, maxPartyDocumentBytes) + raw, err := fileops.ReadFileBounded(path, maxPartyDocumentBytes) if err != nil { return ResolvedParty{}, fmt.Errorf("read party input %q: %w", path, err) } diff --git a/internal/config/pipeline_composition.go b/internal/config/pipeline_composition.go index c0c001f..a8835f0 100644 --- a/internal/config/pipeline_composition.go +++ b/internal/config/pipeline_composition.go @@ -7,7 +7,6 @@ import ( "strings" "unicode" - "gitea.maximumdirect.net/eric/narratio/internal/fileops" "gitea.maximumdirect.net/eric/narratio/internal/pathsafe" "gopkg.in/yaml.v3" ) @@ -386,7 +385,7 @@ func loadPipelineProfileOverlays( } seenPaths[normalized] = profile.name resolved := filepath.Join(rootDir, filepath.FromSlash(normalized)) - file, err := fileops.OpenConfinedRegularFile(rootDir, normalized) + file, err := os.Open(resolved) if err != nil { return nil, fmt.Errorf("open %s path %q beneath root pipeline directory: %w", label, raw, err) } @@ -473,7 +472,7 @@ func loadPipelineImports(rootPath string, declared []string) ([]loadedPipelineIm return nil, fmt.Errorf("composition.imports[%d] path %q imports the root pipeline itself", index, raw) } - file, err := fileops.OpenConfinedRegularFile(rootDir, normalized) + file, err := os.Open(resolved) if err != nil { return nil, fmt.Errorf("open composition.imports[%d] path %q beneath root pipeline directory: %w", index, raw, err) } diff --git a/internal/config/pipeline_composition_test.go b/internal/config/pipeline_composition_test.go index 97ee9a4..75ce016 100644 --- a/internal/config/pipeline_composition_test.go +++ b/internal/config/pipeline_composition_test.go @@ -181,19 +181,7 @@ func TestLoadPipelineCompositionRejectsUnsafeOrInvalidImports(t *testing.T) { if err := os.Mkdir(filepath.Join(dir, "directory.yml"), 0o755); err != nil { t.Fatal(err) } - }, want: "regular file"}, - {name: "symlink file", imports: []string{"link.yml"}, setup: func(t *testing.T, dir string) { - writePipelineSource(t, dir, "target.yml", "workspace:\n root: /work\n") - if err := os.Symlink("target.yml", filepath.Join(dir, "link.yml")); err != nil { - t.Skipf("symlink unavailable: %v", err) - } - }, want: "not a regular file"}, - {name: "symlink directory", imports: []string{"linked/one.yml"}, setup: func(t *testing.T, dir string) { - writePipelineSource(t, dir, "actual/one.yml", "workspace:\n root: /work\n") - if err := os.Symlink("actual", filepath.Join(dir, "linked")); err != nil { - t.Skipf("symlink unavailable: %v", err) - } - }, want: "not a regular directory"}, + }, want: "is a directory"}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/internal/config/pipeline_profiles_test.go b/internal/config/pipeline_profiles_test.go index ce88e7a..8910c66 100644 --- a/internal/config/pipeline_profiles_test.go +++ b/internal/config/pipeline_profiles_test.go @@ -148,7 +148,7 @@ func TestLoadPipelineProfilesValidateEveryDeclaredOverlay(t *testing.T) { if err := os.MkdirAll(filepath.Join(dir, "profiles/testing.yml"), 0o755); err != nil { t.Fatal(err) } - }, want: "regular file"}, + }, want: "is a directory"}, } { t.Run(test.name, func(t *testing.T) { dir := t.TempDir() diff --git a/internal/contracts/bounds.go b/internal/contracts/bounds.go index 26e294a..3e416ac 100644 --- a/internal/contracts/bounds.go +++ b/internal/contracts/bounds.go @@ -164,7 +164,7 @@ func loadTranscriptSegmentIDs(path string) (map[int]struct{}, error) { } func readContractResult(path string, limit int64, category string) ([]byte, error) { - data, err := fileops.ReadRegularFile(path, limit) + data, err := fileops.ReadFileBounded(path, limit) if err != nil { return nil, fmt.Errorf("%s exceeds or cannot be read within %d-byte limit: %w", category, limit, err) } diff --git a/internal/fileops/confined_open_test.go b/internal/fileops/confined_open_test.go deleted file mode 100644 index 2bc1c1b..0000000 --- a/internal/fileops/confined_open_test.go +++ /dev/null @@ -1,148 +0,0 @@ -package fileops - -import ( - "os" - "path/filepath" - "runtime" - "strings" - "testing" -) - -func TestOpenFileConfinedRejectsNonRegularLeaf(t *testing.T) { - leaf := filepath.Join(t.TempDir(), "diagnostic.log") - if err := os.Mkdir(leaf, 0o700); err != nil { - t.Fatalf("Mkdir() error = %v", err) - } - file, err := OpenFileConfined(leaf, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, WorkspaceFileMode) - if file != nil { - _ = file.Close() - t.Fatal("OpenFileConfined() returned a file for a directory") - } - if err == nil || !strings.Contains(err.Error(), "not a regular file") { - t.Fatalf("OpenFileConfined() error = %v, want regular-file rejection", err) - } -} - -func TestOpenFileConfinedRejectsSymlinkWithoutTruncatingTarget(t *testing.T) { - if runtime.GOOS == "windows" { - t.Skip("creating symlinks requires privileges that are not available on every Windows runner") - } - dir := t.TempDir() - target := filepath.Join(dir, "target.log") - const original = "outside content" - if err := os.WriteFile(target, []byte(original), 0o600); err != nil { - t.Fatalf("WriteFile(target) error = %v", err) - } - leaf := filepath.Join(dir, "diagnostic.log") - if err := os.Symlink(target, leaf); err != nil { - t.Fatalf("Symlink() error = %v", err) - } - - file, err := OpenFileConfined(leaf, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, WorkspaceFileMode) - if file != nil { - _ = file.Close() - t.Fatal("OpenFileConfined() returned a file for a symbolic link") - } - if err == nil || !strings.Contains(err.Error(), "symbolic link") { - t.Fatalf("OpenFileConfined() error = %v, want symbolic-link rejection", err) - } - data, readErr := os.ReadFile(target) - if readErr != nil { - t.Fatalf("ReadFile(target) error = %v", readErr) - } - if string(data) != original { - t.Fatalf("target content = %q, want %q", data, original) - } -} - -func TestOpenFileConfinedDoesNotFollowLeafReplacedBeforeTruncate(t *testing.T) { - if runtime.GOOS == "windows" { - t.Skip("creating symlinks requires privileges that are not available on every Windows runner") - } - dir := t.TempDir() - target := filepath.Join(dir, "target.log") - const original = "unrelated content" - if err := os.WriteFile(target, []byte(original), 0o600); err != nil { - t.Fatalf("WriteFile(target) error = %v", err) - } - leaf := filepath.Join(dir, "diagnostic.log") - if err := os.WriteFile(leaf, []byte("old diagnostic"), 0o600); err != nil { - t.Fatalf("WriteFile(leaf) error = %v", err) - } - - file, err := openFileConfined(leaf, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, WorkspaceFileMode, func() error { - if err := os.Remove(leaf); err != nil { - return err - } - return os.Symlink(filepath.Base(target), leaf) - }) - if err != nil { - t.Fatalf("openFileConfined() error = %v", err) - } - if _, err := file.WriteString("new diagnostic"); err != nil { - _ = file.Close() - t.Fatalf("WriteString() error = %v", err) - } - if err := file.Close(); err != nil { - t.Fatalf("Close() error = %v", err) - } - - targetData, err := os.ReadFile(target) - if err != nil { - t.Fatalf("ReadFile(target) error = %v", err) - } - if string(targetData) != original { - t.Fatalf("target content = %q, want %q", targetData, original) - } - leafData, err := os.ReadFile(leaf) - if err != nil { - t.Fatalf("ReadFile(leaf) error = %v", err) - } - if string(leafData) != "new diagnostic" { - t.Fatalf("leaf content = %q, want replacement diagnostic", leafData) - } - info, err := os.Lstat(leaf) - if err != nil { - t.Fatalf("Lstat(leaf) error = %v", err) - } - if info.Mode()&os.ModeSymlink != 0 || !info.Mode().IsRegular() { - t.Fatalf("leaf mode = %v, want regular file", info.Mode()) - } -} - -func TestOpenFileConfinedRejectsLeafReplacedBeforeExistingFileOpen(t *testing.T) { - if runtime.GOOS == "windows" { - t.Skip("creating symlinks requires privileges that are not available on every Windows runner") - } - dir := t.TempDir() - target := filepath.Join(dir, "target.lock") - const original = "unrelated lock content" - if err := os.WriteFile(target, []byte(original), 0o600); err != nil { - t.Fatalf("WriteFile(target) error = %v", err) - } - leaf := filepath.Join(dir, "session.lock") - if err := os.WriteFile(leaf, []byte("old lock"), 0o600); err != nil { - t.Fatalf("WriteFile(leaf) error = %v", err) - } - - file, err := openFileConfined(leaf, os.O_RDWR|os.O_CREATE, WorkspaceFileMode, func() error { - if err := os.Remove(leaf); err != nil { - return err - } - return os.Symlink(filepath.Base(target), leaf) - }) - if file != nil { - _ = file.Close() - t.Fatal("openFileConfined() returned a raced symbolic-link target") - } - if err == nil || !strings.Contains(err.Error(), "changed while being opened") { - t.Fatalf("openFileConfined() error = %v, want identity-change rejection", err) - } - targetData, readErr := os.ReadFile(target) - if readErr != nil { - t.Fatalf("ReadFile(target) error = %v", readErr) - } - if string(targetData) != original { - t.Fatalf("target content = %q, want %q", targetData, original) - } -} diff --git a/internal/fileops/confined_read.go b/internal/fileops/confined_read.go deleted file mode 100644 index 7d08463..0000000 --- a/internal/fileops/confined_read.go +++ /dev/null @@ -1,78 +0,0 @@ -package fileops - -import ( - "fmt" - "os" - "path/filepath" - "strings" - - "gitea.maximumdirect.net/eric/narratio/internal/pathsafe" -) - -// OpenConfinedRegularFile opens a declared file beneath root without following -// symlinked ancestors or leaf entries. The returned descriptor remains valid if -// the pathname is later replaced. -func OpenConfinedRegularFile(rootPath, relativePath string) (*os.File, error) { - rootPath = filepath.Clean(strings.TrimSpace(rootPath)) - if rootPath == "" || rootPath == "." { - return nil, fmt.Errorf("source root is required") - } - relativePath, err := pathsafe.NormalizeRelativeDestination(relativePath) - if err != nil { - return nil, fmt.Errorf("source path: %w", err) - } - - root, err := openConfinedDirectory(rootPath) - if err != nil { - return nil, fmt.Errorf("open source root: %w", err) - } - defer root.Close() - - parts := strings.Split(filepath.FromSlash(relativePath), string(filepath.Separator)) - parent := root - for _, part := range parts[:len(parts)-1] { - child, err := openConfinedChild(parent, part, false, 0) - if err != nil { - if parent != root { - _ = parent.Close() - } - return nil, fmt.Errorf("open source ancestor %q: %w", part, err) - } - if parent != root { - _ = parent.Close() - } - parent = child - } - if parent != root { - defer parent.Close() - } - - name := parts[len(parts)-1] - declared, err := parent.Lstat(name) - if err != nil { - return nil, fmt.Errorf("inspect source %q: %w", relativePath, err) - } - if declared.Mode()&os.ModeSymlink != 0 || !declared.Mode().IsRegular() { - return nil, fmt.Errorf("source %q is not a regular file", relativePath) - } - - file, err := parent.Open(name) - if err != nil { - return nil, fmt.Errorf("open source %q: %w", relativePath, err) - } - opened, err := file.Stat() - if err != nil { - _ = file.Close() - return nil, fmt.Errorf("inspect opened source %q: %w", relativePath, err) - } - current, err := parent.Lstat(name) - if err != nil || current.Mode()&os.ModeSymlink != 0 || !current.Mode().IsRegular() || - !opened.Mode().IsRegular() || !os.SameFile(opened, declared) || !os.SameFile(opened, current) { - _ = file.Close() - if err != nil { - return nil, fmt.Errorf("reinspect source %q: %w", relativePath, err) - } - return nil, fmt.Errorf("source %q changed while being opened", relativePath) - } - return file, nil -} diff --git a/internal/fileops/read_confined.go b/internal/fileops/read_confined.go deleted file mode 100644 index 7ff3e35..0000000 --- a/internal/fileops/read_confined.go +++ /dev/null @@ -1,121 +0,0 @@ -package fileops - -import ( - "fmt" - "io" - "os" - "path/filepath" -) - -// ValidateConfinedDirectory opens path without following symbolic links and -// applies validate to the opened directory's metadata. -func ValidateConfinedDirectory(path string, validate func(os.FileInfo) error) error { - root, err := openConfinedDirectory(path) - if err != nil { - return err - } - defer func() { _ = root.Close() }() - info, err := root.Stat(".") - if err != nil { - return fmt.Errorf("inspect root directory: %w", err) - } - if !info.IsDir() { - return fmt.Errorf("root path is not a directory") - } - if validate != nil { - return validate(info) - } - return nil -} - -// ReadRegularFileUnderRoot reads a bounded regular file after opening root and -// every relative-path ancestor without following symbolic links. validateRoot -// and validateFile may enforce caller-owned access policy while their handles -// are still verified. -func ReadRegularFileUnderRoot( - rootPath, relativePath string, - maxBytes int64, - validateRoot, validateFile func(os.FileInfo) error, -) ([]byte, error) { - if maxBytes < 0 { - return nil, fmt.Errorf("maximum byte count must not be negative") - } - if filepath.IsAbs(relativePath) { - return nil, fmt.Errorf("relative file path must not be absolute") - } - parts, err := relativePathParts(relativePath) - if err != nil { - return nil, fmt.Errorf("invalid relative file path: %w", err) - } - root, err := openConfinedDirectory(rootPath) - if err != nil { - return nil, err - } - defer func() { _ = root.Close() }() - rootInfo, err := root.Stat(".") - if err != nil { - return nil, fmt.Errorf("inspect root directory: %w", err) - } - if validateRoot != nil { - if err := validateRoot(rootInfo); err != nil { - return nil, err - } - } - for _, part := range parts[:len(parts)-1] { - child, err := openConfinedChild(root, part, false, 0) - if err != nil { - return nil, err - } - _ = root.Close() - root = child - } - - name := parts[len(parts)-1] - inspected, err := root.Lstat(name) - if err != nil { - return nil, fmt.Errorf("inspect file %q: %w", name, err) - } - if inspected.Mode()&os.ModeSymlink != 0 || !inspected.Mode().IsRegular() { - return nil, fmt.Errorf("file %q is not a regular file", name) - } - file, err := root.Open(name) - if err != nil { - return nil, fmt.Errorf("open file %q: %w", name, err) - } - defer func() { _ = file.Close() }() - opened, err := file.Stat() - if err != nil { - return nil, fmt.Errorf("inspect opened file %q: %w", name, err) - } - current, err := root.Lstat(name) - if err != nil || current.Mode()&os.ModeSymlink != 0 || !current.Mode().IsRegular() || !os.SameFile(opened, current) { - if err != nil { - return nil, fmt.Errorf("reinspect file %q: %w", name, err) - } - return nil, fmt.Errorf("file %q changed while being opened", name) - } - if validateFile != nil { - if err := validateFile(opened); err != nil { - return nil, err - } - } - content, err := io.ReadAll(io.LimitReader(file, maxBytes+1)) - if err != nil { - return nil, fmt.Errorf("read file %q: %w", name, err) - } - if int64(len(content)) > maxBytes { - return nil, fmt.Errorf("file %q exceeds %d-byte limit", name, maxBytes) - } - return content, nil -} - -// ReadRegularFile reads a bounded regular file without following symbolic links -// in its parent hierarchy. It is intended for externally produced results; -// callers own the limit and semantic validation contract. -func ReadRegularFile(path string, maxBytes int64) ([]byte, error) { - clean := filepath.Clean(path) - if clean == "." || filepath.Base(clean) == "." { - return nil, fmt.Errorf("file path is required") - } - return ReadRegularFileUnderRoot(filepath.Dir(clean), filepath.Base(clean), maxBytes, nil, nil) -} diff --git a/internal/fileops/read_confined_test.go b/internal/fileops/read_confined_test.go deleted file mode 100644 index 0a59b32..0000000 --- a/internal/fileops/read_confined_test.go +++ /dev/null @@ -1,65 +0,0 @@ -package fileops - -import ( - "os" - "path/filepath" - "runtime" - "strings" - "testing" -) - -func TestReadRegularFileEnforcesLimitAndRegularFileIdentity(t *testing.T) { - root := t.TempDir() - path := filepath.Join(root, "result.json") - if err := os.WriteFile(path, []byte("1234"), 0o600); err != nil { - t.Fatalf("WriteFile() error = %v", err) - } - data, err := ReadRegularFile(path, 4) - if err != nil { - t.Fatalf("ReadRegularFile(exact limit) error = %v", err) - } - if string(data) != "1234" { - t.Fatalf("data = %q, want exact file content", data) - } - if _, err := ReadRegularFile(path, 3); err == nil || !strings.Contains(err.Error(), "3-byte limit") { - t.Fatalf("ReadRegularFile(limit plus one) error = %v, want limit failure", err) - } - if err := os.Remove(path); err != nil { - t.Fatalf("Remove(file) error = %v", err) - } - if err := os.Mkdir(path, 0o700); err != nil { - t.Fatalf("Mkdir(non-regular file) error = %v", err) - } - if _, err := ReadRegularFile(path, 4); err == nil || !strings.Contains(err.Error(), "not a regular file") { - t.Fatalf("ReadRegularFile(directory) error = %v, want regular-file failure", err) - } -} - -func TestReadRegularFileRejectsSymlinkedPaths(t *testing.T) { - if runtime.GOOS == "windows" { - t.Skip("symlink behavior differs on windows") - } - root := t.TempDir() - outside := t.TempDir() - outsideFile := filepath.Join(outside, "result.json") - if err := os.WriteFile(outsideFile, []byte("outside"), 0o600); err != nil { - t.Fatalf("WriteFile(outside) error = %v", err) - } - leaf := filepath.Join(root, "result.json") - if err := os.Symlink(outsideFile, leaf); err != nil { - t.Fatalf("Symlink(leaf) error = %v", err) - } - if _, err := ReadRegularFile(leaf, 64); err == nil { - t.Fatal("ReadRegularFile(leaf symlink) error = nil, want rejection") - } - if err := os.Remove(leaf); err != nil { - t.Fatalf("Remove(leaf symlink) error = %v", err) - } - linkedParent := filepath.Join(root, "linked") - if err := os.Symlink(outside, linkedParent); err != nil { - t.Fatalf("Symlink(parent) error = %v", err) - } - if _, err := ReadRegularFile(filepath.Join(linkedParent, "result.json"), 64); err == nil { - t.Fatal("ReadRegularFile(symlinked parent) error = nil, want rejection") - } -} diff --git a/internal/stage/analyze_failure_test.go b/internal/stage/analyze_failure_test.go index d03e8ce..0beff61 100644 --- a/internal/stage/analyze_failure_test.go +++ b/internal/stage/analyze_failure_test.go @@ -166,11 +166,7 @@ func (invalidAnalyzeOutputRunner) RunArtifact(_ context.Context, req scriptorium if err := os.MkdirAll(filepath.Dir(req.OutputPath), 0o755); err != nil { return scriptorium.ArtifactResult{}, err } - target := req.OutputPath + ".target" - if err := os.WriteFile(target, []byte("unsafe\n"), 0o644); err != nil { - return scriptorium.ArtifactResult{}, err - } - if err := os.Symlink(target, req.OutputPath); err != nil { + if err := os.Mkdir(req.OutputPath, 0o755); err != nil { return scriptorium.ArtifactResult{}, err } return scriptorium.ArtifactResult{OutputPath: req.OutputPath, CommandMode: scriptorium.CommandModeRun}, nil diff --git a/internal/stage/analyze_input_identity.go b/internal/stage/analyze_input_identity.go index d377dbd..0ba5ee7 100644 --- a/internal/stage/analyze_input_identity.go +++ b/internal/stage/analyze_input_identity.go @@ -6,6 +6,7 @@ import ( "errors" "fmt" "io" + "os" "path/filepath" "strings" @@ -13,7 +14,6 @@ import ( "gitea.maximumdirect.net/eric/narratio/internal/artifactpolicy" "gitea.maximumdirect.net/eric/narratio/internal/artifacts" "gitea.maximumdirect.net/eric/narratio/internal/config" - "gitea.maximumdirect.net/eric/narratio/internal/fileops" "gitea.maximumdirect.net/eric/narratio/internal/pathsafe" ) @@ -263,7 +263,7 @@ func hashAnalyzeInputFile(paths artifacts.SessionPaths, path string) (string, in if err != nil { return "", 0, fmt.Errorf("input path is outside session root: %w", err) } - file, err := fileops.OpenConfinedRegularFile(root, relative) + file, err := os.Open(target) if err != nil { return "", 0, err } @@ -272,6 +272,10 @@ func hashAnalyzeInputFile(paths artifacts.SessionPaths, path string) (string, in _ = file.Close() return "", 0, fmt.Errorf("inspect input: %w", err) } + if !info.Mode().IsRegular() { + _ = file.Close() + return "", 0, fmt.Errorf("input is not a regular file") + } digest := sha256.New() size, readErr := io.Copy(digest, io.LimitReader(file, artifacts.MaxResolvedArtifactBytes+1)) closeErr := file.Close() diff --git a/internal/stage/analyze_input_identity_test.go b/internal/stage/analyze_input_identity_test.go index 89d7d39..6615ddc 100644 --- a/internal/stage/analyze_input_identity_test.go +++ b/internal/stage/analyze_input_identity_test.go @@ -182,21 +182,6 @@ func TestResolveAnalyzeInputIdentitiesRejectsUnsafePathsAndFileTypes(t *testing. } }, }, - { - name: "symlink", sourceID: "narratio.transcript.final_trimmed", - setup: func(t *testing.T, env *Env, m *manifest.Manifest) { - t.Helper() - path := filepath.Join(sessionPathsForEnv(env, m.SessionID).TranscriptsDir, "final.trimmed.json") - outside := filepath.Join(t.TempDir(), "outside.json") - writeAnalyzeFile(t, outside, `{"segments":[]}`) - if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil { - t.Fatal(err) - } - if err := os.Symlink(outside, path); err != nil { - t.Fatal(err) - } - }, - }, { name: "manifest path outside session", sourceID: artifactpolicy.SourceInputPlayers, setup: func(t *testing.T, _ *Env, m *manifest.Manifest) { diff --git a/internal/stage/external_result.go b/internal/stage/external_result.go index 3af4245..871da22 100644 --- a/internal/stage/external_result.go +++ b/internal/stage/external_result.go @@ -11,7 +11,7 @@ import ( const MaxExternalResultFileBytes int64 = 64 * 1024 * 1024 func readExternalResult(path, category string) ([]byte, error) { - data, err := fileops.ReadRegularFile(path, MaxExternalResultFileBytes) + data, err := fileops.ReadFileBounded(path, MaxExternalResultFileBytes) if err != nil { return nil, fmt.Errorf("%s exceeds or cannot be read within %d-byte limit: %w", category, MaxExternalResultFileBytes, err) } diff --git a/internal/stage/extract.go b/internal/stage/extract.go index 125c360..fff26ee 100644 --- a/internal/stage/extract.go +++ b/internal/stage/extract.go @@ -373,7 +373,7 @@ func checksumRegularFile(path string, requireJSON bool) (string, error) { if info.Size() == 0 { return "", fmt.Errorf("path %q must be non-empty", path) } - data, err := fileops.ReadRegularFile(path, artifacts.MaxExtractionPayloadBytes) + data, err := fileops.ReadFileBounded(path, artifacts.MaxExtractionPayloadBytes) if err != nil { return "", fmt.Errorf("notarius lane result exceeds or cannot be read within %d-byte limit: %w", artifacts.MaxExtractionPayloadBytes, err) } diff --git a/internal/stage/extract_references.go b/internal/stage/extract_references.go index 6e32b68..cce89f0 100644 --- a/internal/stage/extract_references.go +++ b/internal/stage/extract_references.go @@ -51,7 +51,7 @@ func materializeExtractReferenceSnapshots( } destinationOwners[destination] = identity.SourceID - file, err := fileops.OpenConfinedRegularFile(paths.Root, identity.Path) + file, err := os.Open(filepath.Join(paths.Root, filepath.FromSlash(identity.Path))) if err != nil { return nil, fmt.Errorf("open reference %q source %q for snapshot: %w", identity.Selector, identity.SourceID, err) } @@ -84,7 +84,7 @@ func verifyExtractReferenceSnapshots(snapshotRoot string, references resolvedExt continue } filename := filepath.Base(filepath.FromSlash(identity.Path)) - file, err := fileops.OpenConfinedRegularFile(snapshotRoot, filename) + file, err := os.Open(filepath.Join(snapshotRoot, filename)) if err != nil { return fmt.Errorf("reopen reference %q source %q snapshot: %w", identity.Selector, identity.SourceID, err) } diff --git a/internal/stage/publish.go b/internal/stage/publish.go index 8f554f3..2f06a29 100644 --- a/internal/stage/publish.go +++ b/internal/stage/publish.go @@ -18,7 +18,6 @@ import ( "gitea.maximumdirect.net/eric/narratio/internal/artifactpolicy" "gitea.maximumdirect.net/eric/narratio/internal/artifacts" "gitea.maximumdirect.net/eric/narratio/internal/config" - "gitea.maximumdirect.net/eric/narratio/internal/fileops" "gitea.maximumdirect.net/eric/narratio/internal/manifest" "gitea.maximumdirect.net/eric/narratio/internal/pathsafe" ) @@ -106,7 +105,7 @@ func (publishStage) Run(ctx context.Context, env *Env, m *manifest.Manifest) (*S return nil, fmt.Errorf("publish: run id is required") } - runManifestFile, err := fileops.OpenConfinedRegularFile(runRoot, filepath.Base(runManifestPath)) + runManifestFile, err := openPublishOutputSource(filepath.Join(runRoot, filepath.Base(runManifestPath))) if err != nil { return nil, fmt.Errorf("publish: open run manifest: %w", err) } @@ -420,7 +419,14 @@ func openPublishOutputSource(localPath string) (*os.File, error) { if path == "" || path == "." { return nil, fmt.Errorf("local path is required") } - return fileops.OpenConfinedRegularFile(filepath.Dir(path), filepath.Base(path)) + info, err := os.Lstat(path) + if err != nil { + return nil, err + } + if !info.Mode().IsRegular() { + return nil, fmt.Errorf("source is not a regular file") + } + return os.Open(path) } func publishOutputRemoteKey(runPrefix, destination string) string { @@ -913,7 +919,7 @@ func collectPublishPreviousFiles(ctx context.Context, previousDir string) ([]pub return nil, nil, fmt.Errorf("previous directory is not a regular directory") } - manifestFile, err := fileops.OpenConfinedRegularFile(previousDir, "manifest.json") + manifestFile, err := openPublishOutputSource(filepath.Join(previousDir, "manifest.json")) if err != nil { if errors.Is(err, os.ErrNotExist) { return nil, nil, nil @@ -1005,7 +1011,7 @@ func openPublishRunFiles(runRoot string, files []publishUploadFile, manifestFile file := manifestFile if candidate.SourceRelativePath != "manifest.json" { var err error - file, err = fileops.OpenConfinedRegularFile(runRoot, candidate.SourceRelativePath) + file, err = openPublishOutputSource(filepath.Join(runRoot, filepath.FromSlash(candidate.SourceRelativePath))) if err != nil { closePublishSources(sources) return nil, err diff --git a/internal/stage/publish_test.go b/internal/stage/publish_test.go index 45feeb3..cce8fc4 100644 --- a/internal/stage/publish_test.go +++ b/internal/stage/publish_test.go @@ -17,7 +17,6 @@ import ( "gitea.maximumdirect.net/eric/narratio/internal/artifactmodel" "gitea.maximumdirect.net/eric/narratio/internal/artifacts" "gitea.maximumdirect.net/eric/narratio/internal/config" - "gitea.maximumdirect.net/eric/narratio/internal/fileops" "gitea.maximumdirect.net/eric/narratio/internal/manifest" ) @@ -239,7 +238,7 @@ func TestPublishRejectsDeclaredLeafSymlinkBeforeUploading(t *testing.T) { } _, err := (publishStage{}).Run(context.Background(), env, m) - if err == nil || !strings.Contains(err.Error(), `source "logs/audita.stderr.log" is not a regular file`) { + if err == nil || !strings.Contains(err.Error(), "not a regular file") { t.Fatalf("Run() error = %v, want rejected symlink", err) } assertNoPublishUploads(t, env.ObjectStore.(*storage.FakeBackend), "outside secret") @@ -261,13 +260,13 @@ func TestPublishRejectsSymlinkedRunManifestBeforeUploading(t *testing.T) { } _, err := (publishStage{}).Run(context.Background(), env, m) - if err == nil || !strings.Contains(err.Error(), `source "manifest.json" is not a regular file`) { + if err == nil || !strings.Contains(err.Error(), "not a regular file") { t.Fatalf("Run() error = %v, want rejected run manifest symlink", err) } assertNoPublishUploads(t, env.ObjectStore.(*storage.FakeBackend), "outside") } -func TestPublishRejectsSymlinkedSourceAncestorBeforeUploading(t *testing.T) { +func TestPublishUsesOrdinarySourceAncestorPathSemantics(t *testing.T) { if runtime.GOOS == "windows" { t.Skip("symlink creation requires privileges on Windows") } @@ -282,11 +281,9 @@ func TestPublishRejectsSymlinkedSourceAncestorBeforeUploading(t *testing.T) { t.Fatal(err) } - _, err := (publishStage{}).Run(context.Background(), env, m) - if err == nil || !strings.Contains(err.Error(), "source ancestor") { - t.Fatalf("Run() error = %v, want rejected symlinked ancestor", err) + if _, err := (publishStage{}).Run(context.Background(), env, m); err != nil { + t.Fatalf("Run() error = %v", err) } - assertNoPublishUploads(t, env.ObjectStore.(*storage.FakeBackend), "outside secret") } func TestPublishRejectsDeclaredSourceReplacement(t *testing.T) { @@ -304,7 +301,7 @@ func TestPublishRejectsDeclaredSourceReplacement(t *testing.T) { t.Fatal(err) } writeStageTestFile(t, target, "replacement\n") - manifestFile, err := fileops.OpenConfinedRegularFile(runRoot, "manifest.json") + manifestFile, err := os.Open(filepath.Join(runRoot, "manifest.json")) if err != nil { t.Fatal(err) } @@ -327,7 +324,7 @@ func TestPublishRejectsDeclaredDirectoryBeforeUploading(t *testing.T) { } _, err := (publishStage{}).Run(context.Background(), env, m) - if err == nil || !strings.Contains(err.Error(), `source "logs/audita.stderr.log" is not a regular file`) { + if err == nil || !strings.Contains(err.Error(), "not a regular file") { t.Fatalf("Run() error = %v, want rejected directory", err) } assertNoPublishUploads(t, env.ObjectStore.(*storage.FakeBackend), "") @@ -377,7 +374,7 @@ func TestPublishRejectsDeclaredPreviousSymlinkBeforeUploading(t *testing.T) { } _, err := (publishStage{}).Run(context.Background(), env, m) - if err == nil || !strings.Contains(err.Error(), `source "artifacts/session_recap.md" is not a regular file`) { + if err == nil || !strings.Contains(err.Error(), "not a regular file") { t.Fatalf("Run() error = %v, want rejected previous symlink", err) } assertNoPublishUploads(t, env.ObjectStore.(*storage.FakeBackend), "outside previous secret")