diff --git a/docs/contracts/workspace-preprocessing-cli.md b/docs/contracts/workspace-preprocessing-cli.md index c710df66..bb2d71bb 100644 --- a/docs/contracts/workspace-preprocessing-cli.md +++ b/docs/contracts/workspace-preprocessing-cli.md @@ -24,6 +24,6 @@ The bounded schema-v1 stdin envelope is exact: omitted fields are not equivalent * `check`: `resume` is required; `annotations` and `reviewedCandidates` are either both present or both absent. `annotations` is exactly `{basename, contentBase64, sha256}`. * `index-schema`: no additional fields. -No other fields, duplicate JSON value, host path, raw SQL, or raw annotation content are accepted. SQL and annotations use a logical basename, base64 bytes, and a declared `sha256:` digest. The request envelope is independently capped at 24 MiB (after JSON/base64 encoding), enough to carry the frozen 1 MiB-per-file/16 MiB aggregate raw ingress bounds; every supplied field/value must exactly match the command-derived envelope. Candidate responses may carry the internal `hostExport` (`mediaType`, `sha256`, `contentBase64`) only for `suggest-fks`; its object is strict (unknown fields rejected) and is always verified, even without `--output`: YAML media type (`application/yaml` or `text/yaml`), UTF-8, digest, and decoded size at most 700 KiB. It is removed from the public result and written exclusively only after result/run/identity validation. Output publication uses restrictive mode `0600` and refuses existing leaves, links, hardlinks, directories, replacement races, and reparse points. On Linux, publication uses an anonymous `O_TMPFILE` inode and `linkat(..., AT_EMPTY_PATH)`; the link operation is the final commit, and no post-commit check can turn success into a not-published error. On Darwin, the named-stage implementation is a trusted-parent mode: the target parent and ancestors must remain namespace-stable and same-UID stage mutation is explicitly outside the threat model. It verifies the stage identity/link count before using `renameatx_np(..., RENAME_EXCL)` as the final no-replace commit. It does not claim protection against a same-UID hostile hard-linker. On Windows, the stage is held open with `DELETE|WRITE` and zero sharing, then renamed atomically with `SetFileInformationByHandle(FileRenameInfo)` rooted at the retained parent handle; replacement is disabled and the rename is final. +No other fields, duplicate JSON value, host path, raw SQL, or raw annotation content are accepted. SQL and annotations use a logical basename, base64 bytes, and a declared `sha256:` digest. The request envelope is independently capped at 24 MiB (after JSON/base64 encoding), enough to carry the frozen 1 MiB-per-file/16 MiB aggregate raw ingress bounds; every supplied field/value must exactly match the command-derived envelope. Candidate responses may carry the internal `hostExport` (`mediaType`, `sha256`, `contentBase64`) only for `suggest-fks`; its object is strict (unknown fields rejected) and is always verified, even without `--output`: YAML media type (`application/yaml` or `text/yaml`), UTF-8, digest, and decoded size at most 700 KiB. It is removed from the public result and written exclusively only after result/run/identity validation. The exact once-encoded final stdout bytes, including JSON keys and human chrome, are scanned for every declared nonempty secret before publication. Output publication uses restrictive mode `0600` and refuses existing leaves, links, hardlinks, directories, replacement races, and reparse points. On Linux, publication uses an anonymous `O_TMPFILE` inode and `linkat(..., AT_EMPTY_PATH)`; the link operation is the final commit, and no post-commit check can turn success into a not-published error. On Darwin, the named-stage implementation is a trusted-parent mode: the target parent and ancestors must remain namespace-stable and same-UID stage mutation is explicitly outside the threat model. It verifies the stage identity/link count before using `renameatx_np(..., RENAME_EXCL)` as the final no-replace commit. It does not claim protection against a same-UID hostile hard-linker. On Windows, the stage is held open with `DELETE|WRITE` and zero sharing, then renamed atomically with `SetFileInformationByHandle(FileRenameInfo)` rooted at the retained parent handle; replacement is disabled and the rename is final. -The public result has schema version 1 and only these fields: `status`, `code`, workspace/revision/descriptor/operation identities, optional run and child run IDs, completed stages, counts, artifact identities, and warnings. Revisions/descriptors are 40-hex; run IDs are 32-hex; artifact digests are `sha256:`. Allowed statuses are `succeeded`, `unchanged`, `dry_run`, `blocked`, and `failed`. Allowed codes are `ok`, `workspace_not_found`, `workspace_not_activatable`, `binding_missing`, `preprocessing_conflict`, `preprocessing_resume_mismatch`, `manual_review_required`, `evidence_materialization_required`, `effective_config_mismatch`, `semantic_index_incompatible`, `annotation_invalid`, `egress_policy_refused`, and `registry_bootstrap_recovery_conflict`. `succeeded`, `unchanged`, and `dry_run` require `ok` and child exit 0. `blocked` requires one of `manual_review_required`, `evidence_materialization_required`, `preprocessing_conflict`, `preprocessing_resume_mismatch`, or `registry_bootstrap_recovery_conflict`, and child exit 3. `failed` requires a non-`ok` operational code other than those blocked-only codes, and child exit 1. A nonzero child exit is never accepted for another status/code combination. The public `thothctl` exit mapping is fixed independently of child details: `0` for succeeded/unchanged/dry-run, `3` for an expected blocked result, `2` for command grammar or unsafe host-file failures, and `1` for operational failures (including invalid child envelopes, output-limit failures, and child execution failures). Stdout is capped at 1 MiB after final human/JSON encoding and stderr at 64 KiB after sanitization; output is never allowed to exceed those bounds. In human mode, a `registry_bootstrap_recovery_conflict` result prints exactly `Bootstrap recovery is ambiguous or corrupt; inspect the installation registry jobs.` and prints neither a candidate export nor any run/candidate ID. Compose is invoked only as `compose run --rm --no-deps --no-TTY workspace-maintenance ...`; output never includes child stderr or secrets. +The public result has schema version 1 and only these fields: `status`, `code`, workspace/revision/descriptor/operation identities, optional run and child run IDs, completed stages, counts, artifact identities, and warnings. Revisions/descriptors are 40-hex; run IDs are 32-hex; artifact digests are `sha256:`. Allowed statuses are `succeeded`, `unchanged`, `dry_run`, `blocked`, and `failed`. Allowed codes are `ok`, `workspace_not_found`, `workspace_not_activatable`, `binding_missing`, `preprocessing_conflict`, `preprocessing_resume_mismatch`, `manual_review_required`, `evidence_materialization_required`, `effective_config_mismatch`, `semantic_index_incompatible`, `annotation_invalid`, `egress_policy_refused`, and `registry_bootstrap_recovery_conflict`. `succeeded`, `unchanged`, and `dry_run` require `ok` and child exit 0. `blocked` requires one of `manual_review_required`, `evidence_materialization_required`, `preprocessing_conflict`, `preprocessing_resume_mismatch`, or `registry_bootstrap_recovery_conflict`, and child exit 3. `failed` requires a non-`ok` operational code other than those blocked-only codes, and child exit 1. A nonzero child exit is never accepted for another status/code combination. The public `thothctl` exit mapping is fixed independently of child details: `0` for succeeded/unchanged/dry-run, `3` for an expected blocked result, `2` for command grammar or proven unsafe host-file failures, and `1` for operational or indeterminate failures (including invalid child envelopes, output-limit failures, child execution failures, and candidate cleanup uncertainty). A physical stdout write failure after candidate publication is a committed/indeterminate reconcile case; inspect the destination and private stages before retrying. Stdout is capped at 1 MiB after final human/JSON encoding and stderr at 64 KiB after sanitization; output is never allowed to exceed those bounds. In human mode, a `registry_bootstrap_recovery_conflict` result prints exactly `Bootstrap recovery is ambiguous or corrupt; inspect the installation registry jobs.` and prints neither a candidate export nor any run/candidate ID. Compose is invoked only as `compose run --rm --no-deps --no-TTY workspace-maintenance ...`; output never includes child stderr or secrets. diff --git a/tools/thothctl/cmd/thothctl/main.go b/tools/thothctl/cmd/thothctl/main.go index b7ec8883..ebaff926 100644 --- a/tools/thothctl/cmd/thothctl/main.go +++ b/tools/thothctl/cmd/thothctl/main.go @@ -69,6 +69,7 @@ func main() { } func run(ctx context.Context, args []string, stdout, stderr io.Writer) int { + isWorkspaceCommand := len(args) > 2 && args[2] == "workspace" // Workspace results are untrusted child output, so only that dispatch receives // the public bounds. Legacy renderers intentionally retain their established // behavior and must not silently truncate successful output. @@ -82,21 +83,33 @@ func run(ctx context.Context, args []string, stdout, stderr io.Writer) int { } installationPath, command, commandArgs, err := parseArgs(args) if err != nil { + if isWorkspaceCommand { + return writeWorkspaceError(stderr, err, nil, 2) + } fmt.Fprintf(stderr, "thothctl: %s\n\n%s", err, usage) return 2 } installation, err := config.Load(installationPath) if err != nil { + if isWorkspaceCommand { + return writeWorkspaceError(stderr, err, nil, 2) + } fmt.Fprintf(stderr, "thothctl: %s\n", output.Sanitize(err.Error(), nil)) return 2 } secretFiles, err := installation.SecretFiles() if err != nil { + if isWorkspaceCommand { + return writeWorkspaceError(stderr, errors.New("installation secret declarations could not be read"), nil, 2) + } fmt.Fprintln(stderr, "thothctl: installation secret declarations could not be read") return 2 } secretValues, err := output.SecretValuesFromFiles(secretFiles) if err != nil { + if isWorkspaceCommand { + return writeWorkspaceError(stderr, errors.New("declared secret file could not be read"), nil, 2) + } fmt.Fprintln(stderr, "thothctl: declared secret file could not be read") return 2 } @@ -105,7 +118,7 @@ func run(ctx context.Context, args []string, stdout, stderr io.Writer) int { if command == "workspace" { workspaceCommand, parseErr := workspaceops.ParseWorkspaceCommand(append([]string{"workspace"}, commandArgs...)) if parseErr != nil { - return commandUsageError(stderr, parseErr.Error()) + return writeWorkspaceError(stderr, parseErr, secretValues, 2) } jsonMode := true switch c := workspaceCommand.(type) { @@ -134,21 +147,31 @@ func run(ctx context.Context, args []string, stdout, stderr io.Writer) int { } else { finalOutput, err = encodeWorkspaceHuman(result) } - if err != nil || len(finalOutput) > maxPublicStdoutBytes { + if err != nil { + return errors.New("workspace output could not be encoded") + } + if len(finalOutput) > maxPublicStdoutBytes { return errors.New("workspace result exceeds output limit") } + // This is the exact once-encoded byte slice that will be published. + // Scan it after encoding so JSON keys and human renderer chrome are + // inside the same no-secret boundary as typed result values. + if containsDeclaredSecretBytes(finalOutput, secretValues) { + return errors.New("workspace output contains a declared secret") + } return nil }) if operationErr != nil { if workspaceUsageError(operationErr) { - return commandUsageError(stderr, operationErr.Error()) + return writeWorkspaceError(stderr, operationErr, secretValues, 2) } - fmt.Fprintf(stderr, "thothctl: %s\n", output.Sanitize(operationErr.Error(), secretValues)) - return 1 + return writeWorkspaceError(stderr, operationErr, secretValues, 1) } if _, err := stdout.Write(finalOutput); err != nil { - fmt.Fprintln(stderr, "thothctl: workspace result exceeds output limit") - return 1 + // The candidate publication precedes stdout. A physical stdout + // failure therefore requires reconciliation before retrying; it is + // not an output-limit rejection. + return writeWorkspaceError(stderr, errors.New("workspace output write failed; reconcile any committed candidate before retrying"), secretValues, 1) } if result.Status == "blocked" { return 3 @@ -270,6 +293,37 @@ func (w *boundedWriter) Write(p []byte) (int, error) { return n, err } +func containsDeclaredSecretBytes(contents []byte, secrets []string) bool { + for _, secret := range secrets { + if secret != "" && bytes.Contains(contents, []byte(secret)) { + return true + } + } + return false +} + +// writeWorkspaceError is the sole workspace stderr path. It validates the +// complete rendered line, including its fixed prefix, before writing; if no +// deterministic safe line exists it emits empty stderr rather than risk a +// declared-secret collision. +func writeWorkspaceError(stderr io.Writer, err error, secrets []string, code int) int { + message := "workspace operation failed" + if err != nil { + message = output.Sanitize(err.Error(), secrets) + } + candidates := [][]byte{[]byte("workspace operation failed\n")} + if secrets != nil { + candidates = append([][]byte{[]byte("thothctl: " + message + "\n")}, candidates...) + } + for _, candidate := range candidates { + if !containsDeclaredSecretBytes(candidate, secrets) { + _, _ = stderr.Write(candidate) + return code + } + } + return code +} + func encodeWorkspaceJSON(result workspaceops.Result) ([]byte, error) { var encoded bytes.Buffer encoder := json.NewEncoder(&encoded) diff --git a/tools/thothctl/cmd/thothctl/main_test.go b/tools/thothctl/cmd/thothctl/main_test.go index a8513432..a39c29d7 100644 --- a/tools/thothctl/cmd/thothctl/main_test.go +++ b/tools/thothctl/cmd/thothctl/main_test.go @@ -3,6 +3,8 @@ package main import ( "bytes" "context" + "crypto/sha256" + "encoding/base64" "encoding/json" "errors" "fmt" @@ -134,12 +136,21 @@ func TestRunWorkspacePublicDispatchExitMatrix(t *testing.T) { } func TestRunWorkspaceBoundsFinalJSONEncoding(t *testing.T) { - for _, tc := range []struct{ name string; final int; wantCode int }{{"exact", 1 << 20, 0}, {"one-over", (1 << 20) + 1, 1}} { + for _, tc := range []struct { + name string + final int + wantCode int + }{ + {name: "exact", final: 1 << 20, wantCode: 0}, + {name: "one-over", final: (1 << 20) + 1, wantCode: 1}, + } { t.Run(tc.name, func(t *testing.T) { fixture := newCLIFixture(t, "") fixture.setEnvironment(t) payload, encodedLength := boundedWorkspaceResultPayload(t, tc.final) - if encodedLength != tc.final { t.Fatalf("final encoded length = %d, want %d", encodedLength, tc.final) } + if encodedLength != tc.final { + t.Fatalf("final encoded length = %d, want %d", encodedLength, tc.final) + } writeWorkspaceResultFile(t, fixture, payload) var stdout, stderr bytes.Buffer code := run(context.Background(), []string{"--installation", fixture.installationPath, "workspace", "inspect", "--workspace", "psd", "--json"}, &stdout, &stderr) @@ -150,6 +161,78 @@ func TestRunWorkspaceBoundsFinalJSONEncoding(t *testing.T) { } } +type failingWorkspaceWriter struct{} + +func (failingWorkspaceWriter) Write([]byte) (int, error) { + return 0, errors.New("injected stdout failure") +} + +func TestRunWorkspaceReportsCommittedReconcileOnStdoutFailure(t *testing.T) { + fixture := newCLIFixture(t, "") + fixture.setEnvironment(t) + t.Setenv("THOTHCTL_FAKE_WORKSPACE_RESULT", `{"schemaVersion":1,"status":"succeeded","code":"ok","workspaceId":"psd","workspaceRevision":"0000000000000000000000000000000000000000","descriptorBlob":"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa","operation":"inspect","completedStages":[]}`) + var stderr bytes.Buffer + code := run(context.Background(), []string{"--installation", fixture.installationPath, "workspace", "inspect", "--workspace", "psd"}, failingWorkspaceWriter{}, &stderr) + if code != 1 || !strings.Contains(stderr.String(), "reconcile") { + t.Fatalf("exit=%d stderr=%q, want committed reconcile guidance", code, stderr.String()) + } +} + +func TestRunWorkspaceRejectsDeclaredSecretInFinalJSONKeyBeforeCandidateCommit(t *testing.T) { + fixture := newCLIFixture(t, "UNLABELLED_SECRET_FILE=%s\n") + secretPath := filepath.Join(fixture.root, "secret") + if err := os.WriteFile(secretPath, []byte("schemaVersion"), 0o600); err != nil { + t.Fatal(err) + } + fixture.setEnvironment(t, secretPath) + candidate := []byte("candidates: []\n") + digest := fmt.Sprintf("%x", sha256.Sum256(candidate)) + result := fmt.Sprintf(`{"schemaVersion":1,"status":"succeeded","code":"ok","workspaceId":"psd","workspaceRevision":"%s","descriptorBlob":"%s","operation":"suggest-fks","runId":"0123456789abcdef0123456789abcdef","completedStages":[],"artifactIdentities":[{"kind":"fk-candidates","digest":"%s"}],"hostExport":{"mediaType":"application/yaml","sha256":"%s","contentBase64":"%s"}}`, strings.Repeat("0", 40), strings.Repeat("a", 40), digest, digest, base64.StdEncoding.EncodeToString(candidate)) + t.Setenv("THOTHCTL_FAKE_WORKSPACE_RESULT", result) + output := filepath.Join(fixture.root, "candidate.yaml") + var stdout, stderr bytes.Buffer + code := run(context.Background(), []string{"--installation", fixture.installationPath, "workspace", "schema", "suggest-fks", "--workspace", "psd", "--output", output, "--json"}, &stdout, &stderr) + if code != 1 || stdout.Len() != 0 { + t.Fatalf("exit=%d stdout=%q stderr=%q", code, stdout.String(), stderr.String()) + } + if _, err := os.Stat(output); !os.IsNotExist(err) { + t.Fatalf("candidate exists after final-byte secret rejection: %v", err) + } + if strings.Contains(stderr.String(), "schemaVersion") { + t.Fatalf("stderr leaked declared secret: %q", stderr.String()) + } +} + +func TestRunWorkspaceRejectsDeclaredSecretInHumanChrome(t *testing.T) { + fixture := newCLIFixture(t, "UNLABELLED_SECRET_FILE=%s\n") + secretPath := filepath.Join(fixture.root, "secret") + if err := os.WriteFile(secretPath, []byte("Workspace"), 0o600); err != nil { + t.Fatal(err) + } + fixture.setEnvironment(t, secretPath) + t.Setenv("THOTHCTL_FAKE_WORKSPACE_RESULT", `{"schemaVersion":1,"status":"succeeded","code":"ok","workspaceId":"psd","workspaceRevision":"0000000000000000000000000000000000000000","descriptorBlob":"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa","operation":"inspect","completedStages":[]}`) + var stdout, stderr bytes.Buffer + code := run(context.Background(), []string{"--installation", fixture.installationPath, "workspace", "inspect", "--workspace", "psd"}, &stdout, &stderr) + if code != 1 || stdout.Len() != 0 || strings.Contains(stderr.String(), "Workspace") { + t.Fatalf("exit=%d stdout=%q stderr=%q", code, stdout.String(), stderr.String()) + } +} + +func TestRunWorkspaceErrorPrefixNeverLeaksDeclaredSecret(t *testing.T) { + fixture := newCLIFixture(t, "UNLABELLED_SECRET_FILE=%s\n") + secretPath := filepath.Join(fixture.root, "secret") + if err := os.WriteFile(secretPath, []byte("thothctl:"), 0o600); err != nil { + t.Fatal(err) + } + fixture.setEnvironment(t, secretPath) + t.Setenv("THOTHCTL_FAKE_WORKSPACE_RESULT", "not-json") + var stdout, stderr bytes.Buffer + code := run(context.Background(), []string{"--installation", fixture.installationPath, "workspace", "inspect", "--workspace", "psd"}, &stdout, &stderr) + if code != 1 || stdout.Len() != 0 || strings.Contains(stderr.String(), "thothctl:") { + t.Fatalf("exit=%d stdout=%q stderr=%q", code, stdout.String(), stderr.String()) + } +} + func TestRunWorkspaceBoundsFinalHumanEncoding(t *testing.T) { fixture := newCLIFixture(t, "") fixture.setEnvironment(t) diff --git a/tools/thothctl/internal/safeio/files.go b/tools/thothctl/internal/safeio/files.go index ea1c4949..2ae10adb 100644 --- a/tools/thothctl/internal/safeio/files.go +++ b/tools/thothctl/internal/safeio/files.go @@ -13,6 +13,15 @@ import ( var ErrUnsafeFile = errors.New("unsafe file") +// ErrIndeterminateFile means publication cleanup could not establish whether a +// private candidate is still named. Callers must reconcile the destination and +// private stages before retrying; it is never a blind-retry-safe failure. +var ErrIndeterminateFile = errors.New("indeterminate file state") + +// beforeBoundedRead is an internal test seam used to deterministically suspend +// a read between opening the file and resolving its final pathname. +var beforeBoundedRead func() + // ValidateCanonicalPath rejects relative or lexically non-canonical paths before they are opened. func ValidateCanonicalPath(path string) error { if !filepath.IsAbs(path) || filepath.Clean(path) != path || strings.Contains(path, string(filepath.Separator)+".."+string(filepath.Separator)) { @@ -32,6 +41,9 @@ func readBoundedRegularFile(file *os.File, maximum int64) ([]byte, error) { if err != nil || !info.Mode().IsRegular() { return nil, ErrUnsafeFile } + if beforeBoundedRead != nil { + beforeBoundedRead() + } contents, err := io.ReadAll(io.LimitReader(file, maximum+1)) if err != nil || int64(len(contents)) > maximum { return nil, ErrUnsafeFile diff --git a/tools/thothctl/internal/safeio/files_unix.go b/tools/thothctl/internal/safeio/files_darwin.go similarity index 86% rename from tools/thothctl/internal/safeio/files_unix.go rename to tools/thothctl/internal/safeio/files_darwin.go index 66533930..55010834 100644 --- a/tools/thothctl/internal/safeio/files_unix.go +++ b/tools/thothctl/internal/safeio/files_darwin.go @@ -1,4 +1,4 @@ -//go:build !windows && !linux +//go:build darwin package safeio @@ -85,6 +85,36 @@ func closeUnixDescriptors(descriptors []int) { // (including stage hard-link/replacement races and ancestor replacement) is outside // this mode's threat model. Under that precondition renameatx_np(RENAME_EXCL) is the // final fallible no-replace commit and removes the stage atomically. +var darwinUnlinkat = unix.Unlinkat +var darwinFstatat = unix.Fstatat +var darwinCloseStage = func(file *os.File) error { return file.Close() } + +func cleanupDarwinStage(dir int, stage string, staged *unix.Stat_t, closeStage func() error) error { + var current unix.Stat_t + statErr := darwinFstatat(dir, stage, ¤t, unix.AT_SYMLINK_NOFOLLOW) + if statErr == unix.ENOENT { + _ = closeStage() + return ErrUnsafeFile + } + if statErr != nil || staged == nil || current.Ino != staged.Ino || current.Dev != staged.Dev { + _ = closeStage() + return ErrIndeterminateFile + } + unlinkErr := darwinUnlinkat(dir, stage, 0) + closeErr := closeStage() + if unlinkErr == nil { + // Successful unlink proves that no named candidate bytes remain; close + // errors cannot make the unlinked inode reachable by pathname. + return ErrUnsafeFile + } + var after unix.Stat_t + if verifyErr := darwinFstatat(dir, stage, &after, unix.AT_SYMLINK_NOFOLLOW); verifyErr == unix.ENOENT { + return ErrUnsafeFile + } + _ = closeErr + return ErrIndeterminateFile +} + func writeCanonicalExclusive(path string, contents []byte, mode fs.FileMode) error { if err := ValidateCanonicalPath(path); err != nil || len(contents) > 16<<20 || mode.Perm() != 0o600 { return ErrUnsafeFile @@ -119,8 +149,11 @@ func writeCanonicalExclusive(path string, contents []byte, mode fs.FileMode) err } stageFile := os.NewFile(uintptr(stageFD), "thothctl-safeio-stage") if stageFile == nil { - _ = unix.Close(stageFD) - _ = unix.Unlinkat(dir, stage, 0) + closeErr := unix.Close(stageFD) + unlinkErr := darwinUnlinkat(dir, stage, 0) + if closeErr != nil || unlinkErr != nil { + return ErrIndeterminateFile + } return ErrUnsafeFile } closed := false @@ -129,24 +162,15 @@ func writeCanonicalExclusive(path string, contents []byte, mode fs.FileMode) err return nil } closed = true - return stageFile.Close() + return darwinCloseStage(stageFile) } defer func() { _ = closeStage() }() var staged unix.Stat_t if err := unix.Fstat(stageFD, &staged); err != nil || staged.Nlink != 1 || staged.Mode&unix.S_IFMT != unix.S_IFREG { _ = closeStage() - _ = unix.Unlinkat(dir, stage, 0) - return ErrUnsafeFile + return ErrIndeterminateFile } - cleanup := func() { - // Trusted-parent mode makes this identity check + unlink pre-commit safe; - // never unlink a replacement observed at the stage name. - var current unix.Stat_t - if unix.Fstatat(dir, stage, ¤t, unix.AT_SYMLINK_NOFOLLOW) == nil && current.Ino == staged.Ino && current.Dev == staged.Dev { - _ = unix.Unlinkat(dir, stage, 0) - } - } - fail := func() error { _ = closeStage(); cleanup(); return ErrUnsafeFile } + fail := func() error { return cleanupDarwinStage(dir, stage, &staged, closeStage) } if err := stageFile.Chmod(mode); err != nil { return fail() } diff --git a/tools/thothctl/internal/safeio/files_darwin_test.go b/tools/thothctl/internal/safeio/files_darwin_test.go new file mode 100644 index 00000000..fba3b5cb --- /dev/null +++ b/tools/thothctl/internal/safeio/files_darwin_test.go @@ -0,0 +1,44 @@ +//go:build darwin + +package safeio + +import ( + "errors" + "os" + "path/filepath" + "testing" + + "golang.org/x/sys/unix" +) + +func TestWriteCanonicalExclusiveClassifiesUnlinkFailureAsIndeterminate(t *testing.T) { + root := t.TempDir() + stage := filepath.Join(root, "stage") + if err := os.WriteFile(stage, []byte("candidate"), 0o600); err != nil { + t.Fatal(err) + } + dir, err := unix.Open(root, unix.O_RDONLY|unix.O_DIRECTORY, 0) + if err != nil { + t.Fatal(err) + } + defer unix.Close(dir) + fd, err := unix.Openat(dir, "stage", unix.O_RDWR, 0) + if err != nil { + t.Fatal(err) + } + file := os.NewFile(uintptr(fd), "stage") + if file == nil { + t.Fatal("os.NewFile returned nil") + } + defer file.Close() + var staged unix.Stat_t + if err := unix.Fstat(fd, &staged); err != nil { + t.Fatal(err) + } + oldUnlink := darwinUnlinkat + t.Cleanup(func() { darwinUnlinkat = oldUnlink }) + darwinUnlinkat = func(int, string, int) error { return errors.New("injected unlink failure") } + if err := cleanupDarwinStage(dir, "stage", &staged, file.Close); !errors.Is(err, ErrIndeterminateFile) { + t.Fatalf("unlink failure = %v, want ErrIndeterminateFile", err) + } +} diff --git a/tools/thothctl/internal/safeio/files_linux_test.go b/tools/thothctl/internal/safeio/files_linux_test.go new file mode 100644 index 00000000..e1326677 --- /dev/null +++ b/tools/thothctl/internal/safeio/files_linux_test.go @@ -0,0 +1,46 @@ +//go:build linux + +package safeio + +import ( + "errors" + "os" + "path/filepath" + "testing" +) + +func TestReadCanonicalRegularRejectsReplacementDuringRead(t *testing.T) { + root := t.TempDir() + path := filepath.Join(root, "schema.sql") + replacement := filepath.Join(root, "replacement.sql") + parked := filepath.Join(root, "parked.sql") + contents := make([]byte, 64<<20) + if err := os.WriteFile(path, contents, 0o600); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(replacement, contents, 0o600); err != nil { + t.Fatal(err) + } + + entered := make(chan struct{}) + proceed := make(chan struct{}) + beforeBoundedRead = func() { + close(entered) + <-proceed + } + t.Cleanup(func() { beforeBoundedRead = nil }) + result := make(chan error, 1) + go func() { _, err := ReadCanonicalRegular(path, int64(len(contents))); result <- err }() + <-entered + if err := os.Rename(path, parked); err != nil { + t.Fatal(err) + } + if err := os.Rename(replacement, path); err != nil { + t.Fatal(err) + } + close(proceed) + err := <-result + if !errors.Is(err, ErrUnsafeFile) { + t.Fatalf("replacement during read error = %v, want ErrUnsafeFile", err) + } +} diff --git a/tools/thothctl/internal/safeio/files_unix_test.go b/tools/thothctl/internal/safeio/files_unix_test.go index c9e16090..27b96ab1 100644 --- a/tools/thothctl/internal/safeio/files_unix_test.go +++ b/tools/thothctl/internal/safeio/files_unix_test.go @@ -1,14 +1,12 @@ -//go:build !windows +//go:build darwin || linux package safeio import ( - "bytes" "errors" "os" "path/filepath" "testing" - "time" "golang.org/x/sys/unix" ) @@ -33,56 +31,6 @@ func TestReadCanonicalRegularRejectsNamedPipeWithoutBlocking(t *testing.T) { } } -func TestReadCanonicalRegularRejectsReplacementDuringRead(t *testing.T) { - root := t.TempDir() - path := filepath.Join(root, "schema.sql") - replacement := filepath.Join(root, "replacement.sql") - parked := filepath.Join(root, "parked.sql") - contents := bytes.Repeat([]byte("x"), 64<<20) - if err := os.WriteFile(path, contents, 0o600); err != nil { - t.Fatal(err) - } - if err := os.WriteFile(replacement, contents, 0o600); err != nil { - t.Fatal(err) - } - - var caught bool - for attempt := 0; attempt < 3 && !caught; attempt++ { - result := make(chan error, 1) - go func() { - _, err := ReadCanonicalRegular(path, int64(len(contents))) - result <- err - }() - time.Sleep(time.Millisecond) - var finalErr error - for i := 0; i < 20; i++ { - if err := os.Rename(path, parked); err == nil { - if err := os.Rename(replacement, path); err != nil { - t.Fatal(err) - } - time.Sleep(time.Millisecond) - if err := os.Rename(parked, replacement); err != nil { - t.Fatal(err) - } - } - select { - case finalErr = <-result: - i = 20 - default: - } - } - if finalErr == nil { - finalErr = <-result - } - if errors.Is(finalErr, ErrUnsafeFile) { - caught = true - } - } - if !caught { - t.Fatal("replacement during read was not rejected") - } -} - func TestCanonicalDescriptorOwnershipDoesNotLeakAcrossNestedOperations(t *testing.T) { root, err := filepath.EvalSymlinks(t.TempDir()) if err != nil { diff --git a/tools/thothctl/internal/safeio/files_unsupported_unix.go b/tools/thothctl/internal/safeio/files_unsupported_unix.go new file mode 100644 index 00000000..f041f7be --- /dev/null +++ b/tools/thothctl/internal/safeio/files_unsupported_unix.go @@ -0,0 +1,13 @@ +//go:build !windows && !linux && !darwin + +package safeio + +import "io/fs" + +// Unsupported Unix targets fail closed rather than borrowing a platform-specific +// publication primitive. Add a dedicated implementation only after auditing that +// target's namespace and no-replace guarantees. +func ReadCanonicalRegular(string, int64) ([]byte, error) { return nil, ErrUnsafeFile } +func writeCanonicalExclusive(string, []byte, fs.FileMode) error { return ErrUnsafeFile } +func validateCanonicalOutputPath(string) error { return ErrUnsafeFile } +func validatePlatformPathSyntax(string) error { return nil } diff --git a/tools/thothctl/internal/safeio/files_windows.go b/tools/thothctl/internal/safeio/files_windows.go index d65fbfb6..abbb9ab1 100644 --- a/tools/thothctl/internal/safeio/files_windows.go +++ b/tools/thothctl/internal/safeio/files_windows.go @@ -131,6 +131,22 @@ func closeWindowsHandles(handles []windows.Handle) { } } +var windowsDeleteHandle = deleteWindowsHandle +var windowsCloseHandle = windows.CloseHandle +var windowsCloseStage = func(file *os.File) error { return file.Close() } + +func cleanupWindowsStage(handle windows.Handle, closeStage func() error) error { + disposeErr := windowsDeleteHandle(handle) + closeErr := closeStage() + if disposeErr == nil && closeErr == nil { + // A successful disposition plus close proves the named stage is gone. + return ErrUnsafeFile + } + // Either a failed disposition or an uncertain close leaves the named + // candidate's reachability unresolved. Never classify this as retry-safe. + return ErrIndeterminateFile +} + func writeCanonicalExclusive(path string, contents []byte, mode fs.FileMode) error { if err := ValidateCanonicalPath(path); err != nil || len(contents) > 16<<20 || mode.Perm() != 0o600 { return ErrUnsafeFile @@ -160,8 +176,11 @@ func writeCanonicalExclusive(path string, contents []byte, mode fs.FileMode) err } stageFile := os.NewFile(uintptr(stageHandle), "thothctl-safeio-stage") if stageFile == nil { - _ = deleteWindowsHandle(stageHandle) - _ = windows.CloseHandle(stageHandle) + disposeErr := windowsDeleteHandle(stageHandle) + closeErr := windowsCloseHandle(stageHandle) + if disposeErr != nil || closeErr != nil { + return ErrIndeterminateFile + } return ErrUnsafeFile } closed := false @@ -170,14 +189,14 @@ func writeCanonicalExclusive(path string, contents []byte, mode fs.FileMode) err return nil } closed = true - return stageFile.Close() + return windowsCloseStage(stageFile) } defer func() { _ = closeStage() }() var staged windows.ByHandleFileInformation if err := windows.GetFileInformationByHandle(stageHandle, &staged); err != nil || staged.NumberOfLinks != 1 || staged.FileAttributes&windows.FILE_ATTRIBUTE_REPARSE_POINT != 0 || staged.FileAttributes&windows.FILE_ATTRIBUTE_DIRECTORY != 0 { - return failWindowsStage(stageHandle, closeStage) + return cleanupWindowsStage(stageHandle, closeStage) } - fail := func() error { _ = deleteWindowsHandle(stageHandle); _ = closeStage(); return ErrUnsafeFile } + fail := func() error { return cleanupWindowsStage(stageHandle, closeStage) } if n, err := stageFile.Write(contents); err != nil || n != len(contents) { return fail() } @@ -199,13 +218,6 @@ func writeCanonicalExclusive(path string, contents []byte, mode fs.FileMode) err return nil } -// failWindowsStage disposes an exact handle when initial identity inspection fails. -func failWindowsStage(handle windows.Handle, closeStage func() error) error { - _ = deleteWindowsHandle(handle) - _ = closeStage() - return ErrUnsafeFile -} - func deleteWindowsHandle(handle windows.Handle) error { var disposition byte = 1 return windows.SetFileInformationByHandle(handle, windows.FileDispositionInfo, &disposition, uint32(unsafe.Sizeof(disposition))) diff --git a/tools/thothctl/internal/safeio/files_windows_test.go b/tools/thothctl/internal/safeio/files_windows_test.go index ae4325f1..3c1b567d 100644 --- a/tools/thothctl/internal/safeio/files_windows_test.go +++ b/tools/thothctl/internal/safeio/files_windows_test.go @@ -125,6 +125,34 @@ func TestWindowsStageHandleDeniesReadRenameDeleteAndHardlink(t *testing.T) { } } +func TestWriteCanonicalExclusiveClassifiesDispositionFailureAsIndeterminate(t *testing.T) { + root := t.TempDir() + path := filepath.Join(root, "candidate.yaml") + if err := os.WriteFile(path, []byte("existing"), 0o600); err != nil { + t.Fatal(err) + } + oldDelete, oldClose := windowsDeleteHandle, windowsCloseStage + t.Cleanup(func() { windowsDeleteHandle, windowsCloseStage = oldDelete, oldClose }) + windowsDeleteHandle = func(windows.Handle) error { return errors.New("injected disposition failure") } + if err := writeCanonicalExclusive(path, []byte("candidate"), 0o600); !errors.Is(err, ErrIndeterminateFile) { + t.Fatalf("disposition failure = %v, want ErrIndeterminateFile", err) + } +} + +func TestWriteCanonicalExclusiveClassifiesCloseFailureAsIndeterminate(t *testing.T) { + root := t.TempDir() + path := filepath.Join(root, "candidate.yaml") + if err := os.WriteFile(path, []byte("existing"), 0o600); err != nil { + t.Fatal(err) + } + oldDelete, oldClose := windowsDeleteHandle, windowsCloseStage + t.Cleanup(func() { windowsDeleteHandle, windowsCloseStage = oldDelete, oldClose }) + windowsCloseStage = func(*os.File) error { return errors.New("injected close failure") } + if err := writeCanonicalExclusive(path, []byte("candidate"), 0o600); !errors.Is(err, ErrIndeterminateFile) { + t.Fatalf("close failure = %v, want ErrIndeterminateFile", err) + } +} + func TestWriteCanonicalExclusiveRequiresRestrictiveMode(t *testing.T) { if err := writeCanonicalExclusive(`C:\\tmp\\thothctl-output.yaml`, []byte("x"), 0o640); err == nil { t.Fatal("accepted non-restrictive output mode") diff --git a/tools/thothctl/internal/workspaceops/operations.go b/tools/thothctl/internal/workspaceops/operations.go index d5ffd4c7..918987fa 100644 --- a/tools/thothctl/internal/workspaceops/operations.go +++ b/tools/thothctl/internal/workspaceops/operations.go @@ -654,6 +654,9 @@ func publishCandidateBytes(x *hostExport, result Result, path string, b []byte) return nil } if err := safeio.WriteCanonicalExclusive(path, b, 0o600); err != nil { + if errors.Is(err, safeio.ErrIndeterminateFile) { + return errors.New("candidate output state is indeterminate; reconcile the destination and private stages before retrying") + } return errors.New("unsafe output file") } return nil