From 4455a8c5568cabf776ef4cfddc9b85e18697ce70 Mon Sep 17 00:00:00 2001 From: Francesc Campoy Date: Fri, 10 Apr 2026 18:21:51 -0700 Subject: [PATCH 1/3] Use 0644 instead of 0666 for file mode in openFile On systems with a permissive umask (e.g. Docker containers), 0666 produces world-writable files. 0644 is the conventional permission for non-executable source files. Closes #82 --- main.go | 2 +- main_test.go | 23 +++++++++++++++++++++++ 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/main.go b/main.go index 4218595..54e7170 100644 --- a/main.go +++ b/main.go @@ -125,7 +125,7 @@ type file interface { // replaced by testing functions. var openFile = func(name string) (file, error) { - return os.OpenFile(name, os.O_RDWR, 0666) + return os.OpenFile(name, os.O_RDWR, 0644) } func processFile(path string, rewrite, doDiff bool) (foundDiff bool, err error) { diff --git a/main_test.go b/main_test.go index 76c4bb7..788cf3a 100644 --- a/main_test.go +++ b/main_test.go @@ -23,6 +23,29 @@ import ( "github.com/campoy/embedmd/internal/testutil" ) +func TestOpenFilePermissions(t *testing.T) { + f, err := os.CreateTemp(t.TempDir(), "embedmd-perm-*.md") + if err != nil { + t.Fatal(err) + } + name := f.Name() + f.Close() + + got, err := os.OpenFile(name, os.O_RDWR, 0644) + if err != nil { + t.Fatalf("openFile: %v", err) + } + got.Close() + + info, err := os.Stat(name) + if err != nil { + t.Fatal(err) + } + if info.Mode()&0022 != 0 { + t.Errorf("file has group/world write bits set: %v", info.Mode()) + } +} + func TestEmbedStreams(t *testing.T) { tc := []struct { name string From a95c0e5567ad2e3434895de994023cfeeb0aaa7e Mon Sep 17 00:00:00 2001 From: Francesc Campoy Date: Sat, 11 Apr 2026 10:43:51 -0700 Subject: [PATCH 2/3] Preserve original file permissions when rewriting Stat the file before opening it, then explicitly Chmod it back to the original mode after writing. This makes permission preservation robust and explicit, rather than relying on the implicit behaviour of the OS when writing to an already-open file handle. The mode argument to os.OpenFile is dropped (set to 0) since it has no effect when O_CREATE is absent. The Chmod call is now the authoritative place where the file's mode is handled. --- main.go | 20 ++++++++++++--- main_test.go | 72 ++++++++++++++++++++++++++++++++++++---------------- 2 files changed, 66 insertions(+), 26 deletions(-) diff --git a/main.go b/main.go index 54e7170..f7a41f0 100644 --- a/main.go +++ b/main.go @@ -124,15 +124,24 @@ type file interface { } // replaced by testing functions. -var openFile = func(name string) (file, error) { - return os.OpenFile(name, os.O_RDWR, 0644) -} +var ( + openFile = func(name string) (file, error) { + return os.OpenFile(name, os.O_RDWR, 0) + } + statFile = os.Stat + chmodFile = os.Chmod +) func processFile(path string, rewrite, doDiff bool) (foundDiff bool, err error) { if filepath.Ext(path) != ".md" { return false, fmt.Errorf("not a markdown file") } + info, err := statFile(path) + if err != nil { + return false, err + } + f, err := openFile(path) if err != nil { return false, err @@ -164,7 +173,10 @@ func processFile(path string, rewrite, doDiff bool) (foundDiff bool, err error) if err != nil { return false, fmt.Errorf("could not write: %v", err) } - return false, f.Truncate(int64(n)) + if err := f.Truncate(int64(n)); err != nil { + return false, err + } + return false, chmodFile(path, info.Mode()) } _, err = io.Copy(stdout, buf) diff --git a/main_test.go b/main_test.go index 788cf3a..a5714c1 100644 --- a/main_test.go +++ b/main_test.go @@ -5,7 +5,7 @@ // http://www.apache.org/licenses/LICENSE-2.0 // // Unless required by applicable law or agreed to writing, software distributed -// under the License is distributed on a "AS IS" BASIS, WITHOUT WARRANTIES OR +// under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR // CONDITIONS OF ANY KIND, either express or implied. // // See the License for the specific language governing permissions and @@ -16,35 +16,24 @@ package main import ( "bytes" "io" + "io/fs" "os" "strings" "testing" + "time" "github.com/campoy/embedmd/internal/testutil" ) -func TestOpenFilePermissions(t *testing.T) { - f, err := os.CreateTemp(t.TempDir(), "embedmd-perm-*.md") - if err != nil { - t.Fatal(err) - } - name := f.Name() - f.Close() +// fakeInfo implements fs.FileInfo with a fixed mode. +type fakeInfo struct{ mode fs.FileMode } - got, err := os.OpenFile(name, os.O_RDWR, 0644) - if err != nil { - t.Fatalf("openFile: %v", err) - } - got.Close() - - info, err := os.Stat(name) - if err != nil { - t.Fatal(err) - } - if info.Mode()&0022 != 0 { - t.Errorf("file has group/world write bits set: %v", info.Mode()) - } -} +func (f fakeInfo) Name() string { return "" } +func (f fakeInfo) Size() int64 { return 0 } +func (f fakeInfo) Mode() fs.FileMode { return f.mode } +func (f fakeInfo) ModTime() time.Time { return time.Time{} } +func (f fakeInfo) IsDir() bool { return false } +func (f fakeInfo) Sys() interface{} { return nil } func TestEmbedStreams(t *testing.T) { tc := []struct { @@ -127,6 +116,11 @@ func TestEmbedFiles(t *testing.T) { } defer func(f func(string) (file, error)) { openFile = f }(openFile) + defer func(f func(string) (fs.FileInfo, error)) { statFile = f }(statFile) + defer func(f func(string, fs.FileMode) error) { chmodFile = f }(chmodFile) + + statFile = func(string) (fs.FileInfo, error) { return fakeInfo{mode: 0644}, nil } + chmodFile = func(string, fs.FileMode) error { return nil } for _, tt := range tc { f := newFakeFile(tt.in) @@ -143,7 +137,41 @@ func TestEmbedFiles(t *testing.T) { if got := f.buf.String(); tt.out != got { t.Errorf("case [%s]: expected output \n%q; got\n%q", tt.name, tt.out, got) } + } +} + +// TestFilePermissionsPreserved verifies that processFile restores the +// original file permissions after a rewrite. +func TestFilePermissionsPreserved(t *testing.T) { + f, err := os.CreateTemp(t.TempDir(), "embedmd-perm-*.md") + if err != nil { + t.Fatal(err) + } + if _, err := f.WriteString("hello\n"); err != nil { + t.Fatal(err) + } + f.Close() + want := fs.FileMode(0600) + if err := os.Chmod(f.Name(), want); err != nil { + t.Fatal(err) + } + + defer func(fn func(string) (file, error)) { openFile = fn }(openFile) + openFile = func(name string) (file, error) { + return os.OpenFile(name, os.O_RDWR, 0) + } + + if _, err := processFile(f.Name(), true, false); err != nil { + t.Fatalf("processFile: %v", err) + } + + info, err := os.Stat(f.Name()) + if err != nil { + t.Fatal(err) + } + if got := info.Mode(); got != want { + t.Errorf("permissions changed: got %v, want %v", got, want) } } From 58837719ad886f131f825a6175ef190c5872a1f1 Mon Sep 17 00:00:00 2001 From: Francesc Campoy Date: Sat, 11 Apr 2026 10:54:50 -0700 Subject: [PATCH 3/3] Use mode 0 in openFile to clarify permissions are not set MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit os.OpenFile without O_CREATE never modifies an existing file's permissions — WriteAt and Truncate don't either — so the mode argument of 0666 was both misleading and a no-op. Passing 0 makes it explicit that no permission bits are being set here. --- main.go | 22 ++++++---------------- main_test.go | 52 ---------------------------------------------------- 2 files changed, 6 insertions(+), 68 deletions(-) diff --git a/main.go b/main.go index f7a41f0..4d0113f 100644 --- a/main.go +++ b/main.go @@ -124,24 +124,17 @@ type file interface { } // replaced by testing functions. -var ( - openFile = func(name string) (file, error) { - return os.OpenFile(name, os.O_RDWR, 0) - } - statFile = os.Stat - chmodFile = os.Chmod -) +var openFile = func(name string) (file, error) { + // The permission bits (third argument) are only used when O_CREATE is set. + // Since we only open existing files, 0 is passed to make that clear. + return os.OpenFile(name, os.O_RDWR, 0) +} func processFile(path string, rewrite, doDiff bool) (foundDiff bool, err error) { if filepath.Ext(path) != ".md" { return false, fmt.Errorf("not a markdown file") } - info, err := statFile(path) - if err != nil { - return false, err - } - f, err := openFile(path) if err != nil { return false, err @@ -173,10 +166,7 @@ func processFile(path string, rewrite, doDiff bool) (foundDiff bool, err error) if err != nil { return false, fmt.Errorf("could not write: %v", err) } - if err := f.Truncate(int64(n)); err != nil { - return false, err - } - return false, chmodFile(path, info.Mode()) + return false, f.Truncate(int64(n)) } _, err = io.Copy(stdout, buf) diff --git a/main_test.go b/main_test.go index a5714c1..3bbba5a 100644 --- a/main_test.go +++ b/main_test.go @@ -16,25 +16,13 @@ package main import ( "bytes" "io" - "io/fs" "os" "strings" "testing" - "time" "github.com/campoy/embedmd/internal/testutil" ) -// fakeInfo implements fs.FileInfo with a fixed mode. -type fakeInfo struct{ mode fs.FileMode } - -func (f fakeInfo) Name() string { return "" } -func (f fakeInfo) Size() int64 { return 0 } -func (f fakeInfo) Mode() fs.FileMode { return f.mode } -func (f fakeInfo) ModTime() time.Time { return time.Time{} } -func (f fakeInfo) IsDir() bool { return false } -func (f fakeInfo) Sys() interface{} { return nil } - func TestEmbedStreams(t *testing.T) { tc := []struct { name string @@ -116,11 +104,6 @@ func TestEmbedFiles(t *testing.T) { } defer func(f func(string) (file, error)) { openFile = f }(openFile) - defer func(f func(string) (fs.FileInfo, error)) { statFile = f }(statFile) - defer func(f func(string, fs.FileMode) error) { chmodFile = f }(chmodFile) - - statFile = func(string) (fs.FileInfo, error) { return fakeInfo{mode: 0644}, nil } - chmodFile = func(string, fs.FileMode) error { return nil } for _, tt := range tc { f := newFakeFile(tt.in) @@ -140,41 +123,6 @@ func TestEmbedFiles(t *testing.T) { } } -// TestFilePermissionsPreserved verifies that processFile restores the -// original file permissions after a rewrite. -func TestFilePermissionsPreserved(t *testing.T) { - f, err := os.CreateTemp(t.TempDir(), "embedmd-perm-*.md") - if err != nil { - t.Fatal(err) - } - if _, err := f.WriteString("hello\n"); err != nil { - t.Fatal(err) - } - f.Close() - - want := fs.FileMode(0600) - if err := os.Chmod(f.Name(), want); err != nil { - t.Fatal(err) - } - - defer func(fn func(string) (file, error)) { openFile = fn }(openFile) - openFile = func(name string) (file, error) { - return os.OpenFile(name, os.O_RDWR, 0) - } - - if _, err := processFile(f.Name(), true, false); err != nil { - t.Fatalf("processFile: %v", err) - } - - info, err := os.Stat(f.Name()) - if err != nil { - t.Fatal(err) - } - if got := info.Mode(); got != want { - t.Errorf("permissions changed: got %v, want %v", got, want) - } -} - type fakeFile struct { io.ReadCloser buf bytes.Buffer