From f293d4800645ee01b79029982354bfedbc065d96 Mon Sep 17 00:00:00 2001 From: Snider Date: Sat, 25 Apr 2026 04:19:30 +0100 Subject: [PATCH] =?UTF-8?q?fix(agent):=20tighten=20workspace=20file=20perm?= =?UTF-8?q?s=200644=E2=86=920600=20to=20protect=20extracted=20secrets=20(C?= =?UTF-8?q?erberus=20#324)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit .core/reference/fs.go (canonical) + pkg/lib/workspace/default/.core/reference/fs.go (embedded copy): - Write/WriteAtomic/Create/Append default to 0600 - Parent directories use 0700 (was 0755) - WriteMode reapplies the requested mode after writes so overwriting an existing file also tightens permissions Test (pkg/lib/lib_test.go) keeps embedded fs.go synced with canonical + asserts extracted workspaces carry the secure permission defaults. tests/cli/extract copy not hand-edited — that flows from regeneration. Co-authored-by: Codex Closes tasks.lthn.sh/view.php?id=324 --- .core/reference/fs.go | 45 +++++++++++----- pkg/lib/lib_test.go | 54 +++++++++++++++++++ .../workspace/default/.core/reference/fs.go | 45 +++++++++++----- 3 files changed, 118 insertions(+), 26 deletions(-) diff --git a/.core/reference/fs.go b/.core/reference/fs.go index 7f75fa9..c44ba02 100644 --- a/.core/reference/fs.go +++ b/.core/reference/fs.go @@ -148,26 +148,29 @@ func (m *Fs) Read(p string) Result { } // Write saves content to file, creating parent directories as needed. -// Files are created with mode 0644. For sensitive files (keys, secrets), -// use WriteMode with 0600. +// Files are created with mode 0600 by default. +// Use WriteMode when broader access is intentional. func (m *Fs) Write(p, content string) Result { - return m.WriteMode(p, content, 0644) + return m.WriteMode(p, content, 0600) } // WriteMode saves content to file with explicit permissions. -// Use 0600 for sensitive files (encryption output, private keys, auth hashes). +// Use 0644 or 0755 only when broader access is intentional. func (m *Fs) WriteMode(p, content string, mode os.FileMode) Result { vp := m.validatePath(p) if !vp.OK { return vp } full := vp.Value.(string) - if err := os.MkdirAll(filepath.Dir(full), 0755); err != nil { + if err := os.MkdirAll(filepath.Dir(full), 0700); err != nil { return Result{err, false} } if err := os.WriteFile(full, []byte(content), mode); err != nil { return Result{err, false} } + if err := os.Chmod(full, mode); err != nil { + return Result{err, false} + } return Result{OK: true} } @@ -180,7 +183,7 @@ func (m *Fs) TempDir(prefix string) string { root := m.root if root == "" || root == "/" { root = os.TempDir() - } else if err := os.MkdirAll(root, 0755); err != nil { + } else if err := os.MkdirAll(root, 0700); err != nil { return "" } dir, err := os.MkdirTemp(root, prefix) @@ -212,12 +215,12 @@ func (m *Fs) WriteAtomic(p, content string) Result { return vp } full := vp.Value.(string) - if err := os.MkdirAll(filepath.Dir(full), 0755); err != nil { + if err := os.MkdirAll(filepath.Dir(full), 0700); err != nil { return Result{err, false} } tmp := full + ".tmp." + shortRand() - if err := os.WriteFile(tmp, []byte(content), 0644); err != nil { + if err := os.WriteFile(tmp, []byte(content), 0600); err != nil { return Result{err, false} } if err := os.Rename(tmp, full); err != nil { @@ -233,7 +236,7 @@ func (m *Fs) EnsureDir(p string) Result { if !vp.OK { return vp } - if err := os.MkdirAll(vp.Value.(string), 0755); err != nil { + if err := os.MkdirAll(vp.Value.(string), 0700); err != nil { return Result{err, false} } return Result{OK: true} @@ -309,10 +312,18 @@ func (m *Fs) Create(p string) Result { return vp } full := vp.Value.(string) - if err := os.MkdirAll(filepath.Dir(full), 0755); err != nil { + if err := os.MkdirAll(filepath.Dir(full), 0700); err != nil { return Result{err, false} } - return Result{}.New(os.Create(full)) + file, err := os.OpenFile(full, os.O_CREATE|os.O_TRUNC|os.O_WRONLY, 0600) + if err != nil { + return Result{err, false} + } + if err := file.Chmod(0600); err != nil { + file.Close() + return Result{err, false} + } + return Result{}.New(file) } // Append opens the named file for appending, creating it if it doesn't exist. @@ -322,10 +333,18 @@ func (m *Fs) Append(p string) Result { return vp } full := vp.Value.(string) - if err := os.MkdirAll(filepath.Dir(full), 0755); err != nil { + if err := os.MkdirAll(filepath.Dir(full), 0700); err != nil { return Result{err, false} } - return Result{}.New(os.OpenFile(full, os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0644)) + file, err := os.OpenFile(full, os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0600) + if err != nil { + return Result{err, false} + } + if err := file.Chmod(0600); err != nil { + file.Close() + return Result{err, false} + } + return Result{}.New(file) } // ReadStream returns a reader for the file content. diff --git a/pkg/lib/lib_test.go b/pkg/lib/lib_test.go index 551ce2d..404d091 100644 --- a/pkg/lib/lib_test.go +++ b/pkg/lib/lib_test.go @@ -625,6 +625,60 @@ func TestLib_ReferenceFiles_Good_SPDXHeaders(t *testing.T) { } } +func TestLib_ReferenceFs_Good_EmbeddedCopyMatchesSource(t *testing.T) { + _, file, _, ok := runtime.Caller(0) + if !ok { + t.Fatal("runtime.Caller(0) failed") + } + + repoRoot := core.PathDir(core.PathDir(core.PathDir(file))) + sourcePath := core.JoinPath(repoRoot, ".core", "reference", "fs.go") + embeddedPath := core.JoinPath(repoRoot, "pkg", "lib", "workspace", "default", ".core", "reference", "fs.go") + + source := testFs.Read(sourcePath) + if !source.OK { + t.Fatalf("failed to read %s", sourcePath) + } + + embedded := testFs.Read(embeddedPath) + if !embedded.OK { + t.Fatalf("failed to read %s", embeddedPath) + } + + if source.Value.(string) != embedded.Value.(string) { + t.Fatalf("%s diverged from %s", embeddedPath, sourcePath) + } +} + +func TestLib_ExtractWorkspace_Good_ReferenceFsSecureDefaults(t *testing.T) { + dir := t.TempDir() + data := &WorkspaceData{Repo: "test-repo", Task: "tighten extracted reference fs permissions"} + + requireExtractWorkspaceOK(t, ExtractWorkspace("default", dir, data)) + + path := core.JoinPath(dir, ".core", "reference", "fs.go") + r := testFs.Read(path) + if !r.OK { + t.Fatalf("failed to read %s", path) + } + + text := r.Value.(string) + for _, snippet := range []string{ + `return m.WriteMode(p, content, 0600)`, + `os.MkdirAll(filepath.Dir(full), 0700)`, + `os.Chmod(full, mode)`, + `os.MkdirAll(root, 0700)`, + `os.WriteFile(tmp, []byte(content), 0600)`, + `os.MkdirAll(vp.Value.(string), 0700)`, + `os.OpenFile(full, os.O_CREATE|os.O_TRUNC|os.O_WRONLY, 0600)`, + `os.OpenFile(full, os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0600)`, + } { + if !core.Contains(text, snippet) { + t.Errorf("%s missing secure permission default %q", path, snippet) + } + } +} + func TestLib_ExtractWorkspace_Good_ReferenceHeaders(t *testing.T) { dir := t.TempDir() data := &WorkspaceData{Repo: "test-repo", Task: "carry SPDX headers into workspace references"} diff --git a/pkg/lib/workspace/default/.core/reference/fs.go b/pkg/lib/workspace/default/.core/reference/fs.go index 7f75fa9..c44ba02 100644 --- a/pkg/lib/workspace/default/.core/reference/fs.go +++ b/pkg/lib/workspace/default/.core/reference/fs.go @@ -148,26 +148,29 @@ func (m *Fs) Read(p string) Result { } // Write saves content to file, creating parent directories as needed. -// Files are created with mode 0644. For sensitive files (keys, secrets), -// use WriteMode with 0600. +// Files are created with mode 0600 by default. +// Use WriteMode when broader access is intentional. func (m *Fs) Write(p, content string) Result { - return m.WriteMode(p, content, 0644) + return m.WriteMode(p, content, 0600) } // WriteMode saves content to file with explicit permissions. -// Use 0600 for sensitive files (encryption output, private keys, auth hashes). +// Use 0644 or 0755 only when broader access is intentional. func (m *Fs) WriteMode(p, content string, mode os.FileMode) Result { vp := m.validatePath(p) if !vp.OK { return vp } full := vp.Value.(string) - if err := os.MkdirAll(filepath.Dir(full), 0755); err != nil { + if err := os.MkdirAll(filepath.Dir(full), 0700); err != nil { return Result{err, false} } if err := os.WriteFile(full, []byte(content), mode); err != nil { return Result{err, false} } + if err := os.Chmod(full, mode); err != nil { + return Result{err, false} + } return Result{OK: true} } @@ -180,7 +183,7 @@ func (m *Fs) TempDir(prefix string) string { root := m.root if root == "" || root == "/" { root = os.TempDir() - } else if err := os.MkdirAll(root, 0755); err != nil { + } else if err := os.MkdirAll(root, 0700); err != nil { return "" } dir, err := os.MkdirTemp(root, prefix) @@ -212,12 +215,12 @@ func (m *Fs) WriteAtomic(p, content string) Result { return vp } full := vp.Value.(string) - if err := os.MkdirAll(filepath.Dir(full), 0755); err != nil { + if err := os.MkdirAll(filepath.Dir(full), 0700); err != nil { return Result{err, false} } tmp := full + ".tmp." + shortRand() - if err := os.WriteFile(tmp, []byte(content), 0644); err != nil { + if err := os.WriteFile(tmp, []byte(content), 0600); err != nil { return Result{err, false} } if err := os.Rename(tmp, full); err != nil { @@ -233,7 +236,7 @@ func (m *Fs) EnsureDir(p string) Result { if !vp.OK { return vp } - if err := os.MkdirAll(vp.Value.(string), 0755); err != nil { + if err := os.MkdirAll(vp.Value.(string), 0700); err != nil { return Result{err, false} } return Result{OK: true} @@ -309,10 +312,18 @@ func (m *Fs) Create(p string) Result { return vp } full := vp.Value.(string) - if err := os.MkdirAll(filepath.Dir(full), 0755); err != nil { + if err := os.MkdirAll(filepath.Dir(full), 0700); err != nil { return Result{err, false} } - return Result{}.New(os.Create(full)) + file, err := os.OpenFile(full, os.O_CREATE|os.O_TRUNC|os.O_WRONLY, 0600) + if err != nil { + return Result{err, false} + } + if err := file.Chmod(0600); err != nil { + file.Close() + return Result{err, false} + } + return Result{}.New(file) } // Append opens the named file for appending, creating it if it doesn't exist. @@ -322,10 +333,18 @@ func (m *Fs) Append(p string) Result { return vp } full := vp.Value.(string) - if err := os.MkdirAll(filepath.Dir(full), 0755); err != nil { + if err := os.MkdirAll(filepath.Dir(full), 0700); err != nil { return Result{err, false} } - return Result{}.New(os.OpenFile(full, os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0644)) + file, err := os.OpenFile(full, os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0600) + if err != nil { + return Result{err, false} + } + if err := file.Chmod(0600); err != nil { + file.Close() + return Result{err, false} + } + return Result{}.New(file) } // ReadStream returns a reader for the file content.