diff --git a/docs/integrations/promptkit.md b/docs/integrations/promptkit.md index 068cff3..fce02b0 100644 --- a/docs/integrations/promptkit.md +++ b/docs/integrations/promptkit.md @@ -32,6 +32,8 @@ Promptkit receives the YAML data package as an inline input and returns structur When capture is enabled, its preparation artifact projects a provider endpoint to its scheme and host and retains only reviewed execution settings. Provider extras and URL user information, paths, queries, and fragments are omitted. +Capture storage remains confined to the operator-selected debug root; an unsafe +filesystem path causes the requested execution to fail. ## Comparison Execution diff --git a/docs/operations.md b/docs/operations.md index 83e4aa5..2158134 100644 --- a/docs/operations.md +++ b/docs/operations.md @@ -147,6 +147,9 @@ Preparation captures retain only the provider endpoint origin and reviewed execution settings. URL user information, paths, queries, fragments, and unrecognized provider parameters are omitted. +Capture writes are confined to the requested root and fail if an unsafe +filesystem component prevents secure artifact creation. + If capture creation or writing fails, the affected run fails rather than silently continuing without the requested diagnostics. diff --git a/go.mod b/go.mod index 9d988c5..82562c2 100644 --- a/go.mod +++ b/go.mod @@ -7,6 +7,7 @@ require gopkg.in/yaml.v3 v3.0.1 require ( gitea.maximumdirect.net/eric/distributor v0.5.0 gitea.maximumdirect.net/eric/promptkit v0.5.0 + golang.org/x/sys v0.45.0 ) require ( diff --git a/internal/app/app.go b/internal/app/app.go index 278a43f..908e5b7 100644 --- a/internal/app/app.go +++ b/internal/app/app.go @@ -258,6 +258,7 @@ func GenerateDetailed(ctx context.Context, req GenerateRequest) (*ReportResult, if err != nil { return result, promptexec.NewError(promptexec.InvalidConfiguration, "initialize prompt debug", err) } + defer func() { _ = debugWriter.Close() }() inspection, err := InspectPromptExecution(ctx, PromptInspectionRequest{ Resolved: resolved, Executor: req.Executor, @@ -309,6 +310,7 @@ func RunBatchDetailed(ctx context.Context, req BatchRequest) (*BatchResult, erro if err != nil { return nil, promptexec.NewError(promptexec.InvalidConfiguration, "initialize prompt debug", err) } + defer func() { _ = debugWriter.Close() }() candidates, err := batchInspectionCandidates(req, now) if err != nil { return nil, err diff --git a/internal/app/comparison.go b/internal/app/comparison.go index d3dcced..eacaf2c 100644 --- a/internal/app/comparison.go +++ b/internal/app/comparison.go @@ -119,6 +119,7 @@ func compareDetailed(ctx context.Context, req ComparisonRequest, publish compari if err != nil { return result, promptexec.NewError(promptexec.InvalidConfiguration, "initialize prompt debug", err) } + defer func() { _ = debugWriter.Close() }() inspection, err := InspectComparisonExecution(ctx, ComparisonInspectionRequest{ Resolved: resolved, ProfileIDs: req.ProfileIDs, Executor: req.Executor, LookupEnv: os.LookupEnv, }) diff --git a/internal/promptdebug/debug_writer.go b/internal/promptdebug/debug_writer.go index f84cd04..ad7aa52 100644 --- a/internal/promptdebug/debug_writer.go +++ b/internal/promptdebug/debug_writer.go @@ -6,7 +6,6 @@ import ( "encoding/json" "fmt" "net/url" - "os" "path/filepath" "strings" "time" @@ -25,7 +24,8 @@ const ( // PromptDebugWriter stores explicitly requested content-rich diagnostics. A // writer created without a root is disabled. type PromptDebugWriter struct { - root string + root string + directory *secureDirectory } // PromptDebugRef identifies one debug capture directory. @@ -140,14 +140,27 @@ func NewPromptDebugWriter(root string) (*PromptDebugWriter, error) { if cleaned == string(filepath.Separator) { return nil, fmt.Errorf("prompt debug root must not be the filesystem root") } - if err := ensureSecureDirectory(cleaned); err != nil { + directory, err := openSecureDirectory(cleaned) + if err != nil { return nil, fmt.Errorf("initialize prompt debug root %q: %w", cleaned, err) } - return &PromptDebugWriter{root: cleaned}, nil + return &PromptDebugWriter{root: cleaned, directory: directory}, nil } func (w *PromptDebugWriter) Enabled() bool { - return w != nil && w.root != "" + return w != nil && w.root != "" && w.directory != nil +} + +// Close releases the secure directory handle retained for an enabled writer. +// It is safe to call on disabled writers. +func (w *PromptDebugWriter) Close() error { + if w == nil || w.directory == nil { + return nil + } + directory := w.directory + w.directory = nil + w.root = "" + return directory.Close() } // WritePreparation stores the explicitly captured preparation details and @@ -156,10 +169,11 @@ func (w *PromptDebugWriter) WritePreparation(ref PromptDebugRef, preparation pro if !w.Enabled() { return "", nil } - directory, err := w.runDirectory(ref) + directory, secureDirectory, err := w.runDirectory(ref) if err != nil { return "", err } + defer secureDirectory.Close() artifact := PromptPreparationDebugArtifact{ SchemaVersion: promptPreparationDebugSchemaVersion, ReportID: ref.ReportID, @@ -177,7 +191,7 @@ func (w *PromptDebugWriter) WritePreparation(ref PromptDebugRef, preparation pro } artifact.Parameters = parameters } - if err := writeSecureJSON(filepath.Join(directory, "preparation.json"), artifact); err != nil { + if err := secureDirectory.writeJSON("preparation.json", artifact); err != nil { return "", err } return directory, nil @@ -189,10 +203,11 @@ func (w *PromptDebugWriter) WriteExecution(ref PromptDebugRef, execution prompte if !w.Enabled() { return "", nil } - directory, err := w.runDirectory(ref) + directory, secureDirectory, err := w.runDirectory(ref) if err != nil { return "", err } + defer secureDirectory.Close() artifact := PromptExecutionDebugArtifact{ SchemaVersion: promptExecutionDebugSchemaVersion, ReportID: ref.ReportID, @@ -208,27 +223,25 @@ func (w *PromptDebugWriter) WriteExecution(ref PromptDebugRef, execution prompte } artifact.DebugValidationDetails = append([]string(nil), execution.Debug.ValidationDiagnostics...) } - if err := writeSecureJSON(filepath.Join(directory, "execution.json"), artifact); err != nil { + if err := secureDirectory.writeJSON("execution.json", artifact); err != nil { return "", err } return directory, nil } -func (w *PromptDebugWriter) runDirectory(ref PromptDebugRef) (string, error) { +func (w *PromptDebugWriter) runDirectory(ref PromptDebugRef) (string, *secureDirectory, error) { if err := validatePromptDebugRef(ref); err != nil { - return "", err - } - if err := ensureSecureDirectory(w.root); err != nil { - return "", fmt.Errorf("validate prompt debug root %q: %w", w.root, err) + return "", nil, err } directory := filepath.Join(w.root, string(ref.ReportID), ref.ValidDate, ref.RunID) if !isWithinDirectory(w.root, directory) { - return "", fmt.Errorf("prompt debug path escapes root") + return "", nil, fmt.Errorf("prompt debug path escapes root") } - if err := ensureSecureDirectory(directory); err != nil { - return "", fmt.Errorf("create prompt debug directory %q: %w", directory, err) + secureDirectory, err := w.directory.openDirectory(string(ref.ReportID), ref.ValidDate, ref.RunID) + if err != nil { + return "", nil, fmt.Errorf("create prompt debug directory %q: %w", directory, err) } - return directory, nil + return directory, secureDirectory, nil } func validatePromptDebugRef(ref PromptDebugRef) error { @@ -254,100 +267,11 @@ func validatePromptDebugSegment(name string, value string) error { return nil } -func ensureSecureDirectory(path string) error { - if !filepath.IsAbs(path) { - return fmt.Errorf("directory must be absolute") - } - cleaned := filepath.Clean(path) - volume := filepath.VolumeName(cleaned) - current := volume + string(filepath.Separator) - for _, component := range strings.Split(strings.TrimPrefix(cleaned, current), string(filepath.Separator)) { - if component == "" { - continue - } - current = filepath.Join(current, component) - info, err := os.Lstat(current) - if os.IsNotExist(err) { - if err := os.Mkdir(current, debugDirectoryMode); err != nil { - if !os.IsExist(err) { - return err - } - info, err = os.Lstat(current) - if err != nil { - return err - } - if info.Mode()&os.ModeSymlink != 0 { - return fmt.Errorf("directory component %q must not be a symlink", current) - } - if !info.IsDir() { - return fmt.Errorf("directory component %q is not a directory", current) - } - if err := os.Chmod(current, debugDirectoryMode); err != nil { - return err - } - continue - } - if err := os.Chmod(current, debugDirectoryMode); err != nil { - return err - } - continue - } - if err != nil { - return err - } - if info.Mode()&os.ModeSymlink != 0 { - return fmt.Errorf("directory component %q must not be a symlink", current) - } - if !info.IsDir() { - return fmt.Errorf("directory component %q is not a directory", current) - } - } - if err := os.Chmod(cleaned, debugDirectoryMode); err != nil { - return err - } - return nil -} - func isWithinDirectory(root string, path string) bool { relative, err := filepath.Rel(root, path) return err == nil && relative != ".." && !strings.HasPrefix(relative, ".."+string(filepath.Separator)) && !filepath.IsAbs(relative) } -func writeSecureJSON(path string, value any) error { - data, err := json.MarshalIndent(value, "", " ") - if err != nil { - return fmt.Errorf("marshal %q: %w", path, err) - } - if info, err := os.Lstat(path); err == nil { - if info.Mode()&os.ModeSymlink != 0 || !info.Mode().IsRegular() { - return fmt.Errorf("prompt debug file %q is not a regular file", path) - } - } else if !os.IsNotExist(err) { - return err - } - temporary, err := os.CreateTemp(filepath.Dir(path), "."+filepath.Base(path)+".*.tmp") - if err != nil { - return err - } - temporaryPath := temporary.Name() - defer os.Remove(temporaryPath) - if err := temporary.Chmod(debugFileMode); err != nil { - temporary.Close() - return err - } - if _, err := temporary.Write(data); err != nil { - temporary.Close() - return err - } - if err := temporary.Close(); err != nil { - return err - } - if err := os.Rename(temporaryPath, path); err != nil { - return err - } - return nil -} - func promptDebugPreparation(value promptexec.Preparation) PromptDebugPreparation { return PromptDebugPreparation{ PromptID: value.PromptID, PromptVersion: value.PromptVersion, PromptHash: value.PromptHash, diff --git a/internal/promptdebug/debug_writer_test.go b/internal/promptdebug/debug_writer_test.go index 8590593..4fe15e1 100644 --- a/internal/promptdebug/debug_writer_test.go +++ b/internal/promptdebug/debug_writer_test.go @@ -233,6 +233,48 @@ func TestPromptDebugWriterCreatesSharedMissingAncestorsConcurrently(t *testing.T } } +func TestPromptDebugWriterKeepsWritesAnchoredToOpenedRoot(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("directory replacement behavior is covered on Unix hosts") + } + parent := t.TempDir() + root := filepath.Join(parent, "debug") + writer, err := NewPromptDebugWriter(root) + if err != nil { + t.Fatalf("NewPromptDebugWriter() error = %v", err) + } + defer writer.Close() + + anchoredRoot := filepath.Join(parent, "anchored-debug") + outside := filepath.Join(parent, "outside") + if err := os.Mkdir(outside, debugDirectoryMode); err != nil { + t.Fatalf("create outside directory: %v", err) + } + if err := os.Rename(root, anchoredRoot); err != nil { + t.Fatalf("replace opened root: %v", err) + } + if err := os.Symlink(outside, root); err != nil { + t.Skipf("symlink creation is unavailable: %v", err) + } + + ref := promptDebugRef() + ref.RunID = "run-anchored" + if _, err := writer.WritePreparation(ref, promptDebugPreparationFixture(), nil); err != nil { + t.Fatalf("WritePreparation() error = %v", err) + } + anchoredArtifact := filepath.Join(anchoredRoot, "daily", "2026-05-29", "run-anchored", "preparation.json") + if _, err := os.Stat(anchoredArtifact); err != nil { + t.Fatalf("anchored preparation artifact: %v", err) + } + entries, err := os.ReadDir(outside) + if err != nil { + t.Fatalf("read outside directory: %v", err) + } + if len(entries) != 0 { + t.Fatalf("outside directory received debug artifacts: %#v", entries) + } +} + func TestPromptDebugWriterDisabledDoesNotAccessFilesystem(t *testing.T) { writer, err := NewPromptDebugWriter("") if err != nil { diff --git a/internal/promptdebug/secure_directory_other.go b/internal/promptdebug/secure_directory_other.go new file mode 100644 index 0000000..99d3320 --- /dev/null +++ b/internal/promptdebug/secure_directory_other.go @@ -0,0 +1,143 @@ +//go:build !unix + +package promptdebug + +import ( + "crypto/rand" + "encoding/hex" + "encoding/json" + "fmt" + "io" + "os" + "path/filepath" + "strings" +) + +type secureDirectory struct { + root *os.Root +} + +func openSecureDirectory(path string) (*secureDirectory, error) { + if !filepath.IsAbs(path) { + return nil, fmt.Errorf("directory must be absolute") + } + if err := os.MkdirAll(path, debugDirectoryMode); err != nil { + return nil, err + } + info, err := os.Lstat(path) + if err != nil { + return nil, err + } + if info.Mode()&os.ModeSymlink != 0 || !info.IsDir() { + return nil, fmt.Errorf("prompt debug root is not a directory") + } + if err := os.Chmod(path, debugDirectoryMode); err != nil { + return nil, err + } + root, err := os.OpenRoot(path) + if err != nil { + return nil, err + } + return &secureDirectory{root: root}, nil +} + +func (directory *secureDirectory) Close() error { + if directory == nil || directory.root == nil { + return nil + } + root := directory.root + directory.root = nil + return root.Close() +} + +func (directory *secureDirectory) openDirectory(components ...string) (*secureDirectory, error) { + if directory == nil || directory.root == nil { + return nil, fmt.Errorf("prompt debug directory is closed") + } + for _, component := range components { + if err := validateSecureDirectoryName(component); err != nil { + return nil, err + } + } + path := filepath.Join(components...) + if err := directory.root.MkdirAll(path, debugDirectoryMode); err != nil { + return nil, err + } + child, err := directory.root.OpenRoot(path) + if err != nil { + return nil, err + } + return &secureDirectory{root: child}, nil +} + +func validateSecureDirectoryName(name string) error { + if name == "" || name == "." || name == ".." || strings.ContainsRune(name, filepath.Separator) { + return fmt.Errorf("prompt debug directory component %q is invalid", name) + } + return nil +} + +func (directory *secureDirectory) writeJSON(name string, value any) error { + if directory == nil || directory.root == nil { + return fmt.Errorf("prompt debug directory is closed") + } + if err := validateSecureDirectoryName(name); err != nil { + return err + } + data, err := json.MarshalIndent(value, "", " ") + if err != nil { + return fmt.Errorf("marshal prompt debug artifact: %w", err) + } + if info, err := directory.root.Lstat(name); err == nil { + if info.Mode()&os.ModeSymlink != 0 || !info.Mode().IsRegular() { + return fmt.Errorf("prompt debug file %q is not a regular file", name) + } + } else if !os.IsNotExist(err) { + return err + } + temporaryName, temporary, err := directory.createTemporaryFile(name) + if err != nil { + return err + } + defer func() { + if temporary != nil { + _ = temporary.Close() + } + _ = directory.root.Remove(temporaryName) + }() + if err := temporary.Chmod(debugFileMode); err != nil { + return err + } + if written, err := temporary.Write(data); err != nil { + return err + } else if written != len(data) { + return io.ErrShortWrite + } + if err := temporary.Close(); err != nil { + return err + } + temporary = nil + if err := directory.root.Rename(temporaryName, name); err != nil { + return fmt.Errorf("replace prompt debug file %q: %w", name, err) + } + return nil +} + +func (directory *secureDirectory) createTemporaryFile(name string) (string, *os.File, error) { + for attempt := 0; attempt < 16; attempt++ { + random := make([]byte, 12) + if _, err := rand.Read(random); err != nil { + return "", nil, err + } + temporaryName := "." + name + "." + hex.EncodeToString(random) + ".tmp" + temporary, err := directory.root.OpenFile(temporaryName, os.O_WRONLY|os.O_CREATE|os.O_EXCL, debugFileMode) + if os.IsExist(err) { + continue + } + if err != nil { + return "", nil, err + } + return temporaryName, temporary, nil + } + return "", nil, fmt.Errorf("create temporary prompt debug file: too many name collisions") +} diff --git a/internal/promptdebug/secure_directory_unix.go b/internal/promptdebug/secure_directory_unix.go new file mode 100644 index 0000000..b72e11a --- /dev/null +++ b/internal/promptdebug/secure_directory_unix.go @@ -0,0 +1,196 @@ +//go:build unix + +package promptdebug + +import ( + "crypto/rand" + "encoding/hex" + "encoding/json" + "errors" + "fmt" + "io" + "os" + "path/filepath" + "strings" + + "golang.org/x/sys/unix" +) + +// secureDirectory is an opened directory descriptor. Every operation remains +// relative to that descriptor so later pathname swaps cannot redirect writes. +type secureDirectory struct { + fd int +} + +func openSecureDirectory(path string) (*secureDirectory, error) { + if !filepath.IsAbs(path) { + return nil, fmt.Errorf("directory must be absolute") + } + cleaned := filepath.Clean(path) + current, err := unix.Open(string(filepath.Separator), unix.O_RDONLY|unix.O_DIRECTORY|unix.O_CLOEXEC, 0) + if err != nil { + return nil, err + } + defer func() { + if current != -1 { + _ = unix.Close(current) + } + }() + for _, component := range strings.Split(strings.TrimPrefix(cleaned, string(filepath.Separator)), string(filepath.Separator)) { + if component == "" { + continue + } + next, created, err := openOrCreateSecureDirectory(current, component) + if err != nil { + return nil, err + } + if created { + if err := unix.Fchmod(next, debugDirectoryMode); err != nil { + _ = unix.Close(next) + return nil, err + } + } + _ = unix.Close(current) + current = next + } + if err := unix.Fchmod(current, debugDirectoryMode); err != nil { + return nil, err + } + result := &secureDirectory{fd: current} + current = -1 + return result, nil +} + +func (directory *secureDirectory) Close() error { + if directory == nil || directory.fd < 0 { + return nil + } + fd := directory.fd + directory.fd = -1 + return unix.Close(fd) +} + +func (directory *secureDirectory) openDirectory(components ...string) (*secureDirectory, error) { + if directory == nil || directory.fd < 0 { + return nil, fmt.Errorf("prompt debug directory is closed") + } + current := directory.fd + owned := false + defer func() { + if owned { + _ = unix.Close(current) + } + }() + for _, component := range components { + if err := validateSecureDirectoryName(component); err != nil { + return nil, err + } + next, _, err := openOrCreateSecureDirectory(current, component) + if err != nil { + return nil, err + } + if err := unix.Fchmod(next, debugDirectoryMode); err != nil { + _ = unix.Close(next) + return nil, err + } + if owned { + _ = unix.Close(current) + } + current = next + owned = true + } + result := &secureDirectory{fd: current} + owned = false + return result, nil +} + +func openOrCreateSecureDirectory(parent int, name string) (int, bool, error) { + fd, err := unix.Openat(parent, name, unix.O_RDONLY|unix.O_DIRECTORY|unix.O_NOFOLLOW|unix.O_CLOEXEC, 0) + if err == nil { + return fd, false, nil + } + if !errors.Is(err, unix.ENOENT) { + return -1, false, fmt.Errorf("open secure directory %q: %w", name, err) + } + if err := unix.Mkdirat(parent, name, debugDirectoryMode); err != nil && !errors.Is(err, unix.EEXIST) { + return -1, false, fmt.Errorf("create secure directory %q: %w", name, err) + } + fd, err = unix.Openat(parent, name, unix.O_RDONLY|unix.O_DIRECTORY|unix.O_NOFOLLOW|unix.O_CLOEXEC, 0) + if err != nil { + return -1, false, fmt.Errorf("open created secure directory %q: %w", name, err) + } + return fd, true, nil +} + +func validateSecureDirectoryName(name string) error { + if name == "" || name == "." || name == ".." || strings.ContainsRune(name, filepath.Separator) { + return fmt.Errorf("prompt debug directory component %q is invalid", name) + } + return nil +} + +func (directory *secureDirectory) writeJSON(name string, value any) error { + if directory == nil || directory.fd < 0 { + return fmt.Errorf("prompt debug directory is closed") + } + if err := validateSecureDirectoryName(name); err != nil { + return err + } + data, err := json.MarshalIndent(value, "", " ") + if err != nil { + return fmt.Errorf("marshal prompt debug artifact: %w", err) + } + var information unix.Stat_t + if err := unix.Fstatat(directory.fd, name, &information, unix.AT_SYMLINK_NOFOLLOW); err == nil { + if information.Mode&unix.S_IFMT != unix.S_IFREG { + return fmt.Errorf("prompt debug file %q is not a regular file", name) + } + } else if !errors.Is(err, unix.ENOENT) { + return fmt.Errorf("inspect prompt debug file %q: %w", name, err) + } + temporaryName, temporary, err := directory.createTemporaryFile(name) + if err != nil { + return err + } + defer func() { + if temporary != nil { + _ = temporary.Close() + } + _ = unix.Unlinkat(directory.fd, temporaryName, 0) + }() + if err := temporary.Chmod(debugFileMode); err != nil { + return err + } + if written, err := temporary.Write(data); err != nil { + return err + } else if written != len(data) { + return io.ErrShortWrite + } + if err := temporary.Close(); err != nil { + return err + } + temporary = nil + if err := unix.Renameat(directory.fd, temporaryName, directory.fd, name); err != nil { + return fmt.Errorf("replace prompt debug file %q: %w", name, err) + } + return nil +} + +func (directory *secureDirectory) createTemporaryFile(name string) (string, *os.File, error) { + for attempt := 0; attempt < 16; attempt++ { + random := make([]byte, 12) + if _, err := rand.Read(random); err != nil { + return "", nil, err + } + temporaryName := "." + name + "." + hex.EncodeToString(random) + ".tmp" + fd, err := unix.Openat(directory.fd, temporaryName, unix.O_WRONLY|unix.O_CREAT|unix.O_EXCL|unix.O_CLOEXEC|unix.O_NOFOLLOW, debugFileMode) + if errors.Is(err, unix.EEXIST) { + continue + } + if err != nil { + return "", nil, fmt.Errorf("create temporary prompt debug file: %w", err) + } + return temporaryName, os.NewFile(uintptr(fd), temporaryName), nil + } + return "", nil, fmt.Errorf("create temporary prompt debug file: too many name collisions") +}