diff --git a/cmd/ob/commands.go b/cmd/ob/commands.go index 8bd8b16a..6cc92e17 100644 --- a/cmd/ob/commands.go +++ b/cmd/ob/commands.go @@ -7,7 +7,6 @@ import ( "fmt" "io" "os" - "os/exec" "path/filepath" "sort" "strings" @@ -19,6 +18,7 @@ import ( "github.com/labstack/onebox/internal/app" "github.com/labstack/onebox/internal/compose" "github.com/labstack/onebox/internal/engine" + "github.com/labstack/onebox/internal/gitinfo" "github.com/labstack/onebox/internal/journal" "github.com/labstack/onebox/internal/notify" "github.com/labstack/onebox/internal/onebox" @@ -999,9 +999,5 @@ func confirmAt(cmd *cobra.Command, out io.Writer, prompt string) bool { } func gitShortSHA(ctx context.Context, dir string) string { - out, err := exec.CommandContext(ctx, "git", "-C", dir, "rev-parse", "--short=7", "HEAD").Output() - if err != nil { - return "" - } - return strings.TrimSpace(string(out)) + return gitinfo.Revision(ctx, dir) } diff --git a/cmd/ob/git_test.go b/cmd/ob/git_test.go new file mode 100644 index 00000000..c4c8b059 --- /dev/null +++ b/cmd/ob/git_test.go @@ -0,0 +1,30 @@ +package main + +import ( + "os" + "os/exec" + "path/filepath" + "strings" + "testing" +) + +func TestCLIRevisionMarksUntrackedPayloadDirty(t *testing.T) { + dir := t.TempDir() + git := func(args ...string) string { + t.Helper() + out, err := exec.CommandContext(t.Context(), "git", append([]string{"-C", dir}, args...)...).CombinedOutput() + if err != nil { + t.Fatalf("git %v: %v: %s", args, err, out) + } + return strings.TrimSpace(string(out)) + } + git("init") + git("-c", "user.name=Onebox Test", "-c", "user.email=test@example.invalid", "-c", "commit.gpgsign=false", "commit", "--allow-empty", "-m", "fixture") + want := git("rev-parse", "--short=7", "HEAD") + "+dirty" + if err := os.WriteFile(filepath.Join(dir, "payload.txt"), []byte("untracked\n"), 0o600); err != nil { + t.Fatal(err) + } + if got := gitShortSHA(t.Context(), dir); got != want { + t.Fatalf("CLI revision = %q, want %q", got, want) + } +} diff --git a/internal/engine/audit.go b/internal/engine/audit.go index a5c403be..9347c9a9 100644 --- a/internal/engine/audit.go +++ b/internal/engine/audit.go @@ -39,13 +39,17 @@ func (e *Engine) Audit(ctx context.Context, n int) error { // width scan and the print loop want the same string. cells := make([]string, len(rows)) action := len("ACTION") + gitWidth := 9 for i, r := range rows { cells[i] = auditActionCell(r) if len(cells[i]) > action { action = len(cells[i]) } + if len(r.GitSHA) > gitWidth { + gitWidth = len(r.GitSHA) + } } - format := fmt.Sprintf("%%-%ds %%-%ds %%-20s %%-9s %%-12s %%s\n", width, action) + format := fmt.Sprintf("%%-%ds %%-%ds %%-20s %%-%ds %%-12s %%s\n", width, action, gitWidth) fmt.Fprintf(e.Opts.Out, format, "RELEASE", "ACTION", "OPERATOR", "GIT", "OUTCOME", "STARTED") for i, r := range rows { git := r.GitSHA diff --git a/internal/engine/audit_test.go b/internal/engine/audit_test.go index 5a61ea54..2c72533c 100644 --- a/internal/engine/audit_test.go +++ b/internal/engine/audit_test.go @@ -17,7 +17,7 @@ func TestAuditListsOutcomesNewestFirst(t *testing.T) { return transport.Result{Stdout: "R1.jsonl\nR2.jsonl\n"}, true case strings.Contains(cmd, "R1.jsonl"): return transport.Result{Stdout: journalLines( - journal.Record{DeployID: "R1", Phase: "deploy", Event: "start", Operator: "v@mac", GitSHA: "abc1234", TS: "t1"}, + journal.Record{DeployID: "R1", Phase: "deploy", Event: "start", Operator: "v@mac", GitSHA: "abc1234+dirty", TS: "t1"}, journal.Record{DeployID: "R1", Phase: "deploy", Event: "finish", Status: "ok"}, )}, true case strings.Contains(cmd, "R2.jsonl"): @@ -39,9 +39,18 @@ func TestAuditListsOutcomesNewestFirst(t *testing.T) { if strings.Index(s, "R2") > strings.Index(s, "R1") { t.Fatalf("newest first expected:\n%s", s) } - if !strings.Contains(s, "ci@runner") || !strings.Contains(s, "abc1234") { + if !strings.Contains(s, "ci@runner") || !strings.Contains(s, "abc1234+dirty") { t.Fatalf("operator/sha missing:\n%s", s) } + lines := strings.Split(strings.TrimSpace(s), "\n") + column := strings.Index(lines[0], "OUTCOME") + if strings.Index(lines[1], "INCOMPLETE") != column || strings.Index(lines[2], "deployed") != column { + t.Fatalf("dirty revision misaligned audit outcomes:\n%s", s) + } + records, err := e.AuditSnapshot(t.Context(), 10) + if err != nil || len(records) != 2 || records[1].GitSHA != "abc1234+dirty" { + t.Fatalf("structured audit lost dirty provenance: %+v, %v", records, err) + } } func TestAuditExposesSafeExecInvocationEvidence(t *testing.T) { diff --git a/internal/gitinfo/revision.go b/internal/gitinfo/revision.go new file mode 100644 index 00000000..a7c8e0d5 --- /dev/null +++ b/internal/gitinfo/revision.go @@ -0,0 +1,29 @@ +// Package gitinfo reports the local checkout's revision for operation provenance. +package gitinfo + +import ( + "context" + "os/exec" + "strings" +) + +// Revision returns HEAD's short SHA, suffixed with +dirty when tracked or +// untracked files differ. Git-ignored files are excluded, as in git status; +// this is checkout provenance, not a digest of the staged release payload. +// If either HEAD or working-tree state is unavailable, it makes no claim. +func Revision(ctx context.Context, dir string) string { + out, err := exec.CommandContext(ctx, "git", "-C", dir, "rev-parse", "--short=7", "HEAD").Output() + if err != nil { + return "" + } + revision := strings.TrimSpace(string(out)) + status, err := exec.CommandContext(ctx, "git", "--no-optional-locks", "-C", dir, + "status", "--porcelain=v1", "--untracked-files=normal", "--ignore-submodules=none").Output() + if err != nil { + return "" + } + if len(status) > 0 { + revision += "+dirty" + } + return revision +} diff --git a/internal/gitinfo/revision_test.go b/internal/gitinfo/revision_test.go new file mode 100644 index 00000000..76f52f0e --- /dev/null +++ b/internal/gitinfo/revision_test.go @@ -0,0 +1,66 @@ +package gitinfo + +import ( + "os" + "os/exec" + "path/filepath" + "strings" + "testing" +) + +func TestRevisionReportsSubmoduleChangesButExcludesIgnoredFiles(t *testing.T) { + for _, change := range []string{"clean", "ignored", "modified", "untracked", "new commit"} { + t.Run(change, func(t *testing.T) { + git := func(dir string, args ...string) string { + t.Helper() + cmd := exec.CommandContext(t.Context(), "git", append([]string{"-C", dir}, args...)...) + out, err := cmd.CombinedOutput() + if err != nil { + t.Fatalf("git %v: %v: %s", args, err, out) + } + return strings.TrimSpace(string(out)) + } + write := func(dir, name, content string) { + t.Helper() + if err := os.WriteFile(filepath.Join(dir, name), []byte(content), 0o600); err != nil { + t.Fatal(err) + } + } + commit := func(dir string) { + t.Helper() + git(dir, "add", ".") + git(dir, "-c", "user.name=Onebox Test", "-c", "user.email=test@example.invalid", "-c", "commit.gpgsign=false", "commit", "-m", "fixture") + } + source, root := t.TempDir(), t.TempDir() + git(source, "init") + write(source, "payload.txt", "committed\n") + write(source, ".gitignore", "cache.txt\n") + commit(source) + git(root, "init") + git(root, "-c", "protocol.file.allow=always", "submodule", "add", source, "module") + commit(root) + want := git(root, "rev-parse", "--short=7", "HEAD") + // Local preferences must not hide real submodule changes. + git(root, "config", "submodule.module.ignore", "all") + git(root, "config", "status.showUntrackedFiles", "no") + module := filepath.Join(root, "module") + switch change { + case "ignored": + write(module, "cache.txt", "ignored\n") + case "modified", "new commit": + write(module, "payload.txt", "changed\n") + if change == "new commit" { + commit(module) + } + case "untracked": + write(module, "new-payload.txt", "untracked\n") + } + if change != "clean" && change != "ignored" { + want += "+dirty" + } + if got := Revision(t.Context(), root); got != want { + t.Fatalf("revision = %q, want %q", got, want) + } + }) + } +} diff --git a/internal/onebox/git_test.go b/internal/onebox/git_test.go new file mode 100644 index 00000000..1904b6bf --- /dev/null +++ b/internal/onebox/git_test.go @@ -0,0 +1,84 @@ +package onebox + +import ( + "context" + "os" + "os/exec" + "path/filepath" + "strings" + "testing" +) + +func TestGitShortSHAReportsWorkingTreeChanges(t *testing.T) { + for _, change := range []string{"clean", "ignored", "modified", "staged", "untracked", "deleted", "index unavailable"} { + t.Run(change, func(t *testing.T) { + dir := t.TempDir() + git := func(args ...string) string { + t.Helper() + cmd := exec.CommandContext(t.Context(), "git", append([]string{"-C", dir}, args...)...) + out, err := cmd.CombinedOutput() + if err != nil { + t.Fatalf("git %v: %v: %s", args, err, out) + } + return strings.TrimSpace(string(out)) + } + write := func(path, content string) { + t.Helper() + if err := os.WriteFile(filepath.Join(dir, path), []byte(content), 0o600); err != nil { + t.Fatal(err) + } + } + git("init") + write("payload.txt", "committed payload\n") + git("add", "payload.txt") + git("-c", "user.name=Onebox Test", "-c", "user.email=test@example.invalid", "-c", "commit.gpgsign=false", "commit", "-m", "fixture") + want := git("rev-parse", "--short=7", "HEAD") + switch change { + case "ignored": + write(".git/info/exclude", "cache.txt\n") + write("cache.txt", "ignored cache\n") + case "modified", "staged": + write("payload.txt", "uncommitted payload\n") + if change == "staged" { + git("add", "payload.txt") + } + case "untracked": + // An operator's preference must not hide payload changes. + git("config", "status.showUntrackedFiles", "no") + write("new-payload.txt", "untracked payload\n") + case "deleted": + if err := os.Remove(filepath.Join(dir, "payload.txt")); err != nil { + t.Fatal(err) + } + case "index unavailable": + write(".git/index", "corrupted index") + } + if change == "index unavailable" { + want = "" + } else if change != "clean" && change != "ignored" { + want += "+dirty" + } + if got := gitShortSHA(t.Context(), dir); got != want { + t.Errorf("revision = %q, want %q", got, want) + } + }) + } +} + +func TestGitShortSHAOmitsUnavailableRevision(t *testing.T) { + if got := gitShortSHA(t.Context(), t.TempDir()); got != "" { + t.Fatalf("non-repository revision = %q, want empty", got) + } + unborn := t.TempDir() + if out, err := exec.CommandContext(t.Context(), "git", "-C", unborn, "init").CombinedOutput(); err != nil { + t.Fatalf("git init: %v: %s", err, out) + } + if got := gitShortSHA(t.Context(), unborn); got != "" { + t.Fatalf("unborn repository revision = %q, want empty", got) + } + ctx, cancel := context.WithCancel(t.Context()) + cancel() + if got := gitShortSHA(ctx, "."); got != "" { + t.Fatalf("cancelled revision = %q, want empty", got) + } +} diff --git a/internal/onebox/service.go b/internal/onebox/service.go index 1f150b84..b31d50b3 100644 --- a/internal/onebox/service.go +++ b/internal/onebox/service.go @@ -6,9 +6,7 @@ import ( "encoding/hex" "fmt" "io" - "os/exec" "path/filepath" - "strings" "sync" "sync/atomic" "time" @@ -16,6 +14,7 @@ import ( "github.com/labstack/onebox/internal/app" "github.com/labstack/onebox/internal/buildinfo" "github.com/labstack/onebox/internal/engine" + "github.com/labstack/onebox/internal/gitinfo" "github.com/labstack/onebox/internal/release" "github.com/labstack/onebox/internal/shellquote" "github.com/labstack/onebox/internal/transport" @@ -156,15 +155,9 @@ func ensureEnvironment(cfg *app.Resolved, name string) error { return nil } -// gitShortSHA is the working tree's revision, recorded on every operation so a -// journal entry can be traced to the code that produced it. An unavailable or -// dirty revision is simply absent rather than guessed at. +// gitShortSHA shares the CLI's checkout provenance, including dirty state. func gitShortSHA(ctx context.Context, dir string) string { - out, err := exec.CommandContext(ctx, "git", "-C", dir, "rev-parse", "--short=7", "HEAD").Output() - if err != nil { - return "" - } - return strings.TrimSpace(string(out)) + return gitinfo.Revision(ctx, dir) } func noneIfEmpty(s string) string { diff --git a/internal/release/release.go b/internal/release/release.go index f9fa0636..8d06a17b 100644 --- a/internal/release/release.go +++ b/internal/release/release.go @@ -47,8 +47,13 @@ func IsID(name string) bool { return releaseID.MatchString(name) } // NewID builds a lexically time-ordered release id. An unsafe SHA component // is replaced, never interpolated (command-injection rule). func NewID(now time.Time, gitSHA string) string { - if !safeSHA.MatchString(gitSHA) { + sha := strings.TrimSuffix(gitSHA, "+dirty") + if !safeSHA.MatchString(sha) { gitSHA = "nogit" + } else if sha != gitSHA { + // The provenance field keeps +dirty; directory identities use the + // existing safe alphabet so rollback and retention recognize them. + gitSHA = sha + "-dirty" } return now.UTC().Format("20060102-150405") + "-" + gitSHA } diff --git a/internal/release/release_test.go b/internal/release/release_test.go index 8ca36c50..c56a678d 100644 --- a/internal/release/release_test.go +++ b/internal/release/release_test.go @@ -24,6 +24,14 @@ func TestNewID(t *testing.T) { if !strings.HasSuffix(NewID(time.Now(), "not$(safe)"), "-nogit") { t.Fatal("unsafe sha must be replaced with nogit") } + if got := NewID(time.Date(2026, 7, 2, 15, 4, 5, 0, time.UTC), "abc1234+dirty"); got != "20260702-150405-abc1234-dirty" { + t.Fatalf("dirty release ID = %q", got) + } + for _, unsafe := range []string{"not$(safe)+dirty", "abc1234+dirty+dirty", "abc1234-dirty", "abc1234+other"} { + if !strings.HasSuffix(NewID(time.Now(), unsafe), "-nogit") { + t.Errorf("unsafe revision %q must be replaced with nogit", unsafe) + } + } } func seedReleaseChain(t *testing.T, target *transport.Fake, names app.Names, previousID, currentID string) { @@ -305,7 +313,7 @@ func TestPreviousRejectsCorruptUnknownAndBootstrapTargets(t *testing.T) { } func TestIsIDAcceptsWhatNewIDProduces(t *testing.T) { - for _, id := range []string{NewID(time.Now(), "abc1234"), NewID(time.Now(), ""), NewID(time.Now(), "abc1234") + "-v2"} { + for _, id := range []string{NewID(time.Now(), "abc1234"), NewID(time.Now(), "abc1234+dirty"), NewID(time.Now(), ""), NewID(time.Now(), "abc1234") + "-v2"} { if !IsID(id) { t.Errorf("IsID rejected an id NewID produced: %q", id) } diff --git a/site/src/content/docs/explanation/evidence-not-declaration.mdx b/site/src/content/docs/explanation/evidence-not-declaration.mdx index f738b424..bb57e386 100644 --- a/site/src/content/docs/explanation/evidence-not-declaration.mdx +++ b/site/src/content/docs/explanation/evidence-not-declaration.mdx @@ -79,6 +79,17 @@ labels authored, default, environment-override, observed and derived values. Knowing that `retainReleases: 5` was Onebox's choice rather than yours is the difference between reviewing a configuration and reading one. +## Checkout provenance + +The journal's `git_sha` and the audit table's `GIT` column show the checkout's +short HEAD revision. Modified, staged, deleted, or untracked files add `+dirty`; +release directory IDs use `-dirty` to keep their existing safe format. If Git +cannot read either the revision or the working-tree state, the field is omitted. + +This describes the checkout when inspected. Git-ignored files are excluded, and +the revision does not prove which bytes were shipped. The sealed plan's payload +digest binds the staged release contents separately. + ## Drift fails closed A server-side artifact that differs from what the plan bound is a typed drift