From f340bd8986b8ee4f25e2bdc8e0accf25b00beec3 Mon Sep 17 00:00:00 2001 From: Bo Date: Wed, 9 Sep 2026 17:53:22 -0400 Subject: [PATCH 1/4] Improve CLI search, checkpoint recovery, and external evidence status --- cli/cmd/ao/capabilities_truthfulness_test.go | 2 +- cli/cmd/ao/status_composition_test.go | 78 +++++++ cli/docs/COMMANDS.md | 7 + cli/internal/commands/status/module.go | 34 ++- cli/internal/commands/status/module_test.go | 163 +++++++++++++- cli/internal/provenanceapp/mine_session.go | 31 ++- .../provenanceapp/mine_session_test.go | 89 ++++++++ .../provenanceapp/mine_session_unix_test.go | 213 ++++++++++++++++++ cli/internal/skills/find.go | 13 +- cli/internal/skills/find_test.go | 50 ++++ cli/internal/statusapp/statusapp.go | 52 ++++- cli/internal/statusapp/statusapp_test.go | 116 ++++++++++ docs/architecture/go-cli.md | 8 + 13 files changed, 830 insertions(+), 26 deletions(-) create mode 100644 cli/cmd/ao/status_composition_test.go create mode 100644 cli/internal/provenanceapp/mine_session_unix_test.go diff --git a/cli/cmd/ao/capabilities_truthfulness_test.go b/cli/cmd/ao/capabilities_truthfulness_test.go index 8d4828421..2c3f70034 100644 --- a/cli/cmd/ao/capabilities_truthfulness_test.go +++ b/cli/cmd/ao/capabilities_truthfulness_test.go @@ -35,7 +35,7 @@ func TestTruthfulnessSixCommandSliceReportsRealContracts(t *testing.T) { }{ {"ao version", "arbitrary", "text", "pure", map[string]string{"0": "success", "1": "failure"}}, {"ao capabilities", "none", "structured", "pure", map[string]string{"0": "success", "1": "failure"}}, - {"ao status", "arbitrary", "text", "filesystem,clock", map[string]string{"0": "success", "1": "failure"}}, + {"ao status", "arbitrary", "text", "filesystem,environment,clock", map[string]string{"0": "success", "1": "failure"}}, {"ao doctor", "no-args", "none", "filesystem,process,network,environment,clock", map[string]string{"0": "success", "1": "failure"}}, {"ao gate check", "no-args", "text", "filesystem,process,environment,clock", map[string]string{"0": "success", "1": "failure", "2": "invalid-configuration"}}, {"ao config", "none", "text", "filesystem,environment", map[string]string{"0": "success", "1": "failure"}}, diff --git a/cli/cmd/ao/status_composition_test.go b/cli/cmd/ao/status_composition_test.go new file mode 100644 index 000000000..25286baeb --- /dev/null +++ b/cli/cmd/ao/status_composition_test.go @@ -0,0 +1,78 @@ +package main + +import ( + "context" + "encoding/json" + "os" + "os/exec" + "path/filepath" + "strings" + "testing" + "time" + + "github.com/boshu2/agentops/cli/internal/evidence" + "gopkg.in/yaml.v3" +) + +func TestStatusExplicitEvidenceRootComposed(t *testing.T) { + bin, root, cwd := aoBinary(t), t.TempDir(), t.TempDir() + if _, err := evidence.SnapshotIntent(root, []byte("selected evidence")); err != nil { + t.Fatal(err) + } + config := filepath.Join(t.TempDir(), "config.yaml") + if err := os.WriteFile(config, []byte("{}\n"), 0600); err != nil { + t.Fatal(err) + } + t.Setenv("AGENTOPS_CONFIG", config) + t.Setenv("AGENTOPS_OUTPUT", "table") + for _, format := range []string{"text", "json", "yaml"} { + args := []string{"status", "--evidence-root", root} + if format != "text" { + args = append(args, "-o", format) + } + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + cmd := exec.CommandContext(ctx, bin, args...) + cmd.Dir = cwd + out, err := cmd.CombinedOutput() + cancel() + if err != nil { + t.Fatalf("status %s: %v\n%s", format, err, out) + } + if format == "text" { + if !strings.Contains(string(out), "Artifacts: 1 intents, 0 verdicts") { + t.Fatalf("text selected wrong evidence: %s", out) + } + continue + } + var report map[string]map[string]any + if format == "json" { + err = json.Unmarshal(out, &report) + } else { + err = yaml.Unmarshal(out, &report) + } + if err != nil { + t.Fatal(err) + } + loop := report["loop_evidence"] + want := any(float64(1)) + if format == "yaml" { + want = 1 + } + if loop["intent_artifacts"] != want || loop["state"] != "intent_is_latest_evidence" { + t.Fatalf("%s selected wrong evidence: %s", format, out) + } + } +} + +func TestStatusExplicitEvidenceRootCapability(t *testing.T) { + entry := capabilityEntry(t, "ao status") + for _, flag := range entry.Flags { + if flag.Name == "evidence-root" { + if flag.Required || flag.Origin != "local" || entry.Effects != "filesystem,environment,clock" { + t.Fatalf("status root contract: %+v, %+v", flag, entry) + } + return + } + } + t.Fatal("status capabilities omit --evidence-root") +} diff --git a/cli/docs/COMMANDS.md b/cli/docs/COMMANDS.md index 78354187f..315bdb985 100644 --- a/cli/docs/COMMANDS.md +++ b/cli/docs/COMMANDS.md @@ -243,6 +243,13 @@ Display the content-addressed intent and verdict evidence stored by AgentOps. ao status [flags] ``` +**Flags:** + +``` + --evidence-root string Existing explicit non-Git evidence directory (default: working directory's .agents/ao) + -h, --help help for status +``` + --- ### `ao version` diff --git a/cli/internal/commands/status/module.go b/cli/internal/commands/status/module.go index e0a3bfa84..7a28c3435 100644 --- a/cli/internal/commands/status/module.go +++ b/cli/internal/commands/status/module.go @@ -24,15 +24,16 @@ func NewModule(host clicontract.HostOptions) Module { // Contract declares status's real behavior: it accepts (and ignores) arbitrary // positional args exactly as Cobra does today, emits text (JSON under -o json), -// reads the durable evidence stores on the filesystem and stamps recency from -// the clock, and exits 0 on success or 1 on a working-directory failure. +// reads the durable evidence stores and active Git environment boundaries, +// stamps recency from the clock, and exits 0 on success or 1 on invalid input +// or a root-resolution failure. func (Module) Contract() clicontract.CommandContract { return clicontract.CommandContract{ ID: "ao.status", Profiles: clicontract.ProfileDefault | clicontract.ProfileLegacy | clicontract.ProfileCombined, Args: clicontract.ArgsPolicy{Name: "arbitrary", Validate: cobra.ArbitraryArgs}, Output: clicontract.OutputText, - Effects: clicontract.EffectFilesystem | clicontract.EffectClock, + Effects: clicontract.EffectFilesystem | clicontract.EffectEnvironment | clicontract.EffectClock, ExitClasses: map[int]clicontract.ExitClass{ 0: clicontract.ExitSuccess, 1: clicontract.ExitFailure, @@ -43,7 +44,8 @@ func (Module) Contract() clicontract.CommandContract { // Command builds the `ao status` command. The RunE closure delegates entirely // to statusapp so this module performs no direct effect. func (module Module) Command() *cobra.Command { - return &cobra.Command{ + var evidenceRoot string + command := &cobra.Command{ Use: "status", Short: "Show durable AgentOps loop evidence", Long: `Display the content-addressed intent and verdict evidence stored by AgentOps. @@ -52,25 +54,37 @@ The command validates artifact names, content identity, and verdict.v2 shape before counting evidence. It reports recency only; it does not infer an active runtime phase, elapsed execution time, tool activity, retries, or remaining work. +Without --evidence-root, read .agents/ao under the working directory. +With --evidence-root, inspect only intents/sha256 and verdicts/sha256 in that +existing non-Git directory. Invalid roots fail without fallback or writes; +evidence directory and file symlinks are excluded from explicit-root inspection. + Examples: ao status - ao status --json`, + ao status --json + ao status --evidence-root /path/to/evidence --json`, GroupID: "core", RunE: func(cmd *cobra.Command, _ []string) error { + opts := statusapp.RunOptions{ + JSON: module.host.OutputMode() == "json", Stdout: cmd.OutOrStdout(), + } + if cmd.Flags().Changed("evidence-root") { + opts.EvidenceRoot = &evidenceRoot + } if module.host.OutputMode() == "yaml" { // statusapp emits exactly one JSON document to Stdout in JSON // mode; capture it and re-emit as YAML so -o yaml is the same // data yaml-marshaled rather than a silent human-table fallback. var buf bytes.Buffer - if err := statusapp.Run(statusapp.RunOptions{JSON: true, Stdout: &buf}); err != nil { + opts.JSON, opts.Stdout = true, &buf + if err := statusapp.Run(opts); err != nil { return err } return clicontract.JSONToYAML(cmd.OutOrStdout(), buf.Bytes()) } - return statusapp.Run(statusapp.RunOptions{ - JSON: module.host.OutputMode() == "json", - Stdout: cmd.OutOrStdout(), - }) + return statusapp.Run(opts) }, } + command.Flags().StringVar(&evidenceRoot, "evidence-root", "", "Existing explicit non-Git evidence directory (default: working directory's .agents/ao)") + return command } diff --git a/cli/internal/commands/status/module_test.go b/cli/internal/commands/status/module_test.go index 1678ad597..3e9654ac7 100644 --- a/cli/internal/commands/status/module_test.go +++ b/cli/internal/commands/status/module_test.go @@ -7,10 +7,14 @@ import ( "encoding/json" "os" "path/filepath" + "reflect" "strings" "testing" "github.com/boshu2/agentops/cli/internal/clicontract" + "github.com/boshu2/agentops/cli/internal/evidence" + "github.com/boshu2/agentops/cli/internal/statusapp" + "gopkg.in/yaml.v3" ) // newTestModule builds the status module with a fixed output mode, constructing @@ -19,6 +23,33 @@ func newTestModule(outputMode string) Module { return NewModule(clicontract.HostOptions{OutputMode: func() string { return outputMode }}) } +func TestModule_ExplicitEvidenceRootFromUnrelatedDirectory(t *testing.T) { + root := t.TempDir() + if _, err := evidence.SnapshotIntent(root, []byte("selected external intent")); err != nil { + t.Fatal(err) + } + cwd := t.TempDir() + writeIntentArtifact(t, cwd, "unrelated intent one") + writeIntentArtifact(t, cwd, "unrelated intent two") + t.Chdir(cwd) + + var buf bytes.Buffer + command := newTestModule("json").Command() + command.SetOut(&buf) + command.SetErr(&buf) + command.SetArgs([]string{"--evidence-root", root}) + if err := command.Execute(); err != nil { + t.Fatalf("status --evidence-root: %v\n%s", err, buf.String()) + } + var got statusapp.Output + if err := json.Unmarshal(buf.Bytes(), &got); err != nil { + t.Fatal(err) + } + if got.LoopEvidence.IntentArtifacts != 1 || got.LoopEvidence.VerdictArtifacts != 0 { + t.Fatalf("selected evidence counts: %+v", got.LoopEvidence) + } +} + func TestModule_Contract(t *testing.T) { contract := newTestModule("text").Contract() if contract.ID != "ao.status" { @@ -27,9 +58,137 @@ func TestModule_Contract(t *testing.T) { if contract.Output != clicontract.OutputText { t.Fatalf("output = %v, want OutputText", contract.Output) } - if contract.Effects&clicontract.EffectFilesystem == 0 || contract.Effects&clicontract.EffectClock == 0 { - t.Fatalf("effects = %v, want filesystem+clock", contract.Effects) + if contract.Effects != clicontract.EffectFilesystem|clicontract.EffectEnvironment|clicontract.EffectClock { + t.Fatalf("effects = %v, want filesystem+environment+clock", contract.Effects) + } +} + +func TestModule_ExplicitRootFormatsPreserveStructuralBoundary(t *testing.T) { + root := t.TempDir() + stored := storeExternalVerdict(t, root) + // Unrelated files and nested legacy stores must not join the selected stores. + writeIntentArtifact(t, root, "nested legacy intent") + if err := os.WriteFile(filepath.Join(root, "notes.json"), []byte("not evidence"), 0600); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(filepath.Dir(stored.Path), strings.Repeat("a", 64)+".json"), []byte("{}"), 0600); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(filepath.Dir(stored.IntentRef), "notes.txt"), []byte("not an intent"), 0600); err != nil { + t.Fatal(err) + } + t.Chdir(t.TempDir()) + var reports []map[string]any + for _, format := range []string{"text", "json", "yaml"} { + var buf bytes.Buffer + command := newTestModule(format).Command() + command.SetOut(&buf) + command.SetArgs([]string{"--evidence-root", root}) + if err := command.Execute(); err != nil { + t.Fatalf("%s: %v", format, err) + } + if format == "text" { + for _, want := range []string{"Artifacts: 1 intents, 1 verdicts", "Corrupt:", "Not checked:", "caller-supplied subject manifests"} { + if !strings.Contains(buf.String(), want) { + t.Errorf("text missing %q: %s", want, buf.String()) + } + } + continue + } + var report map[string]any + if format == "yaml" { + var value any + if err := yaml.Unmarshal(buf.Bytes(), &value); err != nil { + t.Fatal(err) + } + payload, err := json.Marshal(value) + if err != nil { + t.Fatal(err) + } + buf.Reset() + buf.Write(payload) + } + if err := json.Unmarshal(buf.Bytes(), &report); err != nil { + t.Fatal(err) + } + loop := report["loop_evidence"].(map[string]any) + if loop["intent_artifacts"] != float64(1) || loop["verdict_artifacts"] != float64(1) || len(loop["corrupt"].([]any)) != 2 { + t.Fatalf("%s counts or structural validation: %+v", format, loop) + } + if !reflect.DeepEqual(loop["checked"], []any{"intents/sha256", "verdicts/sha256"}) || len(loop["not_checked"].([]any)) != 5 { + t.Fatalf("%s inspection boundary: %+v", format, loop) + } + reports = append(reports, report) + } + if !reflect.DeepEqual(reports[0], reports[1]) { + t.Fatalf("JSON/YAML reports differ: %+v", reports) + } +} + +func TestModule_InvalidExplicitRootNeverFallsBack(t *testing.T) { + cwd := t.TempDir() + writeIntentArtifact(t, cwd, "fallback must not be inspected") + t.Chdir(cwd) + gitRoot := t.TempDir() + if err := os.Mkdir(filepath.Join(gitRoot, ".git"), 0700); err != nil { + t.Fatal(err) + } + file := filepath.Join(t.TempDir(), "file") + if err := os.WriteFile(file, []byte("not a directory"), 0600); err != nil { + t.Fatal(err) + } + for _, root := range []string{"", " ", filepath.Join(t.TempDir(), "missing"), gitRoot, file} { + for _, format := range []string{"text", "json", "yaml"} { + var buf bytes.Buffer + command := newTestModule(format).Command() + command.SilenceUsage, command.SilenceErrors = true, true + command.SetOut(&buf) + command.SetErr(&buf) + command.SetArgs([]string{"--evidence-root", root}) + if err := command.Execute(); err == nil || !strings.Contains(err.Error(), "invalid --evidence-root") { + t.Fatalf("root %q, %s: %v", root, format, err) + } + if buf.Len() != 0 { + t.Fatalf("invalid root emitted fallback report: %s", buf.String()) + } + } + } +} + +func storeExternalVerdict(t *testing.T, root string) *evidence.StoreResult { + t.Helper() + subject := t.TempDir() + if err := os.WriteFile(filepath.Join(subject, "value"), []byte("synthetic subject"), 0600); err != nil { + t.Fatal(err) + } + manifest, err := evidence.BuildManifest(subject, []string{"value"}, nil, nil, nil) + if err != nil { + t.Fatal(err) + } + manifestPath, err := evidence.StoreDocument(root, "manifest.json", manifest) + if err != nil { + t.Fatal(err) + } + draft := map[string]any{ + "verdict": "PASS", "criteria": []any{map[string]any{"id": "synthetic", "result": "PASS", "evidence_refs": []string{"unresolved:synthetic-check"}}}, + "findings": []any{}, "evidence_refs": []string{"unresolved:synthetic-check"}, "checked": []string{"value"}, "not_checked": []string{}, "validated_at": "2026-07-14T00:00:00Z", + } + draftPath, err := evidence.StoreDocument(root, "draft.json", draft) + if err != nil { + t.Fatal(err) + } + intent, err := evidence.SnapshotIntent(root, []byte("synthetic acceptance")) + if err != nil { + t.Fatal(err) + } + stored, err := evidence.StoreVerdict(evidence.StoreOptions{ + Root: subject, EvidenceRoot: root, SubjectManifest: manifestPath, Draft: draftPath, IntentSource: intent.IntentRef, + Facts: evidence.RuntimeFacts{AuthorContextID: "synthetic-author", ValidatorContextID: "synthetic-judge", FreshnessSource: "runtime", FreshnessAttesterID: "synthetic-runtime", ScopeResult: "PASS"}, + }) + if err != nil { + t.Fatal(err) } + return stored } func TestModule_CommandAttributes(t *testing.T) { diff --git a/cli/internal/provenanceapp/mine_session.go b/cli/internal/provenanceapp/mine_session.go index 1e2f08f38..b4aa46b5b 100644 --- a/cli/internal/provenanceapp/mine_session.go +++ b/cli/internal/provenanceapp/mine_session.go @@ -15,6 +15,7 @@ import ( "strings" "github.com/boshu2/agentops/cli/internal/parser" + "github.com/boshu2/agentops/cli/internal/storage" ) // MineEventSchemaVersion is the schema version for mined per-inference events. @@ -85,6 +86,11 @@ func MineSession(opts MineOptions, out io.Writer) error { startAfter := 0 // mine messages with SourceLine > startAfter var prior *mineState if mineStatePath != "" { + // Reject special entries before reading: opening a FIFO could block, + // and replacing a symlink would change where its checkpoint is stored. + if _, err := mineStateMode(mineStatePath); err != nil { + return fmt.Errorf("mine-session: inspect state %s: %w", mineStatePath, err) + } if b, rerr := os.ReadFile(mineStatePath); rerr == nil { var st mineState if json.Unmarshal(b, &st) == nil { @@ -276,8 +282,27 @@ func writeMineState(path string, st mineState) error { if err != nil { return err } - if dir := filepath.Dir(path); dir != "" && dir != "." { - _ = os.MkdirAll(dir, 0o755) + mode, err := mineStateMode(path) + if err != nil { + return err + } + // Pre-rename failures preserve the previous checkpoint. A subsequent + // directory-sync error can report failure with the new checkpoint visible. + return storage.AtomicWriteFile(path, b, mode) +} + +func mineStateMode(path string) (os.FileMode, error) { + info, err := os.Lstat(path) + if os.IsNotExist(err) { + // AtomicWriteFile explicitly chmods its temporary file. Keep a new + // checkpoint private instead of bypassing a restrictive caller umask. + return 0o600, nil + } + if err != nil { + return 0, err + } + if !info.Mode().IsRegular() { + return 0, fmt.Errorf("checkpoint must be a regular file: %s", path) } - return os.WriteFile(path, b, 0o644) + return info.Mode().Perm(), nil } diff --git a/cli/internal/provenanceapp/mine_session_test.go b/cli/internal/provenanceapp/mine_session_test.go index dc728f842..c54378a3a 100644 --- a/cli/internal/provenanceapp/mine_session_test.go +++ b/cli/internal/provenanceapp/mine_session_test.go @@ -4,6 +4,7 @@ package provenanceapp import ( "bytes" "encoding/json" + "errors" "os" "path/filepath" "strings" @@ -371,3 +372,91 @@ func TestMineSession_IncrementalIdempotentRollback(t *testing.T) { } } } + +func TestWriteMineState_RegularFileBytes(t *testing.T) { + const want = "{\n \"file\": \"session.jsonl\",\n \"last_line\": 2,\n \"prefix_checksum\": \"abcdef1234567890\",\n \"mined_count\": 1\n}" + for _, existing := range []bool{false, true} { + name := "new_nested_checkpoint" + if existing { + name = "replace_regular_checkpoint" + } + t.Run(name, func(t *testing.T) { + path := filepath.Join(t.TempDir(), "nested", "state.json") + if existing { + if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(path, []byte("previous checkpoint"), 0o600); err != nil { + t.Fatal(err) + } + } + if err := writeMineState(path, mineState{File: "session.jsonl", LastLine: 2, PrefixChecksum: "abcdef1234567890", MinedCount: 1}); err != nil { + t.Fatal(err) + } + got, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + if string(got) != want { + t.Fatalf("checkpoint bytes = %q, want %q", got, want) + } + }) + } +} + +func TestWriteMineState_ParentDirectoryFailure(t *testing.T) { + parent := filepath.Join(t.TempDir(), "file-not-directory") + if err := os.WriteFile(parent, []byte("preserve parent"), 0o600); err != nil { + t.Fatal(err) + } + err := writeMineState(filepath.Join(parent, "nested", "state.json"), mineState{}) + var pathErr *os.PathError + if !errors.As(err, &pathErr) { + t.Fatalf("parent failure must remain a filesystem error, got %v", err) + } + got, err := os.ReadFile(parent) + if err != nil || string(got) != "preserve parent" { + t.Fatalf("parent changed: %q, %v", got, err) + } +} + +func TestMineSession_CheckpointDirectoryRejectedThenRetry(t *testing.T) { + dir := t.TempDir() + sess := writeMineSession(t, dir, "s.jsonl", "{\"type\":\"tool_use\",\"tool_name\":\"Read\",\"tool_input\":{}}\n") + state := filepath.Join(dir, "state.json") + if err := os.Mkdir(state, 0o700); err != nil { + t.Fatal(err) + } + marker := filepath.Join(state, "keep") + if err := os.WriteFile(marker, []byte("preserved"), 0o600); err != nil { + t.Fatal(err) + } + out, err := mine(t, MineOptions{File: sess, State: state}) + if err == nil || !strings.Contains(err.Error(), "checkpoint must be a regular file") || out != "" { + t.Fatalf("directory state must fail before emission: output %q, error %v", out, err) + } + if got, err := os.ReadFile(marker); err != nil || string(got) != "preserved" { + t.Fatalf("rejected directory content changed: %q, %v", got, err) + } + if err := os.Remove(marker); err != nil { + t.Fatal(err) + } + if err := os.Remove(state); err != nil { + t.Fatal(err) + } + want, err := mine(t, MineOptions{File: sess}) + if err != nil { + t.Fatal(err) + } + if events := parseMineEvents(t, want); len(events) != 1 || events[0].Tool != "Read" { + t.Fatalf("expected one uncheckpointed Read event: %+v", events) + } + retry, err := mine(t, MineOptions{File: sess, State: state}) + if err != nil || retry != want { + t.Fatalf("retry changed event bytes or stable IDs: got %q, want %q, error %v", retry, want, err) + } + again, err := mine(t, MineOptions{File: sess, State: state}) + if err != nil || again != "" { + t.Fatalf("successful retry did not checkpoint: %q, %v", again, err) + } +} diff --git a/cli/internal/provenanceapp/mine_session_unix_test.go b/cli/internal/provenanceapp/mine_session_unix_test.go new file mode 100644 index 000000000..f073eab5f --- /dev/null +++ b/cli/internal/provenanceapp/mine_session_unix_test.go @@ -0,0 +1,213 @@ +//go:build darwin || linux + +package provenanceapp + +import ( + "bytes" + "context" + "errors" + "fmt" + "os" + "os/exec" + "os/signal" + "path/filepath" + "strings" + "syscall" + "testing" + "time" +) + +func TestMineSession_CheckpointPartialWrite(t *testing.T) { + dir := t.TempDir() + state := filepath.Join(dir, "state.json") + sess := writeMineSession(t, dir, "session.jsonl", "{\"type\":\"tool_use\",\"tool_name\":\"Read\",\"tool_input\":{}}\n") + if _, err := mine(t, MineOptions{File: sess, State: state}); err != nil { + t.Fatal(err) + } + before, err := os.ReadFile(state) + if err != nil { + t.Fatal(err) + } + writeMineSession(t, dir, "session.jsonl", "{\"type\":\"tool_use\",\"tool_name\":\"Read\",\"tool_input\":{}}\n{\"type\":\"tool_use\",\"tool_name\":\"Bash\",\"tool_input\":{}}\n") + var parentLimit syscall.Rlimit + if err := syscall.Getrlimit(syscall.RLIMIT_FSIZE, &parentLimit); err != nil { + t.Fatal(err) + } + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + cmd := exec.CommandContext(ctx, os.Args[0], "-test.run=^TestMineSession_CheckpointPartialWriteChild$") + cmd.Env = append(os.Environ(), "AO_MINE_PARTIAL_WRITE_DIR="+dir) + var stderr bytes.Buffer + cmd.Stderr = &stderr + emitted, err := cmd.Output() + if err != nil { + t.Fatalf("partial-write child: %v; stderr: %s", err, &stderr) + } + if got := stderr.String(); got != "checkpoint write returned EFBIG\n" { + t.Fatalf("child did not observe a real write error: %q", got) + } + var afterLimit syscall.Rlimit + if err := syscall.Getrlimit(syscall.RLIMIT_FSIZE, &afterLimit); err != nil { + t.Fatal(err) + } + if afterLimit != parentLimit { + t.Errorf("parent file-size limit changed: %+v -> %+v", parentLimit, afterLimit) + } + after, err := os.ReadFile(state) + if err != nil { + t.Fatal(err) + } + if !bytes.Equal(after, before) { + t.Errorf("partial write changed valid checkpoint: before %d bytes, after %d bytes", len(before), len(after)) + } + entries, err := os.ReadDir(dir) + if err != nil { + t.Fatal(err) + } + if len(entries) != 2 || entries[0].Name() != "session.jsonl" || entries[1].Name() != "state.json" { + t.Errorf("ordinary-error cleanup left unexpected directory entries: %v", entries) + } + failedEvents := parseMineEvents(t, string(emitted)) + if len(failedEvents) != 1 || failedEvents[0].Tool != "Bash" { + t.Fatalf("failed attempt events = %+v, want one new Bash", failedEvents) + } + retry, err := mine(t, MineOptions{File: sess, State: state}) + if err != nil { + t.Fatal(err) + } + if retry != string(emitted) { + t.Errorf("retry must repeat only uncheckpointed events with identical IDs:\nfailed: %s\nretry: %s", emitted, retry) + } + again, err := mine(t, MineOptions{File: sess, State: state}) + if err != nil || again != "" { + t.Errorf("successful retry did not checkpoint: output %q, error %v", again, err) + } +} + +// The limit belongs only to this bounded child. Ignoring SIGXFSZ makes the +// kernel return a real partial-write error instead of terminating the process. +func TestMineSession_CheckpointPartialWriteChild(t *testing.T) { + dir := os.Getenv("AO_MINE_PARTIAL_WRITE_DIR") + if dir == "" { + return + } + signal.Ignore(syscall.SIGXFSZ) + var limit syscall.Rlimit + if err := syscall.Getrlimit(syscall.RLIMIT_FSIZE, &limit); err != nil { + t.Fatal(err) + } + limit.Cur = 32 + if err := syscall.Setrlimit(syscall.RLIMIT_FSIZE, &limit); err != nil { + t.Fatal(err) + } + err := MineSession(MineOptions{File: filepath.Join(dir, "session.jsonl"), State: filepath.Join(dir, "state.json"), JSON: true}, os.Stdout) + if !errors.Is(err, syscall.EFBIG) { + t.Fatalf("expected file-size write error, got %v", err) + } + fmt.Fprintln(os.Stderr, "checkpoint write returned EFBIG") + os.Exit(0) +} + +func TestWriteMineState_Permissions(t *testing.T) { + for _, mode := range []os.FileMode{0o600, 0o640, 0o644} { + t.Run(fmt.Sprintf("existing_%o", mode), func(t *testing.T) { + path := filepath.Join(t.TempDir(), "state.json") + if err := os.WriteFile(path, []byte("old"), mode); err != nil { + t.Fatal(err) + } + if err := os.Chmod(path, mode); err != nil { + t.Fatal(err) + } + if err := writeMineState(path, mineState{LastLine: 2}); err != nil { + t.Fatal(err) + } + info, err := os.Stat(path) + if err != nil { + t.Fatal(err) + } + if got := info.Mode().Perm(); got != mode { + t.Fatalf("checkpoint mode = %o, want unchanged %o", got, mode) + } + }) + } + t.Run("new_checkpoint_is_private", func(t *testing.T) { + path := filepath.Join(t.TempDir(), "state.json") + if err := writeMineState(path, mineState{}); err != nil { + t.Fatal(err) + } + info, err := os.Stat(path) + if err != nil { + t.Fatal(err) + } + if got := info.Mode().Perm(); got != 0o600 { + t.Fatalf("new checkpoint mode = %o, want 600", got) + } + }) +} + +func TestMineSession_CheckpointSymlinkRejected(t *testing.T) { + for _, dangling := range []bool{false, true} { + t.Run(fmt.Sprintf("dangling_%t", dangling), func(t *testing.T) { + dir := t.TempDir() + sess := writeMineSession(t, dir, "s.jsonl", "{\"type\":\"tool_use\",\"tool_name\":\"Read\",\"tool_input\":{}}\n") + target := filepath.Join(dir, "target.json") + if !dangling { + if _, err := mine(t, MineOptions{File: sess, State: target}); err != nil { + t.Fatal(err) + } + } + before, _ := os.ReadFile(target) + state := filepath.Join(dir, "state.json") + if err := os.Symlink(target, state); err != nil { + t.Fatal(err) + } + out, err := mine(t, MineOptions{File: sess, State: state}) + if err == nil || !strings.Contains(err.Error(), "checkpoint must be a regular file") || out != "" { + t.Fatalf("symlink checkpoint must be rejected: output %q, error %v", out, err) + } + if got, err := os.Readlink(state); err != nil || got != target { + t.Fatalf("checkpoint symlink changed: %q, %v", got, err) + } + after, err := os.ReadFile(target) + if dangling { + if !os.IsNotExist(err) { + t.Fatalf("dangling target created: %v", err) + } + } else if err != nil || !bytes.Equal(after, before) { + t.Fatalf("symlink target changed: before %q, after %q, error %v", before, after, err) + } + }) + } +} + +func TestMineSession_CheckpointFIFORejected(t *testing.T) { + if dir := os.Getenv("AO_MINE_FIFO_DIR"); dir != "" { + out, err := mine(t, MineOptions{File: filepath.Join(dir, "s.jsonl"), State: filepath.Join(dir, "state.json")}) + if err == nil || !strings.Contains(err.Error(), "checkpoint must be a regular file") || out != "" { + t.Fatalf("FIFO checkpoint must be rejected before reading: output %q, error %v", out, err) + } + return + } + dir := t.TempDir() + writeMineSession(t, dir, "s.jsonl", "{\"type\":\"tool_use\",\"tool_name\":\"Read\",\"tool_input\":{}}\n") + state := filepath.Join(dir, "state.json") + if err := syscall.Mkfifo(state, 0o600); err != nil { + t.Fatal(err) + } + // Directly test the writer too: atomic rename must not replace a FIFO. + if err := writeMineState(state, mineState{}); err == nil || !strings.Contains(err.Error(), "checkpoint must be a regular file") { + t.Fatalf("FIFO write must be rejected: %v", err) + } + // A bounded child catches regressions that would block while opening a FIFO. + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + cmd := exec.CommandContext(ctx, os.Args[0], "-test.run=^TestMineSession_CheckpointFIFORejected$") + cmd.Env = append(os.Environ(), "AO_MINE_FIFO_DIR="+dir) + if output, err := cmd.CombinedOutput(); err != nil { + t.Fatalf("FIFO reader child: %v; output: %s", err, output) + } + info, err := os.Lstat(state) + if err != nil || info.Mode()&os.ModeNamedPipe == 0 { + t.Fatalf("FIFO checkpoint replaced: %v, %v", info, err) + } +} diff --git a/cli/internal/skills/find.go b/cli/internal/skills/find.go index 736faaf67..55c777e39 100644 --- a/cli/internal/skills/find.go +++ b/cli/internal/skills/find.go @@ -229,13 +229,12 @@ func tokenStream(s string) []string { // and single-character tokens, returning a deduplicated, order-preserving // slice of meaningful tokens. func tokenize(s string) []string { - fields := strings.FieldsFunc(strings.ToLower(s), func(r rune) bool { - return !unicode.IsLetter(r) && !unicode.IsDigit(r) - }) - seen := make(map[string]bool, len(fields)) - out := make([]string, 0, len(fields)) - for _, f := range fields { - if len(f) < 2 || stopwords[f] || seen[f] { + tokens := tokenStream(s) + seen := make(map[string]bool, len(tokens)) + // Compact the normalized stream in place, keeping each token's first occurrence. + out := tokens[:0] + for _, f := range tokens { + if seen[f] { continue } seen[f] = true diff --git a/cli/internal/skills/find_test.go b/cli/internal/skills/find_test.go index fa2eefa15..2022a0606 100644 --- a/cli/internal/skills/find_test.go +++ b/cli/internal/skills/find_test.go @@ -1,6 +1,7 @@ package skills import ( + "reflect" "strings" "testing" ) @@ -119,6 +120,55 @@ func TestTokenize_DropsStopwordsAndShortTokens(t *testing.T) { } } +func TestTokenNormalization_PreservesStreamAndFirstOccurrence(t *testing.T) { + tests := []struct { + name string + input string + stream []string + unique []string + }{ + {"repetitions", "Council judge council ONE judge", []string{"council", "judge", "council", "one", "judge"}, []string{"council", "judge", "one"}}, + {"punctuation", "CHECK—this, change! check_change", []string{"check", "change", "check", "change"}, []string{"check", "change"}}, + {"unicode", "É CAFÉ 例 １２ café é", []string{"é", "café", "例", "１２", "café", "é"}, []string{"é", "café", "例", "１２"}}, + {"short ascii", "x 7 go 42 X", []string{"go", "42"}, []string{"go", "42"}}, + {"stopwords", "the a an and or of to in on for is it with by at as be this", []string{}, []string{}}, + {"empty", "", []string{}, []string{}}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := tokenStream(tt.input); !reflect.DeepEqual(got, tt.stream) { + t.Errorf("tokenStream(%q) = %v, want %v", tt.input, got, tt.stream) + } + if got := tokenize(tt.input); !reflect.DeepEqual(got, tt.unique) { + t.Errorf("tokenize(%q) = %v, want %v", tt.input, got, tt.unique) + } + }) + } +} + +func TestContainsPhrase_NormalizedContiguity(t *testing.T) { + tests := []struct { + query string + phrase string + want bool + }{ + {"checking this change", "check this change", true}, + {"change this check", "check this change", false}, + {"check another change", "check this change", false}, + {"council one council judge", "one judge", false}, + {"council one council judge", "council judge", true}, + {"council council judge", "council council", true}, + {"council judge", "council council", false}, + } + for _, tt := range tests { + t.Run(tt.query+"/"+tt.phrase, func(t *testing.T) { + if got := containsPhrase(tokenStream(tt.query), tokenStream(tt.phrase)); got != tt.want { + t.Errorf("containsPhrase(%q, %q) = %v, want %v", tt.query, tt.phrase, got, tt.want) + } + }) + } +} + func TestScore_ExclusionSentenceRoutesAway(t *testing.T) { metas := []SkillMeta{ {Name: "premortem", Description: "Fresh-judge a frozen plan. Not for a live decision's reversibility; that is one-way-door. Triggers: \"premortem\", \"challenge this plan\"."}, diff --git a/cli/internal/statusapp/statusapp.go b/cli/internal/statusapp/statusapp.go index becc92806..ebfee5e00 100644 --- a/cli/internal/statusapp/statusapp.go +++ b/cli/internal/statusapp/statusapp.go @@ -15,6 +15,7 @@ import ( "strings" "time" + "github.com/boshu2/agentops/cli/internal/evidencepath" "github.com/boshu2/agentops/cli/internal/verdictcheck" ) @@ -22,6 +23,9 @@ import ( // The working directory and clock are resolved inside Run so the module never // performs a direct filesystem or clock effect. type RunOptions struct { + // EvidenceRoot selects an existing non-Git store. Nil preserves cwd/.agents/ao; + // an explicitly empty value is invalid, rather than a request for fallback. + EvidenceRoot *string // JSON selects machine-readable output when true. JSON bool // Stdout receives the rendered report. It is the command's output stream. @@ -61,6 +65,13 @@ type evidenceSource struct { // Run resolves the working directory and clock, inventories the durable stores, // and renders the report to the configured stream. func Run(opts RunOptions) error { + if opts.EvidenceRoot != nil { + root, err := evidencepath.Validate(*opts.EvidenceRoot) + if err != nil { + return fmt.Errorf("invalid --evidence-root: %w", err) + } + return Render(opts.Stdout, opts.JSON, &Output{LoopEvidence: loadEvidence(root, root, true, time.Now())}) + } cwd, err := os.Getwd() if err != nil { return fmt.Errorf("get working directory: %w", err) @@ -71,6 +82,10 @@ func Run(opts RunOptions) error { // LoadLoopEvidence inventories only the two immutable stores AgentOps owns. // Recency describes durable evidence, never live process state or remaining work. func LoadLoopEvidence(cwd string, now time.Time) *LoopEvidenceStatus { + return loadEvidence(filepath.Join(cwd, ".agents", "ao"), cwd, false, now) +} + +func loadEvidence(root, displayRoot string, explicit bool, now time.Time) *LoopEvidenceStatus { result := &LoopEvidenceStatus{ NotChecked: []string{ "active runtime phase", @@ -82,22 +97,30 @@ func LoadLoopEvidence(cwd string, now time.Time) *LoopEvidenceStatus { } sources := []evidenceSource{ { - kind: "intent", path: filepath.Join(cwd, ".agents", "ao", "intents", "sha256"), + kind: "intent", path: filepath.Join(root, "intents", "sha256"), suffix: ".intent", count: &result.IntentArtifacts, validate: validateIntentArtifact, }, { - kind: "verdict", path: filepath.Join(cwd, ".agents", "ao", "verdicts", "sha256"), + kind: "verdict", path: filepath.Join(root, "verdicts", "sha256"), suffix: ".json", count: &result.VerdictArtifacts, validate: validateVerdictArtifact, }, } var latest time.Time for _, source := range sources { - rel, err := filepath.Rel(cwd, source.path) + rel, err := filepath.Rel(displayRoot, source.path) if err != nil { rel = source.path } result.Checked = append(result.Checked, rel) + if explicit { + if err := checkEvidenceDirectories(root, source.kind+"s"); err != nil { + if !os.IsNotExist(err) { + result.Unavailable = append(result.Unavailable, fmt.Sprintf("%s: %v", rel, err)) + } + continue + } + } entries, err := os.ReadDir(source.path) if os.IsNotExist(err) { continue @@ -116,6 +139,9 @@ func LoadLoopEvidence(cwd string, now time.Time) *LoopEvidenceStatus { continue } if !info.Mode().IsRegular() { + if explicit && info.Mode()&os.ModeSymlink != 0 { + result.Unavailable = append(result.Unavailable, filepath.Join(rel, entry.Name())+": evidence file symlink excluded") + } continue } expectedDigest, ok := artifactDigestFromName(entry.Name(), source.suffix) @@ -161,6 +187,26 @@ func LoadLoopEvidence(cwd string, now time.Time) *LoopEvidenceStatus { return result } +// Check each store component before ReadDir; Lstat never follows an evidence +// directory symlink into another store. The validated root itself is canonical. +func checkEvidenceDirectories(root, store string) error { + path := root + for _, name := range []string{store, "sha256"} { + path = filepath.Join(path, name) + info, err := os.Lstat(path) + if err != nil { + return err + } + if info.Mode()&os.ModeSymlink != 0 { + return fmt.Errorf("evidence directory symlink excluded") + } + if !info.IsDir() { + return fmt.Errorf("evidence store component is not a directory") + } + } + return nil +} + func artifactDigestFromName(name, suffix string) (string, bool) { if !strings.HasSuffix(name, suffix) { return "", false diff --git a/cli/internal/statusapp/statusapp_test.go b/cli/internal/statusapp/statusapp_test.go index e557a93a5..06f525ada 100644 --- a/cli/internal/statusapp/statusapp_test.go +++ b/cli/internal/statusapp/statusapp_test.go @@ -1,11 +1,13 @@ package statusapp import ( + "bytes" "crypto/sha256" "encoding/hex" "encoding/json" "os" "path/filepath" + "reflect" "strings" "testing" "time" @@ -13,6 +15,120 @@ import ( "github.com/boshu2/agentops/cli/internal/verdictcheck" ) +func TestRun_ExplicitRootDoesNotFollowEvidenceSymlinks(t *testing.T) { + outside := t.TempDir() + intent := writeIntentArtifact(t, outside, "outside intent must not be read") + verdict := writeVerdictArtifact(t, outside) + for _, artifact := range []string{intent, verdict} { + store := filepath.Base(filepath.Dir(filepath.Dir(artifact))) + for _, level := range []string{"store", "sha256", "file"} { + t.Run(store+"/"+level, func(t *testing.T) { + root := t.TempDir() + link, target := filepath.Join(root, store), filepath.Dir(filepath.Dir(artifact)) + if level == "sha256" { + link, target = filepath.Join(link, "sha256"), filepath.Dir(artifact) + } + if level == "file" { + link, target = filepath.Join(link, "sha256", filepath.Base(artifact)), artifact + } + if err := os.MkdirAll(filepath.Dir(link), 0700); err != nil { + t.Fatal(err) + } + if err := os.Symlink(target, link); err != nil { + t.Fatal(err) + } + got := runExplicitEvidence(t, root) + if got.IntentArtifacts != 0 || got.VerdictArtifacts != 0 || got.State != "evidence_unavailable" || len(got.Unavailable) != 1 { + t.Fatalf("symlink inspection: %+v", got) + } + if !strings.Contains(got.Unavailable[0], "symlink excluded") { + t.Fatalf("symlink not disclosed: %+v", got) + } + }) + } + } +} + +func TestRun_ExplicitRootIsReadOnly(t *testing.T) { + base := t.TempDir() + for _, name := range []string{"empty", "missing", "Git"} { + t.Run(name, func(t *testing.T) { + root := filepath.Join(base, name) + if name != "missing" { + if err := os.Mkdir(root, 0700); err != nil { + t.Fatal(err) + } + } + if name == "Git" { + if err := os.Mkdir(filepath.Join(root, ".git"), 0700); err != nil { + t.Fatal(err) + } + } + before := treeEntries(t, base) + var out bytes.Buffer + err := Run(RunOptions{EvidenceRoot: &root, JSON: true, Stdout: &out}) + if name == "empty" { + if err != nil { + t.Fatal(err) + } + var got Output + if err := json.Unmarshal(out.Bytes(), &got); err != nil || got.LoopEvidence.State != "no_evidence" { + t.Fatalf("empty explicit root: %v, %s", err, out.String()) + } + } else if err == nil || out.Len() != 0 { + t.Fatalf("invalid explicit root: %v, %s", err, out.String()) + } + if after := treeEntries(t, base); !reflect.DeepEqual(before, after) { + t.Fatalf("inspection wrote directories/files: before %v, after %v", before, after) + } + }) + } +} + +func TestRun_ExplicitRootHonorsActiveGitStorageBinding(t *testing.T) { + root := t.TempDir() + t.Setenv("GIT_OBJECT_DIRECTORY", root) + var out bytes.Buffer + err := Run(RunOptions{EvidenceRoot: &root, JSON: true, Stdout: &out}) + if err == nil || !strings.Contains(err.Error(), "overlaps declared Git storage") || out.Len() != 0 { + t.Fatalf("active Git storage guard: %v, %s", err, out.String()) + } + // The environment guard is confined to explicit-root selection; no-flag + // status retains the original cwd-based legacy behavior. + t.Chdir(t.TempDir()) + if err := Run(RunOptions{JSON: true, Stdout: &out}); err != nil { + t.Fatalf("legacy status changed: %v", err) + } +} + +func runExplicitEvidence(t *testing.T, root string) *LoopEvidenceStatus { + t.Helper() + var out bytes.Buffer + if err := Run(RunOptions{EvidenceRoot: &root, JSON: true, Stdout: &out}); err != nil { + t.Fatal(err) + } + var got Output + if err := json.Unmarshal(out.Bytes(), &got); err != nil { + t.Fatal(err) + } + return got.LoopEvidence +} + +func treeEntries(t *testing.T, root string) []string { + t.Helper() + var entries []string + if err := filepath.WalkDir(root, func(path string, entry os.DirEntry, err error) error { + if err != nil { + return err + } + entries = append(entries, path+":"+entry.Type().String()) + return nil + }); err != nil { + t.Fatal(err) + } + return entries +} + func TestFormatDurationBrief(t *testing.T) { tests := []struct { input time.Duration diff --git a/docs/architecture/go-cli.md b/docs/architecture/go-cli.md index 63cb6b669..6e559f4a7 100644 --- a/docs/architecture/go-cli.md +++ b/docs/architecture/go-cli.md @@ -106,6 +106,14 @@ A gate PASS is a deterministic fact, not a semantic verdict. but never writes verdicts and never converts check success into semantic PASS. Evidence references are reported as declared strings; `ao status` does not resolve or digest-bind their targets. + By default it reads the working directory's `.agents/ao`. To inspect existing + external evidence, use `ao status --evidence-root /path/to/evidence --json` + (or `-o yaml` for the same report). The shared `evidencepath` guard requires an + existing non-Git root and checks active Git environment storage bindings. + Only `intents/sha256` and `verdicts/sha256` are inspected; evidence directory + and file symlinks are excluded. Invalid explicit roots fail without fallback + or writes. Corrupt artifacts remain excluded and reported, and `not_checked` + continues to disclose the limits of structural inspection. ## The Learn seat (off-path) From ad6116cddcf9d18eac41769bf32de9955adc1de9 Mon Sep 17 00:00:00 2001 From: Bo Date: Wed, 9 Sep 2026 18:01:28 -0400 Subject: [PATCH 2/4] test(provenance): restore file limit before coverage flush --- cli/internal/provenanceapp/mine_session_unix_test.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/cli/internal/provenanceapp/mine_session_unix_test.go b/cli/internal/provenanceapp/mine_session_unix_test.go index f073eab5f..9fae965f6 100644 --- a/cli/internal/provenanceapp/mine_session_unix_test.go +++ b/cli/internal/provenanceapp/mine_session_unix_test.go @@ -96,11 +96,16 @@ func TestMineSession_CheckpointPartialWriteChild(t *testing.T) { if err := syscall.Getrlimit(syscall.RLIMIT_FSIZE, &limit); err != nil { t.Fatal(err) } + originalLimit := limit limit.Cur = 32 if err := syscall.Setrlimit(syscall.RLIMIT_FSIZE, &limit); err != nil { t.Fatal(err) } err := MineSession(MineOptions{File: filepath.Join(dir, "session.jsonl"), State: filepath.Join(dir, "state.json"), JSON: true}, os.Stdout) + // Coverage data is flushed at exit; only the checkpoint write is limited. + if restoreErr := syscall.Setrlimit(syscall.RLIMIT_FSIZE, &originalLimit); restoreErr != nil { + t.Fatalf("restore file-size limit: %v", restoreErr) + } if !errors.Is(err, syscall.EFBIG) { t.Fatalf("expected file-size write error, got %v", err) } From 71f1c81eea86407efaef2c48e1c7e54b7cb3d472 Mon Sep 17 00:00:00 2001 From: Bo Date: Wed, 9 Sep 2026 18:32:12 -0400 Subject: [PATCH 3/4] fix(provenance): reject checkpoint permission changes before replacement --- cli/internal/commands/provenance/module.go | 6 ++ cli/internal/provenanceapp/mine_session.go | 5 +- .../provenanceapp/mine_session_darwin_test.go | 69 ++++++++++++++++++ .../provenanceapp/mine_session_linux_test.go | 51 +++++++++++++ .../mine_session_windows_test.go | 47 ++++++++++++ .../provenanceapp/mine_state_security.go | 50 +++++++++++++ .../mine_state_security_darwin.go | 70 ++++++++++++++++++ .../mine_state_security_linux.go | 71 +++++++++++++++++++ .../mine_state_security_other.go | 13 ++++ .../mine_state_security_windows.go | 42 +++++++++++ tests/windows/test-windows-smoke.ps1 | 1 + 11 files changed, 424 insertions(+), 1 deletion(-) create mode 100644 cli/internal/provenanceapp/mine_session_darwin_test.go create mode 100644 cli/internal/provenanceapp/mine_session_linux_test.go create mode 100644 cli/internal/provenanceapp/mine_session_windows_test.go create mode 100644 cli/internal/provenanceapp/mine_state_security.go create mode 100644 cli/internal/provenanceapp/mine_state_security_darwin.go create mode 100644 cli/internal/provenanceapp/mine_state_security_linux.go create mode 100644 cli/internal/provenanceapp/mine_state_security_other.go create mode 100644 cli/internal/provenanceapp/mine_state_security_windows.go diff --git a/cli/internal/commands/provenance/module.go b/cli/internal/commands/provenance/module.go index 1c455c37f..7ef905e41 100644 --- a/cli/internal/commands/provenance/module.go +++ b/cli/internal/commands/provenance/module.go @@ -659,6 +659,12 @@ a watermark + a prefix checksum. If the transcript's already-mined prefix change (rollback) — borrowed from cass's incremental-index discipline (stale-is-usable, recover loudly, never rebuild expensive state unnecessarily). +Checkpoint updates are atomic; new files use mode 0600. Symlinks and special +files are rejected. Existing permissions must match an atomic replacement; +unverifiable or different security metadata returns an error without replacing +the checkpoint. An error syncing the directory after replacement can leave the +new checkpoint visible. Events may already have been emitted before an error. + Output (--json, default): one JSON event per line on stdout. The events feed the PROV-O graph via a downstream step (e.g. wired as an ASSAY --mine-cmd); this command does not itself write the committed ledger.`, diff --git a/cli/internal/provenanceapp/mine_session.go b/cli/internal/provenanceapp/mine_session.go index b4aa46b5b..4a242cca3 100644 --- a/cli/internal/provenanceapp/mine_session.go +++ b/cli/internal/provenanceapp/mine_session.go @@ -286,6 +286,9 @@ func writeMineState(path string, st mineState) error { if err != nil { return err } + if err := checkCheckpointReplacement(path, mode); err != nil { + return err + } // Pre-rename failures preserve the previous checkpoint. A subsequent // directory-sync error can report failure with the new checkpoint visible. return storage.AtomicWriteFile(path, b, mode) @@ -295,7 +298,7 @@ func mineStateMode(path string) (os.FileMode, error) { info, err := os.Lstat(path) if os.IsNotExist(err) { // AtomicWriteFile explicitly chmods its temporary file. Keep a new - // checkpoint private instead of bypassing a restrictive caller umask. + // checkpoint's mode restrictive instead of bypassing the caller's umask. return 0o600, nil } if err != nil { diff --git a/cli/internal/provenanceapp/mine_session_darwin_test.go b/cli/internal/provenanceapp/mine_session_darwin_test.go new file mode 100644 index 000000000..53ed944ce --- /dev/null +++ b/cli/internal/provenanceapp/mine_session_darwin_test.go @@ -0,0 +1,69 @@ +package provenanceapp + +import ( + "bytes" + "os" + "os/exec" + "path/filepath" + "strings" + "testing" +) + +func TestMineSession_CheckpointACLRejected(t *testing.T) { + dir := t.TempDir() + state := filepath.Join(dir, "state.json") + session := writeMineSession(t, dir, "session.jsonl", "{\"type\":\"tool_use\",\"tool_name\":\"Read\",\"tool_input\":{}}\n") + opts := MineOptions{File: session, State: state} + if _, err := mine(t, opts); err != nil { + t.Fatal(err) + } + if err := os.Chmod(state, 0o644); err != nil { + t.Fatal(err) + } + if out, err := exec.Command("/bin/chmod", "+a", "user:nobody deny read", state).CombinedOutput(); err != nil { + t.Fatalf("set restrictive ACL: %v: %s", err, out) + } + before, err := os.ReadFile(state) + if err != nil { + t.Fatal(err) + } + if _, err := mine(t, opts); err == nil || !strings.Contains(err.Error(), "permissions") { + t.Errorf("update with non-preservable permissions: %v", err) + } + after, err := os.ReadFile(state) + if err != nil || !bytes.Equal(before, after) { + t.Errorf("checkpoint changed: %v", err) + } + acl, err := exec.Command("/bin/ls", "-le", state).CombinedOutput() + if err != nil || !strings.Contains(string(acl), "deny read") { + t.Errorf("restrictive ACL lost: %v: %s", err, acl) + } +} + +func TestMineSession_CheckpointInheritedACLRejected(t *testing.T) { + dir := t.TempDir() + state := filepath.Join(dir, "state.json") + session := writeMineSession(t, dir, "session.jsonl", "{\"type\":\"tool_use\",\"tool_name\":\"Read\",\"tool_input\":{}}\n") + opts := MineOptions{File: session, State: state} + if _, err := mine(t, opts); err != nil { + t.Fatal(err) + } + before, err := os.ReadFile(state) + if err != nil { + t.Fatal(err) + } + if out, err := exec.Command("/bin/chmod", "+a", "user:nobody allow read,file_inherit", dir).CombinedOutput(); err != nil { + t.Fatalf("set inheritable ACL: %v: %s", err, out) + } + if _, err := mine(t, opts); err == nil || !strings.Contains(err.Error(), "permissions") { + t.Errorf("update with different inherited permissions: %v", err) + } + after, err := os.ReadFile(state) + if err != nil || !bytes.Equal(before, after) { + t.Errorf("checkpoint changed: %v", err) + } + entries, err := os.ReadDir(dir) + if err != nil || len(entries) != 2 { + t.Errorf("permission probe left files: %v, %v", entries, err) + } +} diff --git a/cli/internal/provenanceapp/mine_session_linux_test.go b/cli/internal/provenanceapp/mine_session_linux_test.go new file mode 100644 index 000000000..b86e76ac4 --- /dev/null +++ b/cli/internal/provenanceapp/mine_session_linux_test.go @@ -0,0 +1,51 @@ +package provenanceapp + +import ( + "bytes" + "encoding/binary" + "os" + "path/filepath" + "strings" + "syscall" + "testing" +) + +func TestMineSession_CheckpointACLRejected(t *testing.T) { + dir := t.TempDir() + state := filepath.Join(dir, "state.json") + session := writeMineSession(t, dir, "session.jsonl", "{\"type\":\"tool_use\",\"tool_name\":\"Read\",\"tool_input\":{}}\n") + opts := MineOptions{File: session, State: state} + if _, err := mine(t, opts); err != nil { + t.Fatal(err) + } + // Named UID 65534 has no access even though the mask/other bits allow read. + acl := binary.LittleEndian.AppendUint32(nil, 2) // POSIX_ACL_XATTR_VERSION + for _, entry := range []struct { + tag, perm uint16 + id uint32 + }{{1, 6, ^uint32(0)}, {2, 0, 65534}, {4, 4, ^uint32(0)}, {16, 4, ^uint32(0)}, {32, 4, ^uint32(0)}} { + acl = binary.LittleEndian.AppendUint16(acl, entry.tag) + acl = binary.LittleEndian.AppendUint16(acl, entry.perm) + acl = binary.LittleEndian.AppendUint32(acl, entry.id) + } + const key = "system.posix_acl_access" + if err := syscall.Setxattr(state, key, acl, 0); err != nil { + t.Fatal(err) + } + before, err := os.ReadFile(state) + if err != nil { + t.Fatal(err) + } + if _, err := mine(t, opts); err == nil || !strings.Contains(err.Error(), "permissions") { + t.Errorf("update with non-preservable permissions: %v", err) + } + after, err := os.ReadFile(state) + if err != nil || !bytes.Equal(before, after) { + t.Errorf("checkpoint changed: %v", err) + } + actual := make([]byte, len(acl)) + n, err := syscall.Getxattr(state, key, actual) + if err != nil || n != len(acl) || !bytes.Equal(acl, actual) { + t.Errorf("restrictive ACL changed: %v", err) + } +} diff --git a/cli/internal/provenanceapp/mine_session_windows_test.go b/cli/internal/provenanceapp/mine_session_windows_test.go new file mode 100644 index 000000000..1786b6df7 --- /dev/null +++ b/cli/internal/provenanceapp/mine_session_windows_test.go @@ -0,0 +1,47 @@ +package provenanceapp + +import ( + "bytes" + "os" + "os/exec" + "path/filepath" + "strings" + "testing" +) + +func TestMineSession_CheckpointACLRejected(t *testing.T) { + dir := t.TempDir() + state := filepath.Join(dir, "state.json") + session := writeMineSession(t, dir, "session.jsonl", "{\"type\":\"tool_use\",\"tool_name\":\"Read\",\"tool_input\":{}}\n") + opts := MineOptions{File: session, State: state} + if _, err := mine(t, opts); err != nil { + t.Fatal(err) + } + // Builtin Guests SID avoids relying on localized account names. + if out, err := exec.Command("icacls", state, "/deny", "*S-1-5-32-546:(R)").CombinedOutput(); err != nil { + t.Fatalf("set restrictive ACL: %v: %s", err, out) + } + info, err := os.Stat(state) + if err != nil { + t.Fatal(err) + } + beforeACL, err := checkpointSecurity(state, info) + if err != nil { + t.Fatal(err) + } + before, err := os.ReadFile(state) + if err != nil { + t.Fatal(err) + } + if _, err := mine(t, opts); err == nil || !strings.Contains(err.Error(), "permissions") { + t.Errorf("update with non-preservable permissions: %v", err) + } + after, err := os.ReadFile(state) + if err != nil || !bytes.Equal(before, after) { + t.Errorf("checkpoint changed: %v", err) + } + afterACL, err := checkpointSecurity(state, info) + if err != nil || !bytes.Equal(beforeACL, afterACL) { + t.Errorf("restrictive ACL changed: %v", err) + } +} diff --git a/cli/internal/provenanceapp/mine_state_security.go b/cli/internal/provenanceapp/mine_state_security.go new file mode 100644 index 000000000..2c0a66bef --- /dev/null +++ b/cli/internal/provenanceapp/mine_state_security.go @@ -0,0 +1,50 @@ +package provenanceapp + +import ( + "bytes" + "fmt" + "os" + "path/filepath" +) + +// The canonical writer replaces the inode. Before giving it checkpoint data, +// compare the existing permissions with an empty replacement prepared the same +// way. This includes inherited ACLs and ownership, not just permission bits. +// Unsupported or different security metadata fails closed. Like the writer, +// this assumes the caller controls concurrent changes to the destination. +func checkCheckpointReplacement(path string, mode os.FileMode) error { + info, err := os.Lstat(path) + if os.IsNotExist(err) { + return nil + } + if err != nil { + return err + } + original, err := checkpointSecurity(path, info) + if err != nil { + return fmt.Errorf("inspect checkpoint permissions: %w", err) + } + probe, err := os.CreateTemp(filepath.Dir(path), ".tmp-*") + if err != nil { + return fmt.Errorf("inspect replacement permissions: %w", err) + } + defer func() { + _ = probe.Close() + _ = os.Remove(probe.Name()) + }() + if err := probe.Chmod(mode); err != nil { + return err + } + probeInfo, err := probe.Stat() + if err != nil { + return err + } + replacement, err := checkpointSecurity(probe.Name(), probeInfo) + if err != nil { + return fmt.Errorf("inspect replacement permissions: %w", err) + } + if !bytes.Equal(original, replacement) { + return fmt.Errorf("checkpoint permissions cannot be preserved by atomic replacement: %s", path) + } + return nil +} diff --git a/cli/internal/provenanceapp/mine_state_security_darwin.go b/cli/internal/provenanceapp/mine_state_security_darwin.go new file mode 100644 index 000000000..e19ef0719 --- /dev/null +++ b/cli/internal/provenanceapp/mine_state_security_darwin.go @@ -0,0 +1,70 @@ +package provenanceapp + +import ( + "encoding/binary" + "fmt" + "os" + "runtime" + "syscall" + "unsafe" +) + +func checkpointSecurity(path string, info os.FileInfo) ([]byte, error) { + stat, ok := info.Sys().(*syscall.Stat_t) + if !ok { + return nil, fmt.Errorf("file ownership unavailable") + } + acl, err := checkpointDarwinACL(path) + if err != nil { + return nil, err + } + metadata := make([]byte, 12+len(acl)) + binary.LittleEndian.PutUint32(metadata, stat.Uid) + binary.LittleEndian.PutUint32(metadata[4:], stat.Gid) + binary.LittleEndian.PutUint32(metadata[8:], stat.Flags) + copy(metadata[12:], acl) + return metadata, nil +} + +// getattrlist returns a length and an attrreference followed by the opaque ACL. +// REPORT_FULLSIZE prevents silent truncation; NOFOLLOW preserves the path check. +// Native ABI: Apple sys/attr.h and bsd/vfs/vfs_attrlist.c. No cgo is required. +func checkpointDarwinACL(path string) ([]byte, error) { + name, err := syscall.BytePtrFromString(path) + if err != nil { + return nil, err + } + attrs := struct { + Count, Reserved uint16 + Common, Volume, Directory, File, Fork uint32 + }{Count: 5, Common: 0x00400000} // ATTR_CMN_EXTENDED_SECURITY + read := func(buf []byte) error { + _, _, errno := syscall.Syscall6(syscall.SYS_GETATTRLIST, + uintptr(unsafe.Pointer(name)), uintptr(unsafe.Pointer(&attrs)), + uintptr(unsafe.Pointer(&buf[0])), uintptr(len(buf)), 5, 0) + runtime.KeepAlive(name) + runtime.KeepAlive(attrs) + if errno != 0 { + return errno + } + return nil + } + buf := make([]byte, 12) + if err := read(buf); err != nil { + return nil, err + } + size := binary.LittleEndian.Uint32(buf) + if size < 12 || size > 65536 { + return nil, fmt.Errorf("unsupported ACL size %d", size) + } + if size > uint32(len(buf)) { + buf = make([]byte, size) + if err := read(buf); err != nil { + return nil, err + } + } + if binary.LittleEndian.Uint32(buf) != uint32(len(buf)) { + return nil, fmt.Errorf("ACL changed during inspection") + } + return buf, nil +} diff --git a/cli/internal/provenanceapp/mine_state_security_linux.go b/cli/internal/provenanceapp/mine_state_security_linux.go new file mode 100644 index 000000000..87dd22dfe --- /dev/null +++ b/cli/internal/provenanceapp/mine_state_security_linux.go @@ -0,0 +1,71 @@ +package provenanceapp + +import ( + "encoding/binary" + "fmt" + "os" + "sort" + "strings" + "syscall" +) + +func checkpointSecurity(path string, info os.FileInfo) ([]byte, error) { + stat, ok := info.Sys().(*syscall.Stat_t) + if !ok { + return nil, fmt.Errorf("file ownership unavailable") + } + size, err := syscall.Listxattr(path, nil) + if err != nil { + return nil, err + } + if size < 0 || size > 65536 { + return nil, fmt.Errorf("unsupported security attribute list size %d", size) + } + names := make([]byte, size) + n, err := syscall.Listxattr(path, names) + if err != nil { + return nil, fmt.Errorf("read security attribute list: %w", err) + } + if n != size { + return nil, fmt.Errorf("security attribute list changed during inspection") + } + keys := strings.Split(string(names), "\x00") + sort.Strings(keys) + metadata := make([]byte, 8) + binary.LittleEndian.PutUint32(metadata, stat.Uid) + binary.LittleEndian.PutUint32(metadata[4:], stat.Gid) + for _, key := range keys { + // Covers POSIX/NFS ACLs and security labels; user data is not permissions. + if key == "" || strings.HasPrefix(key, "user.") { + continue + } + value, err := checkpointSecurityAttribute(path, key) + if err != nil { + return nil, err + } + metadata = binary.LittleEndian.AppendUint32(metadata, uint32(len(key))) + metadata = append(metadata, key...) + metadata = binary.LittleEndian.AppendUint32(metadata, uint32(len(value))) + metadata = append(metadata, value...) + } + return metadata, nil +} + +func checkpointSecurityAttribute(path, key string) ([]byte, error) { + size, err := syscall.Getxattr(path, key, nil) + if err != nil { + return nil, err + } + if size < 0 || size > 65536 { + return nil, fmt.Errorf("unsupported security attribute size %d", size) + } + value := make([]byte, size) + n, err := syscall.Getxattr(path, key, value) + if err != nil { + return nil, fmt.Errorf("read security attribute: %w", err) + } + if n != size { + return nil, fmt.Errorf("security attribute changed during inspection") + } + return value, nil +} diff --git a/cli/internal/provenanceapp/mine_state_security_other.go b/cli/internal/provenanceapp/mine_state_security_other.go new file mode 100644 index 000000000..18b2e7649 --- /dev/null +++ b/cli/internal/provenanceapp/mine_state_security_other.go @@ -0,0 +1,13 @@ +//go:build !darwin && !linux && !windows + +package provenanceapp + +import ( + "fmt" + "os" + "runtime" +) + +func checkpointSecurity(_ string, _ os.FileInfo) ([]byte, error) { + return nil, fmt.Errorf("checkpoint permission inspection unavailable on %s", runtime.GOOS) +} diff --git a/cli/internal/provenanceapp/mine_state_security_windows.go b/cli/internal/provenanceapp/mine_state_security_windows.go new file mode 100644 index 000000000..8e8b7aa3c --- /dev/null +++ b/cli/internal/provenanceapp/mine_state_security_windows.go @@ -0,0 +1,42 @@ +package provenanceapp + +import ( + "fmt" + "os" + "runtime" + "syscall" + "unsafe" +) + +var checkpointGetFileSecurity = syscall.NewLazyDLL("advapi32.dll").NewProc("GetFileSecurityW") + +func checkpointSecurity(path string, _ os.FileInfo) ([]byte, error) { + name, err := syscall.UTF16PtrFromString(path) + if err != nil { + return nil, err + } + // Owner, group, DACL, integrity label, resource attributes and central + // access policy. These queries require READ_CONTROL, not audit privileges. + // https://learn.microsoft.com/en-us/windows/win32/secauthz/security-information + const securityInformation = 0x1 | 0x2 | 0x4 | 0x10 | 0x20 | 0x40 + var size uint32 + _, _, callErr := checkpointGetFileSecurity.Call(uintptr(unsafe.Pointer(name)), securityInformation, + 0, 0, uintptr(unsafe.Pointer(&size))) + if callErr != syscall.ERROR_INSUFFICIENT_BUFFER { + return nil, fmt.Errorf("query security descriptor size: %w", callErr) + } + if size == 0 || size > 65536 { + return nil, fmt.Errorf("unsupported security descriptor size %d", size) + } + buf := make([]byte, size) + ok, _, callErr := checkpointGetFileSecurity.Call(uintptr(unsafe.Pointer(name)), securityInformation, + uintptr(unsafe.Pointer(&buf[0])), uintptr(len(buf)), uintptr(unsafe.Pointer(&size))) + runtime.KeepAlive(name) + if ok == 0 { + return nil, fmt.Errorf("read security descriptor: %w", callErr) + } + if size > uint32(len(buf)) { + return nil, fmt.Errorf("security descriptor changed during inspection") + } + return buf[:size], nil +} diff --git a/tests/windows/test-windows-smoke.ps1 b/tests/windows/test-windows-smoke.ps1 index da59a0d9e..d2f763345 100644 --- a/tests/windows/test-windows-smoke.ps1 +++ b/tests/windows/test-windows-smoke.ps1 @@ -232,6 +232,7 @@ Write-Step "Running focused Windows-sensitive Go tests" Invoke-GoTest -TestArgs @("-timeout", "3m", "./internal/quality") Invoke-GoTest -TestArgs @("-timeout", "3m", "./cmd/ao", "-run", "^(TestBatchForge_appendForgedRecord|TestAppendForgedRecord|TestBatchForgeSkipsAlreadyForged|TestLoadAndFilterTranscripts_RespectsForgedIndex|TestCanonicalArtifactPath|TestCobraDemoConceptsCommand|TestCobraDemoQuickCommand|TestCobraShowConcepts)$") Invoke-GoTest -TestArgs @("-timeout", "3m", "./internal/storage", "-run", "^TestWithLockedFile_") +Invoke-GoTest -TestArgs @("-timeout", "3m", "./internal/provenanceapp", "-run", "^Test(MineSession|WriteMineState)", "-count", "1") Invoke-GoTest -TestArgs @("-timeout", "3m", "./internal/config", "-run", "^TestSave_ConcurrentPatchesPreserveBothUpdates$") Write-Host "Windows smoke tests passed" From 883a0f301439fbcac6d1edbcd3d2166dc972bf55 Mon Sep 17 00:00:00 2001 From: Bo Date: Wed, 9 Sep 2026 18:32:41 -0400 Subject: [PATCH 4/4] fix(provenance): retain Windows security error identity --- cli/internal/provenanceapp/mine_state_security_windows.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/cli/internal/provenanceapp/mine_state_security_windows.go b/cli/internal/provenanceapp/mine_state_security_windows.go index 8e8b7aa3c..7e541f2c7 100644 --- a/cli/internal/provenanceapp/mine_state_security_windows.go +++ b/cli/internal/provenanceapp/mine_state_security_windows.go @@ -1,6 +1,7 @@ package provenanceapp import ( + "errors" "fmt" "os" "runtime" @@ -22,7 +23,7 @@ func checkpointSecurity(path string, _ os.FileInfo) ([]byte, error) { var size uint32 _, _, callErr := checkpointGetFileSecurity.Call(uintptr(unsafe.Pointer(name)), securityInformation, 0, 0, uintptr(unsafe.Pointer(&size))) - if callErr != syscall.ERROR_INSUFFICIENT_BUFFER { + if !errors.Is(callErr, syscall.ERROR_INSUFFICIENT_BUFFER) { return nil, fmt.Errorf("query security descriptor size: %w", callErr) } if size == 0 || size > 65536 {