Skip to content
This repository was archived by the owner on Jul 21, 2026. It is now read-only.

Commit 2de0635

Browse files
solomonneassolomonneas
committed
fix: harden archive file writes
1 parent c44f16e commit 2de0635

5 files changed

Lines changed: 158 additions & 15 deletions

File tree

‎.github/workflows/ci.yml‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,9 @@ jobs:
1414
with:
1515
go-version: stable
1616
- run: go test ./...
17+
- run: go vet ./...
18+
- run: go install golang.org/x/vuln/cmd/govulncheck@latest
19+
- run: govulncheck ./...
1720
- run: go build -o bin/spine ./cmd/spine
1821
- run: scripts/smoke_archive.sh
1922
- run: scripts/smoke_mcp.sh

‎internal/app/app.go‎

Lines changed: 28 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package app
22

33
import (
44
"bytes"
5+
"context"
56
"crypto/sha256"
67
"database/sql"
78
"encoding/csv"
@@ -32,6 +33,8 @@ var stdin io.Reader = os.Stdin
3233

3334
const Version = "0.1.5"
3435

36+
const externalScannerTimeout = 30 * time.Minute
37+
3538
func Run(args []string, out, errw io.Writer) int {
3639
if len(args) == 0 || args[0] == "--help" || args[0] == "-h" || args[0] == "help" {
3740
usage(out)
@@ -117,7 +120,7 @@ func cmdInit(args []string, out, errw io.Writer) int {
117120
}
118121
if _, err := os.Stat(paths.ConfigPath); errors.Is(err, os.ErrNotExist) {
119122
body := fmt.Sprintf("db_path = %q\ncache_dir = %q\n", paths.DBPath, paths.CacheDir)
120-
if err := os.WriteFile(paths.ConfigPath, []byte(body), security.PrivateFileMode); err != nil {
123+
if err := security.WritePrivateFileAtomic(paths.ConfigPath, []byte(body)); err != nil {
121124
return fatalf(errw, "init: %s", err)
122125
}
123126
}
@@ -679,11 +682,16 @@ func cmdImportAgentTrail(args []string, out, errw io.Writer) int {
679682
if values["redact"] != "" {
680683
cmdArgs = append(cmdArgs, "--redact", values["redact"])
681684
}
682-
cmd := exec.Command("agenttrail", cmdArgs...)
685+
ctx, cancel := context.WithTimeout(context.Background(), externalScannerTimeout)
686+
defer cancel()
687+
cmd := exec.CommandContext(ctx, "agenttrail", cmdArgs...)
683688
var stderr bytes.Buffer
684689
cmd.Stderr = &stderr
685690
b, err := cmd.Output()
686691
if err != nil {
692+
if ctx.Err() == context.DeadlineExceeded {
693+
return fatalf(errw, "import agenttrail: timed out after %s", externalScannerTimeout)
694+
}
687695
return fatalf(errw, "import agenttrail: %s", strings.TrimSpace(stderr.String()))
688696
}
689697
if bools["json"] {
@@ -730,7 +738,9 @@ func runAgentTrailImport(db *sql.DB, sourceKind, sourcePath string, values map[s
730738
if values["redact"] != "" {
731739
cmdArgs = append(cmdArgs, "--redact", values["redact"])
732740
}
733-
cmd := exec.Command("agenttrail", cmdArgs...)
741+
ctx, cancel := context.WithTimeout(context.Background(), externalScannerTimeout)
742+
defer cancel()
743+
cmd := exec.CommandContext(ctx, "agenttrail", cmdArgs...)
734744
stdout, err := cmd.StdoutPipe()
735745
if err != nil {
736746
return ingest.AdapterResult{}, agentTrailSummary{}, err
@@ -742,6 +752,9 @@ func runAgentTrailImport(db *sql.DB, sourceKind, sourcePath string, values map[s
742752
}
743753
result, importErr := ingest.ImportAdapterReader(db, stdout, "agenttrail://"+sourceKind+"/"+sourcePath, sourceKind)
744754
waitErr := cmd.Wait()
755+
if ctx.Err() == context.DeadlineExceeded {
756+
return ingest.AdapterResult{}, agentTrailSummary{}, fmt.Errorf("agenttrail timed out after %s", externalScannerTimeout)
757+
}
745758
if importErr != nil {
746759
return ingest.AdapterResult{}, agentTrailSummary{}, importErr
747760
}
@@ -796,22 +809,24 @@ func cmdAdapterGenerate(name string, generator sources.Generator, args []string,
796809
return fatalf(errw, "adapter %s: %s", name, err)
797810
}
798811
var w io.Writer = out
799-
var f *os.File
812+
var output *security.AtomicFile
813+
defer func() { _ = output.Abort() }()
800814
if values["out"] != "-" {
801-
if err := security.EnsurePrivateParent(values["out"]); err != nil {
802-
return fatalf(errw, "adapter %s: %s", name, err)
803-
}
804-
f, err = os.OpenFile(values["out"], os.O_CREATE|os.O_TRUNC|os.O_WRONLY, security.PrivateFileMode)
815+
output, err = security.CreateAtomicFile(values["out"])
805816
if err != nil {
806817
return fatalf(errw, "adapter %s: %s", name, err)
807818
}
808-
defer f.Close()
809-
w = f
819+
w = output.File
810820
}
811821
result, err := generator(rest[0], sources.Options{Limit: limit, Since: values["since"]}, w)
812822
if err != nil {
813823
return fatalf(errw, "adapter %s: %s", name, err)
814824
}
825+
if output != nil {
826+
if err := output.Commit(); err != nil {
827+
return fatalf(errw, "adapter %s: %s", name, err)
828+
}
829+
}
815830
if bools["json"] && values["out"] != "-" {
816831
writeJSON(out, result)
817832
}
@@ -1312,10 +1327,10 @@ func saveEvidenceBundle(bundle map[string]any) error {
13121327
if err != nil {
13131328
return err
13141329
}
1315-
if err := os.WriteFile(path, append(b, '\n'), security.PrivateFileMode); err != nil {
1330+
if err := security.WritePrivateFileAtomic(path, append(b, '\n')); err != nil {
13161331
return err
13171332
}
1318-
return security.ChmodPrivateFile(path)
1333+
return nil
13191334
}
13201335

13211336
func loadEvidenceBundle(id string) (map[string]any, error) {
@@ -1526,7 +1541,7 @@ order by s.kind, c.name, i.created_at, i.id`)
15261541
fmt.Fprintf(&b, "Summary: %s\n\n", r.summary)
15271542
}
15281543
}
1529-
if err := os.WriteFile(path, []byte(b.String()), security.PrivateFileMode); err != nil {
1544+
if err := security.WritePrivateFileAtomic(path, []byte(b.String())); err != nil {
15301545
return count, err
15311546
}
15321547
count++

‎internal/app/app_test.go‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -150,6 +150,38 @@ func TestImportAdapterFromStdin(t *testing.T) {
150150
}
151151
}
152152

153+
func TestAdapterExportFilesArePrivateAndAtomic(t *testing.T) {
154+
withTempHome(t)
155+
fixture := repoPath(t, "testdata/harnesses/codex-session.fixture.jsonl")
156+
dir := t.TempDir()
157+
outPath := filepath.Join(dir, "codex.adapter.jsonl")
158+
159+
runOK(t, "adapter", "codex", fixture, "--out", outPath, "--json")
160+
assertPrivate(t, outPath)
161+
162+
if err := os.WriteFile(outPath, []byte("original\n"), 0o600); err != nil {
163+
t.Fatal(err)
164+
}
165+
code, stdout, stderr := run("adapter", "codex", filepath.Join(dir, "missing"), "--out", outPath)
166+
if code == 0 {
167+
t.Fatalf("expected failure, stdout=%s stderr=%s", stdout, stderr)
168+
}
169+
b, err := os.ReadFile(outPath)
170+
if err != nil {
171+
t.Fatal(err)
172+
}
173+
if string(b) != "original\n" {
174+
t.Fatalf("output was replaced on failure: %q", string(b))
175+
}
176+
matches, err := filepath.Glob(filepath.Join(dir, ".codex.adapter.jsonl.tmp-*"))
177+
if err != nil {
178+
t.Fatal(err)
179+
}
180+
if len(matches) != 0 {
181+
t.Fatalf("temp files left behind: %v", matches)
182+
}
183+
}
184+
153185
func TestImportAgentTrailWrapper(t *testing.T) {
154186
withTempHome(t)
155187
runOK(t, "init")

‎internal/app/sourceharvest.go‎

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package app
33
import (
44
"bufio"
55
"bytes"
6+
"context"
67
"crypto/sha256"
78
"database/sql"
89
"encoding/hex"
@@ -58,7 +59,9 @@ func cmdImportSourceHarvest(args []string, out, errw io.Writer) int {
5859
return fatalf(errw, "import sourceharvest: %s", err)
5960
}
6061
defer db.Close()
61-
cmd := exec.Command("sourceharvest", passArgs...)
62+
ctx, cancel := context.WithTimeout(context.Background(), externalScannerTimeout)
63+
defer cancel()
64+
cmd := exec.CommandContext(ctx, "sourceharvest", passArgs...)
6265
stdout, err := cmd.StdoutPipe()
6366
if err != nil {
6467
return fatalf(errw, "import sourceharvest: %s", err)
@@ -70,6 +73,9 @@ func cmdImportSourceHarvest(args []string, out, errw io.Writer) int {
7073
}
7174
result, importErr := ingest.ImportAdapterReader(db, stdout, "sourceharvest://"+strings.Join(passArgs, " "), "")
7275
waitErr := cmd.Wait()
76+
if ctx.Err() == context.DeadlineExceeded {
77+
return fatalf(errw, "import sourceharvest: timed out after %s", externalScannerTimeout)
78+
}
7379
if importErr != nil {
7480
return fatalf(errw, "import sourceharvest: %s", importErr)
7581
}
@@ -100,7 +106,9 @@ func cmdImportSourceHarvest(args []string, out, errw io.Writer) int {
100106
}
101107

102108
func dryRunSourceHarvest(args []string) (int, []string, error) {
103-
cmd := exec.Command("sourceharvest", args...)
109+
ctx, cancel := context.WithTimeout(context.Background(), externalScannerTimeout)
110+
defer cancel()
111+
cmd := exec.CommandContext(ctx, "sourceharvest", args...)
104112
stdout, err := cmd.StdoutPipe()
105113
if err != nil {
106114
return 0, nil, err
@@ -126,9 +134,15 @@ func dryRunSourceHarvest(args []string) (int, []string, error) {
126134
records++
127135
}
128136
if err := scanner.Err(); err != nil {
137+
if ctx.Err() == context.DeadlineExceeded {
138+
return records, warnings, fmt.Errorf("sourceharvest timed out after %s", externalScannerTimeout)
139+
}
129140
return 0, nil, err
130141
}
131142
if err := cmd.Wait(); err != nil {
143+
if ctx.Err() == context.DeadlineExceeded {
144+
return records, warnings, fmt.Errorf("sourceharvest timed out after %s", externalScannerTimeout)
145+
}
132146
msg := strings.TrimSpace(stderr.String())
133147
if msg == "" {
134148
msg = err.Error()

‎internal/security/security.go‎

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package security
22

33
import (
4+
"errors"
45
"os"
56
"path/filepath"
67
)
@@ -24,3 +25,81 @@ func EnsurePrivateParent(path string) error {
2425
func ChmodPrivateFile(path string) error {
2526
return os.Chmod(path, PrivateFileMode)
2627
}
28+
29+
type AtomicFile struct {
30+
File *os.File
31+
finalPath string
32+
tempPath string
33+
closed bool
34+
committed bool
35+
}
36+
37+
func CreateAtomicFile(path string) (*AtomicFile, error) {
38+
if err := EnsurePrivateParent(path); err != nil {
39+
return nil, err
40+
}
41+
dir := filepath.Dir(path)
42+
pattern := "." + filepath.Base(path) + ".tmp-*"
43+
f, err := os.CreateTemp(dir, pattern)
44+
if err != nil {
45+
return nil, err
46+
}
47+
if err := f.Chmod(PrivateFileMode); err != nil {
48+
_ = f.Close()
49+
_ = os.Remove(f.Name())
50+
return nil, err
51+
}
52+
return &AtomicFile{File: f, finalPath: path, tempPath: f.Name()}, nil
53+
}
54+
55+
func (f *AtomicFile) Close() error {
56+
if f == nil || f.closed {
57+
return nil
58+
}
59+
err := f.File.Close()
60+
f.closed = true
61+
return err
62+
}
63+
64+
func (f *AtomicFile) Commit() error {
65+
if f == nil {
66+
return nil
67+
}
68+
if err := f.Close(); err != nil {
69+
_ = os.Remove(f.tempPath)
70+
return err
71+
}
72+
if err := os.Rename(f.tempPath, f.finalPath); err != nil {
73+
_ = os.Remove(f.tempPath)
74+
return err
75+
}
76+
f.committed = true
77+
return nil
78+
}
79+
80+
func (f *AtomicFile) Abort() error {
81+
if f == nil || f.committed {
82+
return nil
83+
}
84+
closeErr := f.Close()
85+
removeErr := os.Remove(f.tempPath)
86+
if closeErr != nil {
87+
return closeErr
88+
}
89+
if removeErr != nil && !errors.Is(removeErr, os.ErrNotExist) {
90+
return removeErr
91+
}
92+
return nil
93+
}
94+
95+
func WritePrivateFileAtomic(path string, data []byte) error {
96+
f, err := CreateAtomicFile(path)
97+
if err != nil {
98+
return err
99+
}
100+
defer func() { _ = f.Abort() }()
101+
if _, err := f.File.Write(data); err != nil {
102+
return err
103+
}
104+
return f.Commit()
105+
}

0 commit comments

Comments
 (0)