From 99b2e1cd81bca2bf6ecbd26e7012ed8d61824b04 Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Mon, 10 Aug 2026 18:24:29 +0000 Subject: [PATCH] Harden API key file loading --- docs/config.md | 8 +- docs/internal/fileops.md | 8 ++ docs/operations.md | 3 + docs/roadmap/implementation.md | 2 +- internal/app/commands_test.go | 2 +- internal/app/operator_helpers_test.go | 8 +- internal/app/remote_session_test.go | 4 +- internal/app/restore_test.go | 4 +- internal/app/secrets_env.go | 63 ++++++++--- internal/app/secrets_env_posix.go | 22 ++++ internal/app/secrets_env_test.go | 157 +++++++++++++++++++++++++- internal/app/secrets_env_windows.go | 11 ++ internal/fileops/read_confined.go | 110 ++++++++++++++++++ 13 files changed, 370 insertions(+), 32 deletions(-) create mode 100644 internal/app/secrets_env_posix.go create mode 100644 internal/app/secrets_env_windows.go create mode 100644 internal/fileops/read_confined.go diff --git a/docs/config.md b/docs/config.md index dc4a398..86fde4f 100644 --- a/docs/config.md +++ b/docs/config.md @@ -94,7 +94,13 @@ inputs: - Do not place raw secrets in YAML. - Use env var names in config (for example `pipeline.audita.llm_api_key_env`). -- Optionally load env files from `pipeline.secrets.env_dir`. +- Optionally load credential files from `pipeline.secrets.env_dir`. Each valid + environment-variable filename supplies one value; trailing CR/LF is removed. +- An existing process environment value takes precedence over a credential file. +- Credential directories and files must not be symlinks and must be regular, + bounded files (at most 8 KiB per value). On POSIX, provision the directory + with no group/other access (normally `0700`) and files with no group/other + access (normally `0600`). - Commands that need storage/auth load filesystem secrets before constructing adapters. ## Publish Configuration Summary diff --git a/docs/internal/fileops.md b/docs/internal/fileops.md index 07f2c8c..a201059 100644 --- a/docs/internal/fileops.md +++ b/docs/internal/fileops.md @@ -31,6 +31,14 @@ tree through those directory handles. It rejects root deletion and any symlink encountered in the target path or tree; repeated removal of a missing target is successful. Command and post-publish policy remains owned by `internal/app`. +## Confined Reads + +`ReadRegularFileUnderRoot` is the no-follow, bounded read primitive for a +caller-selected root and relative file path. It verifies the root and every +ancestor through directory handles, admits only a stable regular-file handle, +and lets the caller enforce its own byte limit and access policy. Credential +mode policy and environment precedence remain owned by `internal/app`. + ## Replacement Contract `ReplaceFileAtomic` requires an existing destination directory. It creates a diff --git a/docs/operations.md b/docs/operations.md index 6a7daf2..e75b922 100644 --- a/docs/operations.md +++ b/docs/operations.md @@ -303,6 +303,9 @@ Windows access control. API keys are credentials, not ordinary workspace data. Store them outside the shared workspace or in a separately restricted credential location; ordinary workspace group access must never be treated as authorization to read keys. +On POSIX, provision a credential directory as `0700` and credential files as +`0600`; Narratio rejects group- or other-readable configured credential paths. +On Windows, restrict the directory and files with ACLs to the credential owner. ## Cleanup diff --git a/docs/roadmap/implementation.md b/docs/roadmap/implementation.md index 94b7e08..030a5d9 100644 --- a/docs/roadmap/implementation.md +++ b/docs/roadmap/implementation.md @@ -21,7 +21,7 @@ All stages are pending when this plan is created. | 3 | Consolidate crash-durable atomic file replacement | RSK-002, DUP-001, DUP-005 | Completed | | 4 | Add confined destination and download/install capabilities | COR-003, DUP-003, TST-003 | Completed | | 5 | Confine recursive cleanup and replace sentinel locks | RSK-003 | Completed | -| 6 | Harden API-key file acquisition | RSK-010 | Pending | +| 6 | Harden API-key file acquisition | RSK-010 | Completed | | 7 | Bound and verify external result acquisition | RSK-013, TST-007 | Pending | | 8 | Terminate owned subprocess trees | RSK-011 | Pending | | 9 | Redact and cap subprocess diagnostics | RSK-012 | Pending | diff --git a/internal/app/commands_test.go b/internal/app/commands_test.go index ec4398a..a1da021 100644 --- a/internal/app/commands_test.go +++ b/internal/app/commands_test.go @@ -189,7 +189,7 @@ func TestExecuteRunStagePolishLoadsCredentialFromSecretsDir(t *testing.T) { configDir := t.TempDir() sessionID := "2026-05-03" secretsDir := filepath.Join(configDir, "secrets") - if err := os.MkdirAll(secretsDir, 0o755); err != nil { + if err := os.MkdirAll(secretsDir, secretDirectoryPrivateMode); err != nil { t.Fatalf("MkdirAll(%q): %v", secretsDir, err) } if err := os.WriteFile(filepath.Join(secretsDir, "OPENROUTER_API_KEY"), []byte("from-secret-file\n"), 0o600); err != nil { diff --git a/internal/app/operator_helpers_test.go b/internal/app/operator_helpers_test.go index f77c636..41be95c 100644 --- a/internal/app/operator_helpers_test.go +++ b/internal/app/operator_helpers_test.go @@ -189,8 +189,8 @@ func TestExecuteSessionInitRemoteLoadsSecretsBeforeObjectStoreInit(t *testing.T) secretKeyEnv := "NARRATIO_TEST_SESSION_INIT_OBJECT_SECRET" restoreEnvAfterTest(t, accessKeyEnv, secretKeyEnv) secretsDir := t.TempDir() - mustWriteTestFile(t, filepath.Join(secretsDir, accessKeyEnv), "test-key-id\n") - mustWriteTestFile(t, filepath.Join(secretsDir, secretKeyEnv), "test-secret\n") + mustWriteSecretFile(t, filepath.Join(secretsDir, accessKeyEnv), "test-key-id\n") + mustWriteSecretFile(t, filepath.Join(secretsDir, secretKeyEnv), "test-secret\n") addSecretsToPipelineConfig(t, pipelinePath, secretsDir, accessKeyEnv, secretKeyEnv) fake := &storage.FakeBackend{} @@ -417,8 +417,8 @@ func TestExecuteSessionValidateLoadsSecretsBeforeObjectStoreInit(t *testing.T) { secretKeyEnv := "NARRATIO_TEST_VALIDATE_OBJECT_SECRET" restoreEnvAfterTest(t, accessKeyEnv, secretKeyEnv) secretsDir := t.TempDir() - mustWriteTestFile(t, filepath.Join(secretsDir, accessKeyEnv), "test-key-id\n") - mustWriteTestFile(t, filepath.Join(secretsDir, secretKeyEnv), "test-secret\n") + mustWriteSecretFile(t, filepath.Join(secretsDir, accessKeyEnv), "test-key-id\n") + mustWriteSecretFile(t, filepath.Join(secretsDir, secretKeyEnv), "test-secret\n") addSecretsToPipelineConfig(t, pipelinePath, secretsDir, accessKeyEnv, secretKeyEnv) if err := os.WriteFile(sessionPath, []byte(`session_id: 2026-05-03 inputs: diff --git a/internal/app/remote_session_test.go b/internal/app/remote_session_test.go index 0d6909b..89e6bbc 100644 --- a/internal/app/remote_session_test.go +++ b/internal/app/remote_session_test.go @@ -51,8 +51,8 @@ func TestExecuteRemoteSessionFallbackLoadsSecretsBeforeObjectStoreInit(t *testin secretKeyEnv := "NARRATIO_TEST_REMOTE_SESSION_SECRET" restoreEnvAfterTest(t, accessKeyEnv, secretKeyEnv) secretsDir := t.TempDir() - mustWriteTestFile(t, filepath.Join(secretsDir, accessKeyEnv), "remote-session-key-id\n") - mustWriteTestFile(t, filepath.Join(secretsDir, secretKeyEnv), "remote-session-secret\n") + mustWriteSecretFile(t, filepath.Join(secretsDir, accessKeyEnv), "remote-session-key-id\n") + mustWriteSecretFile(t, filepath.Join(secretsDir, secretKeyEnv), "remote-session-secret\n") addSecretsToPipelineConfig(t, pipelinePath, secretsDir, accessKeyEnv, secretKeyEnv) fake := &storage.FakeBackend{} diff --git a/internal/app/restore_test.go b/internal/app/restore_test.go index 6ba01fd..7d95cfa 100644 --- a/internal/app/restore_test.go +++ b/internal/app/restore_test.go @@ -213,8 +213,8 @@ func TestExecuteRestoreLoadsSecretsBeforeObjectStoreInit(t *testing.T) { workspaceRoot := t.TempDir() pipelinePath, campaignPath, sessionPath := writeValidConfigFiles(t, workspaceRoot) secretsDir := filepath.Join(t.TempDir(), "secrets") - mustWriteTestFile(t, filepath.Join(secretsDir, accessKeyEnv), "test-access-key-id\n") - mustWriteTestFile(t, filepath.Join(secretsDir, secretKeyEnv), "test-secret-key\n") + mustWriteSecretFile(t, filepath.Join(secretsDir, accessKeyEnv), "test-access-key-id\n") + mustWriteSecretFile(t, filepath.Join(secretsDir, secretKeyEnv), "test-secret-key\n") f, err := os.OpenFile(pipelinePath, os.O_APPEND|os.O_WRONLY, 0) if err != nil { t.Fatalf("open pipeline config for append: %v", err) diff --git a/internal/app/secrets_env.go b/internal/app/secrets_env.go index de2c7a3..7338e01 100644 --- a/internal/app/secrets_env.go +++ b/internal/app/secrets_env.go @@ -9,9 +9,18 @@ import ( "strings" "gitea.maximumdirect.net/eric/narratio/internal/config" + "gitea.maximumdirect.net/eric/narratio/internal/fileops" ) var envVarNamePattern = regexp.MustCompile(`^[A-Za-z_][A-Za-z0-9_]*$`) +var readSecretsDirectoryEntries = os.ReadDir + +// APIKeyFileByteLimit bounds one credential value read from the configured +// secrets directory. API keys are expected to be short single-line values. +const APIKeyFileByteLimit int64 = 8 * 1024 + +const secretDirectoryPrivateMode os.FileMode = 0o700 +const secretFilePrivateMode os.FileMode = 0o600 type secretsLoadStats struct { Dir string @@ -40,9 +49,13 @@ func loadSecretsFromConfig(cfg *config.Config, logger *slog.Logger) (*secretsLoa } resolvedDir = filepath.Clean(resolvedDir) - entries, err := os.ReadDir(resolvedDir) + if err := fileops.ValidateConfinedDirectory(resolvedDir, validateSecretDirectory); err != nil { + return nil, fmt.Errorf("API-key loader: validate secrets env_dir: %w", err) + } + + entries, err := readSecretsDirectoryEntries(resolvedDir) if err != nil { - return nil, fmt.Errorf("read secrets env_dir %q: %w", resolvedDir, err) + return nil, fmt.Errorf("API-key loader: read secrets env_dir %q: %w", resolvedDir, err) } stats := &secretsLoadStats{Dir: resolvedDir} @@ -52,24 +65,16 @@ func loadSecretsFromConfig(cfg *config.Config, logger *slog.Logger) (*secretsLoa stats.Skipped++ continue } - if entry.IsDir() { - stats.Skipped++ - continue - } - - secretPath := filepath.Join(resolvedDir, name) - bytes, err := os.ReadFile(secretPath) - if err != nil { - return nil, fmt.Errorf("read secret file %q: %w", secretPath, err) - } - value := strings.TrimRight(string(bytes), "\r\n") - if _, exists := os.LookupEnv(name); exists { stats.PreservedExisting++ continue } + value, err := readAPIKeyFile(resolvedDir, name) + if err != nil { + return nil, err + } if err := os.Setenv(name, value); err != nil { - return nil, fmt.Errorf("set environment variable %q from %q: %w", name, secretPath, err) + return nil, fmt.Errorf("API-key loader: set environment variable %q: %w", name, err) } stats.Loaded++ } @@ -86,3 +91,31 @@ func loadSecretsFromConfig(cfg *config.Config, logger *slog.Logger) (*secretsLoa return stats, nil } + +func validateSecretDirectory(info os.FileInfo) error { + if !info.IsDir() { + return fmt.Errorf("secrets env_dir is not a directory") + } + return validateSecretDirectoryPrivacy(info) +} + +func validateSecretFile(info os.FileInfo) error { + if !info.Mode().IsRegular() { + return fmt.Errorf("secret entry is not a regular file") + } + return validateSecretFilePrivacy(info) +} + +func readAPIKeyFile(directory, name string) (string, error) { + content, err := fileops.ReadRegularFileUnderRoot( + directory, + name, + APIKeyFileByteLimit, + validateSecretDirectory, + validateSecretFile, + ) + 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 +} diff --git a/internal/app/secrets_env_posix.go b/internal/app/secrets_env_posix.go new file mode 100644 index 0000000..2ca971b --- /dev/null +++ b/internal/app/secrets_env_posix.go @@ -0,0 +1,22 @@ +//go:build !windows + +package app + +import ( + "fmt" + "os" +) + +func validateSecretDirectoryPrivacy(info os.FileInfo) error { + if info.Mode().Perm()&^secretDirectoryPrivateMode != 0 { + return fmt.Errorf("secret directory mode %04o grants group or other access; require %04o", info.Mode().Perm(), secretDirectoryPrivateMode) + } + return nil +} + +func validateSecretFilePrivacy(info os.FileInfo) error { + if info.Mode().Perm()&^secretFilePrivateMode != 0 { + return fmt.Errorf("secret file mode %04o grants group or other access; require %04o", info.Mode().Perm(), secretFilePrivateMode) + } + return nil +} diff --git a/internal/app/secrets_env_test.go b/internal/app/secrets_env_test.go index 2821b3b..d6f0d96 100644 --- a/internal/app/secrets_env_test.go +++ b/internal/app/secrets_env_test.go @@ -12,6 +12,8 @@ import ( func TestLoadSecretsFromConfigLoadsValidFiles(t *testing.T) { dir := t.TempDir() + unsetSecretEnvironment(t, "NARRATIO_TEST_SECRET_A") + unsetSecretEnvironment(t, "NARRATIO_TEST_SECRET_B") mustWriteSecretFile(t, filepath.Join(dir, "NARRATIO_TEST_SECRET_A"), "value-1\n") mustWriteSecretFile(t, filepath.Join(dir, "NARRATIO_TEST_SECRET_B"), "value-2\r\n") mustWriteSecretFile(t, filepath.Join(dir, "not-valid-name.txt"), "ignored") @@ -73,7 +75,7 @@ func TestLoadSecretsFromConfigPreservesExistingEnv(t *testing.T) { func TestLoadSecretsFromConfigRelativeDirUsesCWD(t *testing.T) { cwd := t.TempDir() secretsDir := filepath.Join(cwd, "secrets") - if err := os.MkdirAll(secretsDir, 0o755); err != nil { + if err := os.MkdirAll(secretsDir, secretDirectoryPrivateMode); err != nil { t.Fatalf("MkdirAll(%q): %v", secretsDir, err) } mustWriteSecretFile(t, filepath.Join(secretsDir, "OBJECT_STORAGE_KEY_ID"), "id-123\n") @@ -113,8 +115,8 @@ func TestLoadSecretsFromConfigMissingDirFails(t *testing.T) { if err == nil { t.Fatal("expected error, got nil") } - if !strings.Contains(err.Error(), "read secrets env_dir") { - t.Fatalf("error = %q, want read-dir context", err.Error()) + if !strings.Contains(err.Error(), "API-key loader") { + t.Fatalf("error = %q, want API-key loader context", err.Error()) } } @@ -124,6 +126,7 @@ func TestLoadSecretsFromConfigUnreadableValidEntryFails(t *testing.T) { } dir := t.TempDir() + ensureSecretDirectory(t, dir) broken := filepath.Join(dir, "OPENROUTER_API_KEY") if err := os.Symlink(filepath.Join(dir, "does-not-exist"), broken); err != nil { t.Fatalf("Symlink(%q): %v", broken, err) @@ -139,17 +142,159 @@ func TestLoadSecretsFromConfigUnreadableValidEntryFails(t *testing.T) { if err == nil { t.Fatal("expected error, got nil") } - if !strings.Contains(err.Error(), "read secret file") { - t.Fatalf("error = %q, want read secret file context", err.Error()) + if !strings.Contains(err.Error(), "API-key loader") { + t.Fatalf("error = %q, want API-key loader context", err.Error()) + } +} + +func TestLoadSecretsFromConfigRejectsUnsafeModes(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("Windows access control is verified with ACLs") + } + for _, tc := range []struct { + name string + directory os.FileMode + file os.FileMode + wantSecret string + }{ + {name: "directory", directory: 0o750, file: secretFilePrivateMode, wantSecret: "directory-secret"}, + {name: "file", directory: secretDirectoryPrivateMode, file: 0o640, wantSecret: "file-secret"}, + } { + t.Run(tc.name, func(t *testing.T) { + dir := t.TempDir() + unsetSecretEnvironment(t, "OPENROUTER_API_KEY") + path := filepath.Join(dir, "OPENROUTER_API_KEY") + mustWriteSecretFile(t, path, tc.wantSecret) + if err := os.Chmod(dir, tc.directory); err != nil { + t.Fatalf("Chmod(directory) error = %v", err) + } + if err := os.Chmod(path, tc.file); err != nil { + t.Fatalf("Chmod(file) error = %v", err) + } + + _, err := loadSecretsFromConfig(secretConfig(dir), nil) + if err == nil || !strings.Contains(err.Error(), "API-key loader") { + t.Fatalf("loadSecretsFromConfig() error = %v, want API-key loader failure", err) + } + if strings.Contains(err.Error(), tc.wantSecret) { + t.Fatalf("error leaked secret content: %q", err) + } + }) + } +} + +func TestLoadSecretsFromConfigRejectsNonRegularAndOversizedEntries(t *testing.T) { + dir := t.TempDir() + ensureSecretDirectory(t, dir) + unsetSecretEnvironment(t, "OPENROUTER_API_KEY") + if err := os.Mkdir(filepath.Join(dir, "OPENROUTER_API_KEY"), secretDirectoryPrivateMode); err != nil { + t.Fatalf("Mkdir(non-regular entry) error = %v", err) + } + if _, err := loadSecretsFromConfig(secretConfig(dir), nil); err == nil || !strings.Contains(err.Error(), "not a regular file") { + t.Fatalf("loadSecretsFromConfig(non-regular) error = %v, want regular-file failure", err) + } + + if err := os.Remove(filepath.Join(dir, "OPENROUTER_API_KEY")); err != nil { + t.Fatalf("Remove(non-regular entry) error = %v", err) + } + overLimit := strings.Repeat("credential-value-", int(APIKeyFileByteLimit/17)+1) + mustWriteSecretFile(t, filepath.Join(dir, "OPENROUTER_API_KEY"), overLimit) + _, err := loadSecretsFromConfig(secretConfig(dir), nil) + if err == nil || !strings.Contains(err.Error(), "API-key loader") || !strings.Contains(err.Error(), "8192-byte limit") { + t.Fatalf("loadSecretsFromConfig(over-limit) error = %v, want bounded API-key loader failure", err) + } + if strings.Contains(err.Error(), overLimit[:32]) { + t.Fatalf("error leaked secret content: %q", err) + } +} + +func TestLoadSecretsFromConfigRejectsSymlinkAndAncestorReplacement(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("symlink behavior differs on windows") + } + dir := t.TempDir() + outside := t.TempDir() + ensureSecretDirectory(t, dir) + unsetSecretEnvironment(t, "OPENROUTER_API_KEY") + outsideValue := "outside-secret-value" + mustWriteSecretFile(t, filepath.Join(outside, "OPENROUTER_API_KEY"), outsideValue) + if err := os.Symlink(filepath.Join(outside, "OPENROUTER_API_KEY"), filepath.Join(dir, "OPENROUTER_API_KEY")); err != nil { + t.Fatalf("Symlink(leaf) error = %v", err) + } + _, err := loadSecretsFromConfig(secretConfig(dir), nil) + if err == nil || !strings.Contains(err.Error(), "API-key loader") { + t.Fatalf("loadSecretsFromConfig(symlink) error = %v, want failure", err) + } + if strings.Contains(err.Error(), outsideValue) { + t.Fatalf("error leaked secret content: %q", err) + } + + if err := os.Remove(filepath.Join(dir, "OPENROUTER_API_KEY")); err != nil { + t.Fatalf("Remove(leaf symlink) error = %v", err) + } + mustWriteSecretFile(t, filepath.Join(dir, "OPENROUTER_API_KEY"), "inside-secret") + original := readSecretsDirectoryEntries + readSecretsDirectoryEntries = func(path string) ([]os.DirEntry, error) { + entries, err := original(path) + if err != nil { + return nil, err + } + if err := os.Remove(filepath.Join(dir, "OPENROUTER_API_KEY")); err != nil { + return nil, err + } + if err := os.Remove(dir); err != nil { + return nil, err + } + if err := os.Symlink(outside, dir); err != nil { + return nil, err + } + return entries, nil + } + t.Cleanup(func() { readSecretsDirectoryEntries = original }) + _, err = loadSecretsFromConfig(secretConfig(dir), nil) + if err == nil || !strings.Contains(err.Error(), "API-key loader") { + t.Fatalf("loadSecretsFromConfig(ancestor replacement) error = %v, want failure", err) + } + if _, exists := os.LookupEnv("OPENROUTER_API_KEY"); exists { + t.Fatal("ancestor replacement loaded a secret value") } } func mustWriteSecretFile(t *testing.T, path, contents string) { t.Helper() - if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil { + if err := os.MkdirAll(filepath.Dir(path), secretDirectoryPrivateMode); err != nil { t.Fatalf("MkdirAll(%q): %v", path, err) } + if err := os.Chmod(filepath.Dir(path), secretDirectoryPrivateMode); err != nil { + t.Fatalf("Chmod(%q): %v", filepath.Dir(path), err) + } if err := os.WriteFile(path, []byte(contents), 0o600); err != nil { t.Fatalf("WriteFile(%q): %v", path, err) } } + +func secretConfig(dir string) *config.Config { + return &config.Config{Pipeline: &config.PipelineConfig{Secrets: &config.SecretsConfig{EnvDir: dir}}} +} + +func unsetSecretEnvironment(t *testing.T, name string) { + t.Helper() + previous, existed := os.LookupEnv(name) + if err := os.Unsetenv(name); err != nil { + t.Fatalf("Unsetenv(%q): %v", name, err) + } + t.Cleanup(func() { + if existed { + _ = os.Setenv(name, previous) + return + } + _ = os.Unsetenv(name) + }) +} + +func ensureSecretDirectory(t *testing.T, directory string) { + t.Helper() + if err := os.Chmod(directory, secretDirectoryPrivateMode); err != nil { + t.Fatalf("Chmod(%q): %v", directory, err) + } +} diff --git a/internal/app/secrets_env_windows.go b/internal/app/secrets_env_windows.go new file mode 100644 index 0000000..2c2510a --- /dev/null +++ b/internal/app/secrets_env_windows.go @@ -0,0 +1,11 @@ +//go:build windows + +package app + +import "os" + +// Windows access control is determined by ACLs rather than POSIX mode bits. +func validateSecretDirectoryPrivacy(_ os.FileInfo) error { return nil } + +// Windows access control is determined by ACLs rather than POSIX mode bits. +func validateSecretFilePrivacy(_ os.FileInfo) error { return nil } diff --git a/internal/fileops/read_confined.go b/internal/fileops/read_confined.go new file mode 100644 index 0000000..4f6ee9e --- /dev/null +++ b/internal/fileops/read_confined.go @@ -0,0 +1,110 @@ +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 +}