diff --git a/README.md b/README.md index 16e48aa..a108ce8 100644 --- a/README.md +++ b/README.md @@ -261,9 +261,12 @@ are now tracked only in the [issues](https://git.eeqj.de/sneak/mfer/issues). - `mfer gen` / `mfer gen .` - recurses under current directory and writes out an `index.mf` + - records every file's mode as `0000` unless given `--include-permissions`, + which records each file's permission bits (`0777` at most) - `mfer check` / `mfer check .` - verifies checksums of all files in manifest, displaying error and exiting - nonzero if any files are missing or corrupted + nonzero if any files are missing or corrupted, or have permission bits + other than the mode the manifest records, unless that is `0000` - warns about each file under the base directory that the manifest does not list, hidden files included; with `--no-extra-files` each one is a failure instead @@ -271,16 +274,18 @@ are now tracked only in the [issues](https://git.eeqj.de/sneak/mfer/issues). - fetches `/stuff/index.mf` and downloads all files listed in manifest into the current directory, or the one given with `--dest`, and assures cryptographic integrity of downloaded files. A file already there with the - size and hash the manifest lists is skipped. Once every file is in place, - the manifest is saved there as `index.mf`, so `mfer check` can verify the - tree later. Each file is downloaded to a temp file beside it, such as - `.a.txt.tmp` for `a.txt`, then moved into place. A manifest is refused - before any file is downloaded if it lists a file where fetch writes - another: at another listed file or a directory one is in, at the temp file - of a listed file, or at `index.mf` or `.index.mf.tmp` at the top of the - tree. Names are compared in any letter case, on every filesystem, since on - a case-insensitive one `A.txt` and `a.txt` are one file; a directory two - listed files are in must be spelled alike in both. + size, hash and recorded mode the manifest lists is skipped. Once every + file is in place, the manifest is saved there as `index.mf`, so + `mfer check` can verify the tree later. Each file is downloaded to a temp + file beside it, such as `.a.txt.tmp` for `a.txt`, given the mode the + manifest records unless that is `0000`, then moved into place. A manifest + is refused before any file is downloaded if it records a mode above + `0777`, or lists a file where fetch writes another: at another listed file + or a directory one is in, at the temp file of a listed file, or at + `index.mf` or `.index.mf.tmp` at the top of the tree. Names are compared + in any letter case, on every filesystem, since on a case-insensitive one + `A.txt` and `a.txt` are one file; a directory two listed files are in must + be spelled alike in both. - `mfer fetch --require-signature https://example.com/stuff/` - as above, but first refuses a manifest not signed by the key with that fingerprint, as `mfer check --require-signature` does, before downloading diff --git a/docs/FORMAT.md b/docs/FORMAT.md index 44e8e7c..94ec656 100644 --- a/docs/FORMAT.md +++ b/docs/FORMAT.md @@ -6,8 +6,11 @@ Version 1.0 An `.mf` file is a binary manifest that describes a directory tree of files, including their paths, sizes, and cryptographic checksums. It supports optional -GPG signatures for integrity verification and optional timestamps for metadata -preservation. +GPG signatures for integrity verification and optional timestamps and file +permissions for metadata preservation. + +Nothing goes in the 1.0 manifest that 1.0 does not read or write: no field is +reserved or kept for later use. ## File Structure @@ -50,7 +53,7 @@ enforce a decompression size limit to prevent decompression bombs. The reference implementation limits decompressed size to 256 MB. It writes zstd frames with a window of at most 8 MiB, the largest window the zstd format recommends decoders support, and refuses frames that ask for a larger one. It also refuses an inner -message whose file entries, hashes, timestamps and MIME types, counted at 160, +message whose file entries, hashes, timestamps and MIME types, counted at 176, 112, 64 and 16 bytes each, add up to more than 8 times its size. ## Inner Message (`MFFile`) @@ -77,6 +80,21 @@ Each file entry contains: | `mimeType` | 301 | string (optional) | MIME type | | `mtime` | 302 | Timestamp (optional) | Modification time | | `ctime` | 303 | Timestamp (optional) | Change time (inode metadata change) | +| `mode` | 304 | uint32 | Permission bits (see File Mode) | + +## File Mode + +`mode` holds a file's Unix permission bits, the nine `rwx` bits, so it is never +above `0777` (octal); the setuid, setgid and sticky bits are never recorded. +Writers record `0000` unless whoever creates the manifest asks for permissions. +`0000`, the proto3 default, means no mode was recorded: readers never check or +apply it. + +The reference implementation records modes when `gen` or `freshen` is given +`--include-permissions`. `check` fails a file whose permission bits differ from +a recorded mode other than `0000`. `fetch` sets a recorded mode other than +`0000` on each file it writes, and refuses a manifest that records a mode above +`0777` before it requests any file. ## Path Rules @@ -127,6 +145,7 @@ By default, manifests are generated deterministically: - File entries are sorted by `path` in **lexicographic byte order** - `createdAt` is omitted unless explicitly requested +- `mode` is `0000` unless explicitly requested This ensures that two independent runs over the same directory tree produce byte-identical `.mf` files (assuming file contents and metadata have not diff --git a/internal/cli/entry_test.go b/internal/cli/entry_test.go index a7ac2f6..60da46e 100644 --- a/internal/cli/entry_test.go +++ b/internal/cli/entry_test.go @@ -3,6 +3,7 @@ package cli import ( "bytes" + "encoding/json" "errors" "fmt" "io" @@ -374,6 +375,98 @@ func TestGenerateAndCheckCommand(t *testing.T) { assert.Equal(t, 0, exitCode, "check failed: %s", testStderr(t, opts)) } +// TestGenerateRecordsModeOnlyWhenAsked runs gen with and without +// --include-permissions: without it every mode is recorded as 0000, with +// it each file's permission bits. list -l and export show the recorded +// modes. +func TestGenerateRecordsModeOnlyWhenAsked(t *testing.T) { + t.Parallel() + + fs := afero.NewMemMapFs() + require.NoError(t, fs.MkdirAll(testDir, 0o755)) + writeTestFile(t, fs, "/testdir/notes.txt", "hello world") + writeTestFile(t, fs, "/testdir/run.sh", "#!/bin/sh\n") + require.NoError(t, fs.Chmod("/testdir/run.sh", 0o755)) + + for _, tc := range []struct { + flags []string + want map[string]string // recorded mode by path + }{ + {nil, map[string]string{"notes.txt": "0000", "run.sh": "0000"}}, + { + []string{"--" + flagIncludePermissions}, + map[string]string{"notes.txt": "0644", "run.sh": "0755"}, + }, + } { + opts := testOpts(slices.Concat( + []string{testApp, cmdGenerate, "-q", "-f", "-o", testOutput}, tc.flags, + []string{testDir}, + ), fs) + require.Equal(t, 0, runCLI(opts), "stderr: %s", testStderr(t, opts)) + + opts = testOpts([]string{testApp, cmdList, "-l", testOutput}, fs) + require.Equal(t, 0, runCLI(opts), "stderr: %s", testStderr(t, opts)) + + listed := map[string]string{} + out := strings.TrimSuffix(testStdout(t, opts), "\n") + + for _, line := range strings.Split(out, "\n") { + fields := strings.Split(line, "\t") // mode, size, mtime, path + require.Len(t, fields, 4, line) + listed[fields[3]] = fields[0] + } + + assert.Equal(t, tc.want, listed, "list -l %v", tc.flags) + + opts = testOpts([]string{testApp, cmdExport, testOutput}, fs) + require.Equal(t, 0, runCLI(opts), "stderr: %s", testStderr(t, opts)) + + var entries []ExportEntry + require.NoError(t, json.Unmarshal([]byte(testStdout(t, opts)), &entries)) + + exported := map[string]string{} + for _, e := range entries { + exported[e.Path] = e.Mode + } + + assert.Equal(t, tc.want, exported, "export %v", tc.flags) + } +} + +// TestCheckComparesRecordedMode changes a file's mode after gen: check +// must fail on it when gen recorded the file's mode, and pass when gen +// recorded 0000. +func TestCheckComparesRecordedMode(t *testing.T) { + t.Parallel() + + for _, tc := range []struct { + flags []string + exitCode int + }{ + {nil, 0}, + {[]string{"--" + flagIncludePermissions}, 1}, + } { + fs := afero.NewMemMapFs() + require.NoError(t, fs.MkdirAll(testDir, 0o755)) + writeTestFile(t, fs, testFile1, "hello world") + + opts := testOpts(slices.Concat( + []string{testApp, cmdGenerate, "-q", "-o", testMF}, tc.flags, + []string{testDir}, + ), fs) + require.Equal(t, 0, runCLI(opts), "stderr: %s", testStderr(t, opts)) + + require.NoError(t, fs.Chmod(testFile1, 0o600)) + + opts = testOpts([]string{testApp, cmdCheck, testFlagBase, testDir, testMF}, fs) + assert.Equal(t, tc.exitCode, runCLI(opts), "%v: %s", tc.flags, testStderr(t, opts)) + + if tc.exitCode != 0 { + assert.Contains(t, testStderr(t, opts), "MODE_MISMATCH: file1.txt") + } + } +} + // sharedWriter appends to a buffer shared with other sharedWriters, so // output written to stdout and stderr is kept in the order it was written. // Each write first waits for delay. diff --git a/internal/cli/errmsg_test.go b/internal/cli/errmsg_test.go index bfcd336..e7fc0e5 100644 --- a/internal/cli/errmsg_test.go +++ b/internal/cli/errmsg_test.go @@ -162,7 +162,7 @@ func signedManifest(t *testing.T, files map[string][]byte) []byte { for path, content := range files { _, err = b.AddFile(mfer.RelFilePath(path), mfer.FileSize(len(content)), - mfer.ModTime{}, bytes.NewReader(content), nil) + mfer.ModTime{}, 0, bytes.NewReader(content), nil) require.NoError(t, err) } diff --git a/internal/cli/export.go b/internal/cli/export.go index 3ceda69..8f9d2c0 100644 --- a/internal/cli/export.go +++ b/internal/cli/export.go @@ -18,6 +18,7 @@ type ExportEntry struct { Hashes []string `json:"hashes"` Mtime *string `json:"mtime,omitempty"` Ctime *string `json:"ctime,omitempty"` + Mode string `json:"mode"` // octal, "0000" when none was recorded } func (mfa *CLIApp) exportManifestOperation( @@ -49,6 +50,7 @@ func (mfa *CLIApp) exportManifestOperation( Path: f.GetPath(), Size: f.GetSize(), Hashes: make([]string, 0, len(f.GetHashes())), + Mode: fmt.Sprintf("%04o", f.GetMode()), } for _, h := range f.GetHashes() { diff --git a/internal/cli/export_test.go b/internal/cli/export_test.go index ef51e02..8bb700c 100644 --- a/internal/cli/export_test.go +++ b/internal/cli/export_test.go @@ -132,7 +132,7 @@ func TestListFromHTTPURL(t *testing.T) { exitCode := runCLI(&RunOptions{ Appname: testApp, - Args: []string{testApp, "list", server.URL + "/index.mf"}, + Args: []string{testApp, cmdList, server.URL + "/index.mf"}, Stdin: &bytes.Buffer{}, Stdout: &stdout, Stderr: &stderr, diff --git a/internal/cli/fetch.go b/internal/cli/fetch.go index cc0a8aa..999c329 100644 --- a/internal/cli/fetch.go +++ b/internal/cli/fetch.go @@ -95,6 +95,9 @@ var ( // writes another file. errNameClash = errors.New( "manifest lists a file where fetch writes another file") + // errModeOutOfRange indicates a manifest that lists a mode with more + // than the permission bits, such as setuid. + errModeOutOfRange = errors.New("manifest lists a mode outside 0777") ) // DownloadProgress reports the progress of a single file download. @@ -292,11 +295,11 @@ func downloadManifestFiles( } // alreadyPresent reports whether localPath under dest is a regular file -// with the size and one of the hashes the manifest lists for entry. It -// hashes the whole file, since a matching size alone would accept a -// corrupted or partly written one. A file it cannot read, or reaches only -// through a symlink, is not present: fetch downloads it, and the download -// reports the problem. +// with the size, the recorded mode if any, and one of the hashes the +// manifest lists for entry. It hashes the whole file, since a matching +// size alone would accept a corrupted or partly written one. A file it +// cannot read, or reaches only through a symlink, is not present: fetch +// downloads it, and the download reports the problem. func alreadyPresent(dest, localPath string, entry *mfer.MFFilePath) bool { if checkNoSymlinks(dest, localPath) != nil { return false @@ -309,6 +312,12 @@ func alreadyPresent(dest, localPath string, entry *mfer.MFFilePath) bool { return false } + // A recorded mode of 0 means none was recorded. + if entry.GetMode() != 0 && + info.Mode().Perm() != os.FileMode(entry.GetMode()).Perm() { + return false + } + // G304: localPath is a relative path that sanitizePath keeps inside // dest as text, and checkNoSymlinks just found no symlink in it. f, err := os.Open(path) //nolint:gosec // G304: see comment above @@ -414,9 +423,9 @@ func (mfa *CLIApp) fetchManifestOperation( // fetchManifest downloads the manifest at manifestURL and parses it, // enforcing --require-signature if it is given and refusing a manifest -// that lists a file where fetch writes another. It returns the manifest as -// downloaded, to be saved once the files are in place, and the files it -// lists. +// that lists a file where fetch writes another or a mode outside 0777. It +// returns the manifest as downloaded, to be saved once the files are in +// place, and the files it lists. func fetchManifest( ctx context.Context, cmd *cli.Command, client retryingClient, manifestURL string, ) ([]byte, []*mfer.MFFilePath, error) { @@ -460,6 +469,16 @@ func fetchManifest( return nil, nil, err } + // fetch sets each recorded mode on the file it writes, so a mode above + // 0777, which could carry setuid, setgid or sticky bits, is refused + // before any file is requested. + for _, f := range files { + if f.GetMode() > uint32(os.ModePerm) { + return nil, nil, fmt.Errorf("%w: %s (%#o)", + errModeOutOfRange, f.GetPath(), f.GetMode()) + } + } + log.Infof("manifest contains %d files", len(files)) return manifestData, files, nil @@ -866,6 +885,19 @@ func saveResponse( return err } + // A recorded mode of 0 means none was recorded. Of any other, only the + // permission bits are set, whatever the umask, so setuid, setgid and + // sticky never are, whichever caller passed the entry. + if entry.GetMode() != 0 { + err = out.Chmod(os.FileMode(entry.GetMode()).Perm()) + if err != nil { + _ = out.Close() + _ = os.Remove(filepath.Join(dest, tmpPath)) + + return fmt.Errorf("failed to set mode: %w", err) + } + } + // Set up hash computation h := sha256.New() diff --git a/internal/cli/fetch_test.go b/internal/cli/fetch_test.go index da9a434..832045c 100644 --- a/internal/cli/fetch_test.go +++ b/internal/cli/fetch_test.go @@ -4,6 +4,7 @@ package cli import ( "bytes" "context" + "crypto/sha256" "fmt" "io" "maps" @@ -19,9 +20,13 @@ import ( "testing" "time" + "github.com/google/uuid" + "github.com/klauspost/compress/zstd" + "github.com/multiformats/go-multihash" "github.com/spf13/afero" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "google.golang.org/protobuf/proto" "sneak.berlin/go/mfer/mfer" ) @@ -1299,6 +1304,193 @@ func TestFetchRefusesNamesEqualIgnoringCase(t *testing.T) { }) } +// TestFetchSetsRecordedMode fetches a tree whose manifest records the +// modes 0640 and 0755. Each file must get its mode whatever the umask, and +// check must pass on the result. After one file's mode is changed, a +// second fetch must download that file again, and only it, to restore its +// mode. +func TestFetchSetsRecordedMode(t *testing.T) { + t.Parallel() + + files := map[string][]byte{ + testFileTxt: []byte("a file"), + "tool.sh": []byte("#!/bin/sh\n"), + } + modes := map[string]os.FileMode{testFileTxt: 0o640, "tool.sh": 0o755} + + sourceFs := afero.NewMemMapFs() + for p, content := range files { + require.NoError(t, afero.WriteFile(sourceFs, "/"+p, content, modes[p])) + } + + scanner := mfer.NewScannerWithOptions(&mfer.ScannerOptions{ + Fs: sourceFs, + IncludePermissions: true, + }) + require.NoError(t, scanner.EnumerateFS(sourceFs, "/", nil)) + + var manifest bytes.Buffer + require.NoError(t, scanner.ToManifest(context.Background(), &manifest, nil)) + + tree := fetchTestHandler(manifest.Bytes(), files) + + var ( + mu sync.Mutex + requested []string + ) + + server := httptest.NewServer( + http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + mu.Lock() + + requested = append(requested, r.URL.Path) + + mu.Unlock() + + tree.ServeHTTP(w, r) + })) + defer server.Close() + + dest := t.TempDir() + fetch := []string{testApp, cmdFetch, "-q", "--" + flagDest, dest, server.URL} + + opts := testOpts(fetch, afero.NewOsFs()) + require.Equal(t, 0, runCLI(opts), testStderr(t, opts)) + + for p, mode := range modes { + info, err := os.Stat(filepath.Join(dest, p)) + require.NoError(t, err) + assert.Equal(t, mode, info.Mode().Perm(), p) + } + + opts = testOpts([]string{ + testApp, cmdCheck, "-q", testFlagBase, dest, + filepath.Join(dest, defaultManifestName), + }, afero.NewOsFs()) + require.Equal(t, 0, runCLI(opts), testStderr(t, opts)) + + require.NoError(t, os.Chmod(filepath.Join(dest, testFileTxt), 0o600)) + + mu.Lock() + requested = nil + mu.Unlock() + + opts = testOpts(fetch, afero.NewOsFs()) + require.Equal(t, 0, runCLI(opts), testStderr(t, opts)) + + info, err := os.Stat(filepath.Join(dest, testFileTxt)) + require.NoError(t, err) + assert.Equal(t, os.FileMode(0o640), info.Mode().Perm()) + + mu.Lock() + defer mu.Unlock() + + assert.ElementsMatch(t, + []string{"/" + defaultManifestName, "/" + testFileTxt}, requested) +} + +// TestFetchRefusesModeOutsidePermissionBits fetches manifests that record +// a mode with the setuid bit, once as Unix writes it (04755) and once as +// Go keeps it in os.FileMode. fetch must refuse both before it creates the +// destination or requests any file. +func TestFetchRefusesModeOutsidePermissionBits(t *testing.T) { + t.Parallel() + + files := map[string][]byte{testFileTxt: []byte("a file")} + + assertFetchRefused(t, manifestWithMode(t, testFileTxt, files[testFileTxt], 0o4755), + files, "manifest lists a mode outside 0777: file.txt (04755)") + + assertFetchRefused(t, + manifestWithMode(t, testFileTxt, files[testFileTxt], + uint32(os.ModeSetuid|0o755)), + files, "manifest lists a mode outside 0777: file.txt (040000755)") +} + +// TestDownloadFileSetsOnlyPermissionBits downloads a file whose entry +// records mode 0755 together with setuid, setgid or sticky, as Go keeps +// them in os.FileMode. fetchManifest would refuse such an entry; called +// directly, downloadFile must still set only the permission bits. +func TestDownloadFileSetsOnlyPermissionBits(t *testing.T) { + t.Parallel() + + content := []byte("#!/bin/sh\n") + + digest := sha256.Sum256(content) + hash, err := multihash.Encode(digest[:], multihash.SHA2_256) + require.NoError(t, err) + + server := httptest.NewServer( + http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + _, _ = w.Write(content) + })) + defer server.Close() + + for _, special := range []os.FileMode{os.ModeSetuid, os.ModeSetgid, os.ModeSticky} { + entry := &mfer.MFFilePath{ + Path: testFileTxt, + Size: int64(len(content)), + Hashes: []*mfer.MFFileChecksum{{MultiHash: hash}}, + Mode: uint32(special | 0o755), + } + + dest := t.TempDir() + + err := downloadFile(context.Background(), testClient(), + server.URL+"/"+testFileTxt, dest, testFileTxt, entry, nil) + require.NoError(t, err, special) + + info, err := os.Stat(filepath.Join(dest, testFileTxt)) + require.NoError(t, err) + assert.Equal(t, os.FileMode(0o755), info.Mode(), special) + } +} + +// manifestWithMode returns a manifest listing one file, path, with content +// and the given recorded mode. It is assembled by hand, since the builder +// never records a mode outside 0777. +func manifestWithMode(t *testing.T, path string, content []byte, mode uint32) []byte { + t.Helper() + + digest := sha256.Sum256(content) + hash, err := multihash.Encode(digest[:], multihash.SHA2_256) + require.NoError(t, err) + + id := uuid.New() + + inner, err := proto.Marshal(&mfer.MFFile{ + Version: mfer.MFFile_VERSION_ONE, + Files: []*mfer.MFFilePath{{ + Path: path, + Size: int64(len(content)), + Hashes: []*mfer.MFFileChecksum{{MultiHash: hash}}, + Mode: mode, + }}, + Uuid: id[:], + }) + require.NoError(t, err) + + encoder, err := zstd.NewWriter(nil) + require.NoError(t, err) + + compressed := encoder.EncodeAll(inner, nil) + require.NoError(t, encoder.Close()) + + sum := sha256.Sum256(compressed) + + outer, err := proto.Marshal(&mfer.MFFileOuter{ + Version: mfer.MFFileOuter_VERSION_ONE, + CompressionType: mfer.MFFileOuter_COMPRESSION_ZSTD, + Size: int64(len(inner)), + Sha256: sum[:], + Uuid: id[:], + InnerMessage: compressed, + }) + require.NoError(t, err) + + return append([]byte(mfer.MAGIC), outer...) +} + // builtManifest returns a manifest of files, built directly rather than // scanned, since a scan lists no hidden files and never a path starting // with "./". @@ -1309,7 +1501,7 @@ func builtManifest(t *testing.T, files map[string][]byte) []byte { for p, content := range files { _, err := builder.AddFile(mfer.RelFilePath(p), mfer.FileSize(len(content)), - mfer.ModTime(time.Now()), bytes.NewReader(content), nil) + mfer.ModTime(time.Now()), 0, bytes.NewReader(content), nil) require.NoError(t, err) } diff --git a/internal/cli/freshen.go b/internal/cli/freshen.go index ee7ae37..e39fbb5 100644 --- a/internal/cli/freshen.go +++ b/internal/cli/freshen.go @@ -48,6 +48,7 @@ type freshenEntry struct { path string size int64 mtime time.Time + mode fs.FileMode // mode to record, 0 for none needsHash bool // true if new or changed existing *mfer.MFFilePath // existing manifest entry if unchanged } @@ -55,13 +56,14 @@ type freshenEntry struct { // freshenScanner walks the filesystem and compares it against the // entries of an existing manifest. type freshenScanner struct { - fs afero.Fs - absBase string - excluded []fs.FileInfo // files left out of the listing - includeDotfiles bool - followSymlinks bool - showProgress bool - existingByPath map[string]*mfer.MFFilePath + fs afero.Fs + absBase string + excluded []fs.FileInfo // files left out of the listing + includeDotfiles bool + followSymlinks bool + includePermissions bool + showProgress bool + existingByPath map[string]*mfer.MFFilePath entries []*freshenEntry scanCount int64 @@ -93,6 +95,12 @@ func (s *freshenScanner) resolveSymlink(path string) (fs.FileInfo, bool) { // recordEntry classifies a scanned file as changed, unchanged, or added // relative to the existing manifest. func (s *freshenScanner) recordEntry(relPath string, info fs.FileInfo) { + // A mode of 0 records 0000, which means none was recorded. + var mode fs.FileMode + if s.includePermissions { + mode = info.Mode().Perm() + } + existing, inManifest := s.existingByPath[relPath] if !inManifest { s.added++ @@ -102,16 +110,17 @@ func (s *freshenScanner) recordEntry(relPath string, info fs.FileInfo) { path: relPath, size: info.Size(), mtime: info.ModTime(), + mode: mode, needsHash: true, }) return } - // Check if changed (size or mtime). An entry with no recorded mtime - // cannot be compared, so it counts as changed and gets re-hashed; - // silently treating the absent mtime as the Unix epoch would classify - // every such entry as changed without saying why. + // Check if changed (size, mtime, or the mode to record). An entry + // with no recorded mtime cannot be compared, so it counts as changed + // and gets re-hashed; silently treating the absent mtime as the Unix + // epoch would classify every such entry as changed without saying why. existingMtime, haveMtime := entryMtime(existing) if !haveMtime { log.Debugf("%s: manifest entry has no mtime, treating as changed", @@ -119,7 +128,8 @@ func (s *freshenScanner) recordEntry(relPath string, info fs.FileInfo) { } if !haveMtime || existing.GetSize() != info.Size() || - !existingMtime.Equal(info.ModTime()) { + !existingMtime.Equal(info.ModTime()) || + fs.FileMode(existing.GetMode()) != mode { s.changed++ log.Verbosef("M %s", relPath) @@ -127,6 +137,7 @@ func (s *freshenScanner) recordEntry(relPath string, info fs.FileInfo) { path: relPath, size: info.Size(), mtime: info.ModTime(), + mode: mode, needsHash: true, }) } else { @@ -136,6 +147,7 @@ func (s *freshenScanner) recordEntry(relPath string, info fs.FileInfo) { path: relPath, size: info.Size(), mtime: info.ModTime(), + mode: mode, needsHash: false, existing: existing, }) @@ -296,7 +308,7 @@ func (h *freshenHasher) processEntry(e *freshenEntry) error { h.hashedFiles++ // Add to builder with computed hash - err = addFileToBuilder(h.builder, e.path, e.size, e.mtime, hash) + err = addFileToBuilder(h.builder, e.path, e.size, e.mtime, e.mode, hash) if err != nil { return fmt.Errorf("failed to add %s: %w", e.path, err) } @@ -379,13 +391,14 @@ func (mfa *CLIApp) freshenScan( } scanner := &freshenScanner{ - fs: mfa.Fs, - absBase: absBase, - excluded: excluded, - includeDotfiles: cmd.Bool("include-dotfiles"), - followSymlinks: cmd.Bool("follow-symlinks"), - showProgress: showProgress, - existingByPath: existingByPath, + fs: mfa.Fs, + absBase: absBase, + excluded: excluded, + includeDotfiles: cmd.Bool("include-dotfiles"), + followSymlinks: cmd.Bool("follow-symlinks"), + includePermissions: cmd.Bool(flagIncludePermissions), + showProgress: showProgress, + existingByPath: existingByPath, } err := afero.Walk(mfa.Fs, absBase, scanner.walk) @@ -611,10 +624,12 @@ func hashFile(r io.Reader, progress func(int64)) ([]byte, int64, error) { // addFileToBuilder adds a new file entry to the builder func addFileToBuilder( - b *mfer.Builder, path string, size int64, mtime time.Time, hash []byte, + b *mfer.Builder, path string, size int64, mtime time.Time, mode fs.FileMode, + hash []byte, ) error { return b.AddFileWithHash( - mfer.RelFilePath(path), mfer.FileSize(size), mfer.ModTime(mtime), hash) + mfer.RelFilePath(path), mfer.FileSize(size), mfer.ModTime(mtime), mode, + hash) } // addExistingToBuilder adds an existing manifest entry to the builder. @@ -634,7 +649,7 @@ func addExistingToBuilder(b *mfer.Builder, entry *mfer.MFFilePath) error { err := b.AddFileWithHash(mfer.RelFilePath(entry.GetPath()), mfer.FileSize(entry.GetSize()), mfer.ModTime(mtime), - entry.GetHashes()[0].GetMultiHash()) + fs.FileMode(entry.GetMode()), entry.GetHashes()[0].GetMultiHash()) if err != nil { return fmt.Errorf( "manifest entry %s: %w (regenerate the manifest with mfer generate)", diff --git a/internal/cli/freshen_test.go b/internal/cli/freshen_test.go index 375220c..bb86985 100644 --- a/internal/cli/freshen_test.go +++ b/internal/cli/freshen_test.go @@ -99,6 +99,42 @@ func TestFreshenUnchanged(t *testing.T) { } } +// TestFreshenRecordsModeOnlyWhenAsked changes only the modes of a tree +// after gen made its manifest, which recorded every mode as 0000. freshen +// --include-permissions must record each file's permission bits, and a +// later freshen without it must record 0000 again, although no file's +// content or mtime changed. +func TestFreshenRecordsModeOnlyWhenAsked(t *testing.T) { + t.Parallel() + + fs := afero.NewOsFs() + root, manifestPath := setupFreshenDir(t, fs, + map[string]string{testFileTxt: "a file", testDirFile: "a file in dir"}) + require.NoError(t, fs.Chmod(filepath.Join(root, testFileTxt), 0o640)) + require.NoError(t, fs.Chmod(filepath.Join(root, testDirFile), 0o755)) + + recordedModes := func() map[string]uint32 { + modes := map[string]uint32{} + for _, f := range manifestFiles(t, fs, manifestPath) { + modes[f.GetPath()] = f.GetMode() + } + + return modes + } + + opts := testOpts([]string{ + testApp, cmdFreshen, "-q", "--" + flagIncludePermissions, + testFlagBase, root, manifestPath, + }, fs) + require.Equal(t, 0, runCLI(opts), "stderr: %s", testStderr(t, opts)) + assert.Equal(t, map[string]uint32{testFileTxt: 0o640, testDirFile: 0o755}, + recordedModes()) + + runFreshen(t, fs, root, manifestPath) + assert.Equal(t, map[string]uint32{testFileTxt: 0, testDirFile: 0}, + recordedModes()) +} + // assertManifestLists asserts that the manifest at manifestPath lists // exactly the files in want, each with the size and SHA-256 hash of its // content in want and the mtime of the file of that name under root. diff --git a/internal/cli/gen.go b/internal/cli/gen.go index 5630b29..16ac69c 100644 --- a/internal/cli/gen.go +++ b/internal/cli/gen.go @@ -92,10 +92,11 @@ func (mfa *CLIApp) collectInputPaths(args cli.Args) ([]string, error) { func (mfa *CLIApp) buildScannerOptions(cmd *cli.Command) *mfer.ScannerOptions { output := cmd.String("output") opts := &mfer.ScannerOptions{ - IncludeDotfiles: cmd.Bool("include-dotfiles"), - FollowSymLinks: cmd.Bool("follow-symlinks"), - IncludeTimestamps: cmd.Bool("include-timestamps"), - Fs: mfa.Fs, + IncludeDotfiles: cmd.Bool("include-dotfiles"), + FollowSymLinks: cmd.Bool("follow-symlinks"), + IncludeTimestamps: cmd.Bool("include-timestamps"), + IncludePermissions: cmd.Bool(flagIncludePermissions), + Fs: mfa.Fs, // Neither a manifest being replaced nor a temp file left by an // interrupted run belongs in the new manifest. ExcludePaths: []string{output, manifestTempPath(output)}, diff --git a/internal/cli/list.go b/internal/cli/list.go index 8119f2f..adc9e37 100644 --- a/internal/cli/list.go +++ b/internal/cli/list.go @@ -52,8 +52,8 @@ func (mfa *CLIApp) listManifestOperation(ctx context.Context, cmd *cli.Command) mtimeStr = mtime.Format(time.RFC3339) } - _, _ = fmt.Fprintf(mfa.Stdout, "%d\t%s\t%s%s", - f.GetSize(), mtimeStr, f.GetPath(), lineEnd) + _, _ = fmt.Fprintf(mfa.Stdout, "%04o\t%d\t%s\t%s%s", + f.GetMode(), f.GetSize(), mtimeStr, f.GetPath(), lineEnd) } else { _, _ = fmt.Fprintf(mfa.Stdout, "%s%s", f.GetPath(), lineEnd) } diff --git a/internal/cli/mfer.go b/internal/cli/mfer.go index 9e86b23..6fed3ff 100644 --- a/internal/cli/mfer.go +++ b/internal/cli/mfer.go @@ -21,12 +21,14 @@ const ( cmdFreshen = "freshen" cmdExport = "export" cmdFetch = "fetch" + cmdList = "list" cmdVersion = "version" - flagProgress = "progress" - flagTimeout = "timeout" - flagDest = "dest" - flagRequireSignature = "require-signature" + flagProgress = "progress" + flagTimeout = "timeout" + flagDest = "dest" + flagRequireSignature = "require-signature" + flagIncludePermissions = "include-permissions" manifestArgsUsage = "[manifest file]" @@ -167,6 +169,16 @@ func requireSignatureFlag() *cli.StringFlag { } } +// includePermissionsFlag returns the --include-permissions flag taken by the +// generate and freshen subcommands. +func includePermissionsFlag() *cli.BoolFlag { + return &cli.BoolFlag{ + Name: flagIncludePermissions, + Usage: "Record each file's permission bits in manifest " + + "(recorded as 0000 by default)", + } +} + func (mfa *CLIApp) generateCommand() *cli.Command { return &cli.Command{ Name: cmdGenerate, @@ -224,6 +236,7 @@ func (mfa *CLIApp) generateCommand() *cli.Command { Usage: "Include createdAt timestamp in manifest " + "(omitted by default for determinism)", }, + includePermissionsFlag(), ), } } @@ -307,6 +320,7 @@ func (mfa *CLIApp) freshenCommand() *cli.Command { Usage: "Include createdAt timestamp in manifest " + "(omitted by default for determinism)", }, + includePermissionsFlag(), ), } } @@ -340,7 +354,7 @@ func (mfa *CLIApp) versionCommand() *cli.Command { func (mfa *CLIApp) listCommand() *cli.Command { return &cli.Command{ - Name: "list", + Name: cmdList, Aliases: []string{"ls"}, Usage: "List files in manifest", ArgsUsage: manifestArgsUsage, @@ -350,7 +364,7 @@ func (mfa *CLIApp) listCommand() *cli.Command { &cli.BoolFlag{ Name: "long", Aliases: []string{"l"}, - Usage: "Show size and mtime", + Usage: "Show mode, size and mtime", }, &cli.BoolFlag{ Name: "print0", diff --git a/mfer/builder.go b/mfer/builder.go index 610c750..fc2e70c 100644 --- a/mfer/builder.go +++ b/mfer/builder.go @@ -8,6 +8,7 @@ import ( "errors" "fmt" "io" + "io/fs" "sort" "strings" "sync" @@ -137,12 +138,14 @@ func (b *Builder) SetSeed(seed string) { } // AddFile reads file content from reader, computes hashes, and adds to manifest. +// Only mode's permission bits (mode.Perm()) are recorded; 0 records none. // Progress updates are sent to the progress channel (if non-nil) without blocking. // Returns the number of bytes read. func (b *Builder) AddFile( path RelFilePath, size FileSize, mtime ModTime, + mode fs.FileMode, reader io.Reader, progress chan<- FileHashProgress, ) (FileSize, error) { @@ -198,6 +201,7 @@ func (b *Builder) AddFile( {MultiHash: mh}, }, Mtime: mtime.Timestamp(), + Mode: uint32(mode.Perm()), } b.mu.Lock() @@ -229,12 +233,14 @@ func (b *Builder) FileCount() int { // AddFileWithHash adds a file entry with a pre-computed hash. // This is useful when the hash is already known (e.g., from an existing manifest). +// Only mode's permission bits (mode.Perm()) are recorded; 0 records none. // Returns an error if path is invalid, size is negative, or hash is not a // multihash with a digest of at least 32 bytes, as long as SHA-256's. func (b *Builder) AddFileWithHash( path RelFilePath, size FileSize, mtime ModTime, + mode fs.FileMode, hash Multihash, ) error { err := ValidatePath(string(path)) @@ -268,6 +274,7 @@ func (b *Builder) AddFileWithHash( {MultiHash: hash}, }, Mtime: mtime.Timestamp(), + Mode: uint32(mode.Perm()), } b.mu.Lock() diff --git a/mfer/builder_test.go b/mfer/builder_test.go index 275c747..b8c6073 100644 --- a/mfer/builder_test.go +++ b/mfer/builder_test.go @@ -35,7 +35,7 @@ func TestBuilderAddFile(t *testing.T) { reader := bytes.NewReader(content) bytesRead, err := b.AddFile( - "test.txt", FileSize(len(content)), ModTime(time.Now()), reader, nil, + "test.txt", FileSize(len(content)), ModTime(time.Now()), 0, reader, nil, ) require.NoError(t, err) assert.Equal(t, FileSize(len(content)), bytesRead) @@ -49,7 +49,7 @@ func TestBuilderAddFileWithHash(t *testing.T) { hash, err := multihash.Encode(make([]byte, sha256.Size), multihash.SHA2_256) require.NoError(t, err) - err = b.AddFileWithHash("test.txt", 100, ModTime(time.Now()), hash) + err = b.AddFileWithHash("test.txt", 100, ModTime(time.Now()), 0, hash) require.NoError(t, err) assert.Equal(t, 1, b.FileCount()) } @@ -64,7 +64,7 @@ func TestBuilderAddFileWithHashValidation(t *testing.T) { t.Parallel() b := NewBuilder() - err := b.AddFileWithHash("", 100, ModTime(time.Now()), sha256Hash) + err := b.AddFileWithHash("", 100, ModTime(time.Now()), 0, sha256Hash) require.Error(t, err) assert.Contains(t, err.Error(), "path") }) @@ -73,7 +73,7 @@ func TestBuilderAddFileWithHashValidation(t *testing.T) { t.Parallel() b := NewBuilder() - err := b.AddFileWithHash("test.txt", -1, ModTime(time.Now()), sha256Hash) + err := b.AddFileWithHash("test.txt", -1, ModTime(time.Now()), 0, sha256Hash) require.Error(t, err) assert.Contains(t, err.Error(), "size") }) @@ -82,7 +82,7 @@ func TestBuilderAddFileWithHashValidation(t *testing.T) { t.Parallel() b := NewBuilder() - err := b.AddFileWithHash("test.txt", 100, ModTime(time.Now()), sha256Hash) + err := b.AddFileWithHash("test.txt", 100, ModTime(time.Now()), 0, sha256Hash) require.NoError(t, err) assert.Equal(t, 1, b.FileCount()) }) @@ -118,7 +118,7 @@ func TestBuilderAddFileWithHashRejectsBadHashes(t *testing.T) { t.Parallel() b := NewBuilder() - err := b.AddFileWithHash("test.txt", 100, ModTime(time.Now()), tt.hash) + err := b.AddFileWithHash("test.txt", 100, ModTime(time.Now()), 0, tt.hash) require.ErrorIs(t, err, tt.want) assert.Equal(t, 0, b.FileCount()) }) @@ -133,7 +133,7 @@ func TestBuilderBuild(t *testing.T) { reader := bytes.NewReader(content) _, err := b.AddFile( - "test.txt", FileSize(len(content)), ModTime(time.Now()), reader, nil, + "test.txt", FileSize(len(content)), ModTime(time.Now()), 0, reader, nil, ) require.NoError(t, err) @@ -196,7 +196,7 @@ func TestBuilderDeterministicOutput(t *testing.T) { for _, f := range files { r := bytes.NewReader([]byte(f.content)) _, err := b.AddFile( - RelFilePath(f.path), FileSize(len(f.content)), mtime, r, nil, + RelFilePath(f.path), FileSize(len(f.content)), mtime, 0, r, nil, ) require.NoError(t, err) } @@ -279,7 +279,7 @@ func TestBuilderAddFileSizeMismatch(t *testing.T) { reader := bytes.NewReader(content) // Declare wrong size - _, err := b.AddFile("test.txt", FileSize(100), ModTime(time.Now()), reader, nil) + _, err := b.AddFile("test.txt", FileSize(100), ModTime(time.Now()), 0, reader, nil) require.Error(t, err) assert.Contains(t, err.Error(), "size mismatch") } @@ -291,12 +291,12 @@ func TestBuilderAddFileInvalidPath(t *testing.T) { content := []byte("data") reader := bytes.NewReader(content) - _, err := b.AddFile("", FileSize(len(content)), ModTime(time.Now()), reader, nil) + _, err := b.AddFile("", FileSize(len(content)), ModTime(time.Now()), 0, reader, nil) require.Error(t, err) reader.Reset(content) _, err = b.AddFile( - "/absolute", FileSize(len(content)), ModTime(time.Now()), reader, nil, + "/absolute", FileSize(len(content)), ModTime(time.Now()), 0, reader, nil, ) assert.Error(t, err) } @@ -310,7 +310,7 @@ func TestBuilderAddFileWithProgress(t *testing.T) { progress := make(chan FileHashProgress, 100) bytesRead, err := b.AddFile( - "test.txt", FileSize(len(content)), ModTime(time.Now()), reader, progress, + "test.txt", FileSize(len(content)), ModTime(time.Now()), 0, reader, progress, ) close(progress) require.NoError(t, err) @@ -345,7 +345,7 @@ func TestBuilderBuildRoundTrip(t *testing.T) { for _, f := range files { reader := bytes.NewReader(f.content) _, err := b.AddFile( - RelFilePath(f.path), FileSize(len(f.content)), ModTime(now), reader, nil, + RelFilePath(f.path), FileSize(len(f.content)), ModTime(now), 0, reader, nil, ) require.NoError(t, err) } @@ -387,7 +387,7 @@ func TestBuilderBuildRoundTripLargeManifest(t *testing.T) { for i := range 4000 { path := RelFilePath(fmt.Sprintf("dir/file-%05d.txt", i)) - require.NoError(t, b.AddFileWithHash(path, FileSize(i), ModTime{}, hash)) + require.NoError(t, b.AddFileWithHash(path, FileSize(i), ModTime{}, 0, hash)) } var buf bytes.Buffer @@ -452,7 +452,7 @@ func TestManifestString(t *testing.T) { content := []byte("test") reader := bytes.NewReader(content) _, err := b.AddFile( - "test.txt", FileSize(len(content)), ModTime(time.Now()), reader, nil, + "test.txt", FileSize(len(content)), ModTime(time.Now()), 0, reader, nil, ) require.NoError(t, err) @@ -484,7 +484,7 @@ func TestBuilderOmitsCreatedAtByDefault(t *testing.T) { b := NewBuilder() content := []byte("hello") _, err := b.AddFile( - "test.txt", FileSize(len(content)), ModTime(time.Now()), + "test.txt", FileSize(len(content)), ModTime(time.Now()), 0, bytes.NewReader(content), nil, ) require.NoError(t, err) @@ -506,7 +506,7 @@ func TestBuilderIncludesCreatedAtWhenRequested(t *testing.T) { content := []byte("hello") _, err := b.AddFile( - "test.txt", FileSize(len(content)), ModTime(time.Now()), + "test.txt", FileSize(len(content)), ModTime(time.Now()), 0, bytes.NewReader(content), nil, ) require.NoError(t, err) @@ -532,7 +532,7 @@ func TestBuilderDeterministicFileOrder(t *testing.T) { content := []byte("content of " + name) _, err := b.AddFile( RelFilePath(name), FileSize(len(content)), - ModTime(time.Unix(1000, 0)), bytes.NewReader(content), nil, + ModTime(time.Unix(1000, 0)), 0, bytes.NewReader(content), nil, ) require.NoError(t, err) } diff --git a/mfer/checker.go b/mfer/checker.go index 2f37203..2d121a9 100644 --- a/mfer/checker.go +++ b/mfer/checker.go @@ -36,6 +36,7 @@ const ( StatusMissing // File not found on disk StatusSizeMismatch // File size differs from manifest StatusHashMismatch // File hash differs from manifest + StatusModeMismatch // File permission bits differ from a recorded mode StatusExtra // File exists on disk but not in manifest StatusError // Error occurred during verification ) @@ -50,6 +51,8 @@ func (s Status) String() string { return "SIZE_MISMATCH" case StatusHashMismatch: return "HASH_MISMATCH" + case StatusModeMismatch: + return "MODE_MISMATCH" case StatusExtra: return "EXTRA" case StatusError: @@ -408,11 +411,22 @@ func (c *Checker) checkFile(entry *MFFilePath, checkedBytes *FileSize) Result { return Result{Path: relPath, Status: StatusError, Message: err.Error()} } - // Check against all hashes in manifest (at least one must match) + // Check against all hashes in manifest (at least one must match), + // then against the recorded mode, where one is: 0 means none was. for _, hash := range entry.GetHashes() { - if bytes.Equal(computed, hash.GetMultiHash()) { - return Result{Path: relPath, Status: StatusOK} + if !bytes.Equal(computed, hash.GetMultiHash()) { + continue } + + if entry.GetMode() != 0 && info.Mode().Perm() != os.FileMode(entry.GetMode()) { + return Result{ + Path: relPath, + Status: StatusModeMismatch, + Message: "mode mismatch", + } + } + + return Result{Path: relPath, Status: StatusOK} } return Result{ diff --git a/mfer/checker_test.go b/mfer/checker_test.go index 92f6faa..d7180b4 100644 --- a/mfer/checker_test.go +++ b/mfer/checker_test.go @@ -34,6 +34,7 @@ func TestStatusString(t *testing.T) { {StatusMissing, "MISSING"}, {StatusSizeMismatch, "SIZE_MISMATCH"}, {StatusHashMismatch, "HASH_MISMATCH"}, + {StatusModeMismatch, "MODE_MISMATCH"}, {StatusExtra, "EXTRA"}, {StatusError, "ERROR"}, {Status(99), "UNKNOWN"}, @@ -59,7 +60,7 @@ func createTestManifest( for path, content := range files { reader := bytes.NewReader(content) _, err := builder.AddFile( - RelFilePath(path), FileSize(len(content)), ModTime(time.Now()), reader, nil, + RelFilePath(path), FileSize(len(content)), ModTime(time.Now()), 0, reader, nil, ) require.NoError(t, err) } @@ -279,7 +280,8 @@ func TestCheckMissingFile(t *testing.T) { missingCount++ assert.Equal(t, RelFilePath("missing.txt"), r.Path) - case StatusSizeMismatch, StatusHashMismatch, StatusExtra, StatusError: + case StatusSizeMismatch, StatusHashMismatch, StatusModeMismatch, + StatusExtra, StatusError: // Not expected in this test; counted assertions below will fail. } } @@ -349,6 +351,54 @@ func TestCheckHashMismatch(t *testing.T) { assert.Equal(t, RelFilePath(testFileName), r.Path) } +// A recorded mode other than 0000 that differs from the file's permission +// bits fails the check; a recorded 0000 is never checked. +func TestCheckMode(t *testing.T) { + t.Parallel() + + for _, tc := range []struct { + name string + recorded os.FileMode + onDisk os.FileMode + want Status + }{ + {"recorded mode matches", 0o640, 0o640, StatusOK}, + {"recorded mode differs", 0o640, 0o600, StatusModeMismatch}, + {"0000 is not checked", 0, 0o600, StatusOK}, + } { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + fs := afero.NewMemMapFs() + content := []byte("content") + + b := NewBuilder() + _, err := b.AddFile(testFileName, FileSize(len(content)), ModTime{}, + tc.recorded, bytes.NewReader(content), nil) + require.NoError(t, err) + + var buf bytes.Buffer + require.NoError(t, b.Build(context.Background(), &buf)) + require.NoError(t, afero.WriteFile(fs, testManifestPath, buf.Bytes(), 0o644)) + require.NoError(t, fs.MkdirAll(testDataDir, 0o755)) + require.NoError(t, afero.WriteFile(fs, + testDataDir+"/"+testFileName, content, tc.onDisk)) + + chk, err := NewChecker(&CheckerOptions{ + ManifestPath: testManifestPath, + BasePath: testDataDir, + Fs: fs, + }) + require.NoError(t, err) + + results := make(chan Result, 1) + require.NoError(t, chk.Check(context.Background(), results, nil)) + + assert.Equal(t, tc.want, (<-results).Status) + }) + } +} + func TestCheckWithProgress(t *testing.T) { t.Parallel() diff --git a/mfer/constants.go b/mfer/constants.go index 6d8267e..006e4b2 100644 --- a/mfer/constants.go +++ b/mfer/constants.go @@ -29,8 +29,8 @@ const ( // Bytes decoding sets aside for each file entry, hash, timestamp and // MIME type, however short its encoding. checkDecodedSize refuses an // inner message for which these add up to more than maxDecodedGrowth - // times its size. - decodedFileEntrySize = 160 + // times its size. The mode is held in the file entry itself. + decodedFileEntrySize = 176 decodedHashSize = 112 decodedTimestampSize = 64 decodedMIMETypeSize = 16 @@ -38,7 +38,7 @@ const ( // Each file entry mfer writes holds a path of at least one byte, a // multihash at least as long as SHA-256's 34 bytes (AddFileWithHash // refuses shorter ones) and a modification time: at least 47 bytes, - // counted at 336. So its manifests add up to at most about 7.15 times - // their size, and this limit is about 12% above that. + // counted at 352. So its manifests add up to at most about 7.49 times + // their size, and this limit is about 7% above that. maxDecodedGrowth = 8 ) diff --git a/mfer/deserialize_path_test.go b/mfer/deserialize_path_test.go index 7ac7221..596f7e0 100644 --- a/mfer/deserialize_path_test.go +++ b/mfer/deserialize_path_test.go @@ -119,11 +119,11 @@ func TestDeserializeRejectsInvalidEntryPaths(t *testing.T) { } // Entries of a path, an empty hash, an empty MIME type and empty modification -// and change times are counted at 416 bytes each (160 + 112 + 16 + 64 + 64) -// and take 16 bytes plus the path to encode. A 35-character path makes that -// 51 bytes, about 8.2 times: refused, and leaving any one of the five -// uncounted, even the MIME type, brings it under 8. A 37-character path makes -// it 53 bytes, about 7.8 times: loaded. +// and change times are counted at 432 bytes each (176 + 112 + 16 + 64 + 64) +// and take 16 bytes plus the path to encode. A 37-character path makes that +// 53 bytes, about 8.2 times: refused, and leaving any one of the five +// uncounted, even the MIME type, brings it under 8. A 39-character path makes +// it 55 bytes, about 7.9 times: loaded. func TestDeserializeRefusesEntriesThatDecodeTooLarge(t *testing.T) { t.Parallel() @@ -131,8 +131,8 @@ func TestDeserializeRefusesEntriesThatDecodeTooLarge(t *testing.T) { pathLen int refused bool }{ - {35, true}, - {37, false}, + {37, true}, + {39, false}, } for _, tt := range tests { @@ -215,7 +215,7 @@ func TestDeserializeLoadsDensestManifest(t *testing.T) { const files = 10000 for i := range files { name := RelFilePath(strconv.FormatInt(int64(i), 36)) - require.NoError(t, b.AddFileWithHash(name, 0, ModTime(time.Unix(0, 0)), hash)) + require.NoError(t, b.AddFileWithHash(name, 0, ModTime(time.Unix(0, 0)), 0, hash)) } var buf bytes.Buffer @@ -233,7 +233,7 @@ func TestDeserializeValidManifestRoundTrips(t *testing.T) { require.NoError(t, err) b := NewBuilder() - require.NoError(t, b.AddFileWithHash("dir/file.txt", 123, ModTime{}, hash)) + require.NoError(t, b.AddFileWithHash("dir/file.txt", 123, ModTime{}, 0, hash)) var buf bytes.Buffer require.NoError(t, b.Build(context.Background(), &buf)) diff --git a/mfer/gpg_test.go b/mfer/gpg_test.go index 1e24be0..cd92df7 100644 --- a/mfer/gpg_test.go +++ b/mfer/gpg_test.go @@ -187,7 +187,7 @@ func TestBuilderWithSigning(t *testing.T) { // Add a test file content := []byte("test file content") reader := bytes.NewReader(content) - _, err := b.AddFile("test.txt", FileSize(len(content)), ModTime{}, reader, nil) + _, err := b.AddFile("test.txt", FileSize(len(content)), ModTime{}, 0, reader, nil) require.NoError(t, err) // Build the manifest @@ -314,7 +314,7 @@ func TestManifestSignatureVerification(t *testing.T) { // Add a test file content := []byte("test file content for verification") reader := bytes.NewReader(content) - _, err := b.AddFile("test.txt", FileSize(len(content)), ModTime{}, reader, nil) + _, err := b.AddFile("test.txt", FileSize(len(content)), ModTime{}, 0, reader, nil) require.NoError(t, err) // Build the manifest @@ -344,7 +344,7 @@ func TestManifestTamperedSignatureFails(t *testing.T) { content := []byte("test file content") reader := bytes.NewReader(content) - _, err := b.AddFile("test.txt", FileSize(len(content)), ModTime{}, reader, nil) + _, err := b.AddFile("test.txt", FileSize(len(content)), ModTime{}, 0, reader, nil) require.NoError(t, err) var buf bytes.Buffer @@ -377,7 +377,7 @@ func TestBuilderWithoutSigning(t *testing.T) { // Add a test file content := []byte("test file content") reader := bytes.NewReader(content) - _, err := b.AddFile("test.txt", FileSize(len(content)), ModTime{}, reader, nil) + _, err := b.AddFile("test.txt", FileSize(len(content)), ModTime{}, 0, reader, nil) require.NoError(t, err) // Build the manifest diff --git a/mfer/mf.pb.go b/mfer/mf.pb.go index 621fffc..0e1813c 100644 --- a/mfer/mf.pb.go +++ b/mfer/mf.pb.go @@ -337,9 +337,11 @@ type MFFilePath struct { // gotta have at least one: Hashes []*MFFileChecksum `protobuf:"bytes,3,rep,name=hashes,proto3" json:"hashes,omitempty"` // optional per-file metadata - MimeType *string `protobuf:"bytes,301,opt,name=mimeType,proto3,oneof" json:"mimeType,omitempty"` - Mtime *Timestamp `protobuf:"bytes,302,opt,name=mtime,proto3,oneof" json:"mtime,omitempty"` - Ctime *Timestamp `protobuf:"bytes,303,opt,name=ctime,proto3,oneof" json:"ctime,omitempty"` + MimeType *string `protobuf:"bytes,301,opt,name=mimeType,proto3,oneof" json:"mimeType,omitempty"` + Mtime *Timestamp `protobuf:"bytes,302,opt,name=mtime,proto3,oneof" json:"mtime,omitempty"` + Ctime *Timestamp `protobuf:"bytes,303,opt,name=ctime,proto3,oneof" json:"ctime,omitempty"` + // permission bits, at most 0777; 0 when not recorded + Mode uint32 `protobuf:"varint,304,opt,name=mode,proto3" json:"mode,omitempty"` unknownFields protoimpl.UnknownFields sizeCache protoimpl.SizeCache } @@ -416,6 +418,13 @@ func (x *MFFilePath) GetCtime() *Timestamp { return nil } +func (x *MFFilePath) GetMode() uint32 { + if x != nil { + return x.Mode + } + return 0 +} + type MFFileChecksum struct { state protoimpl.MessageState `protogen:"open.v1"` // 1.0 golang implementation must write a multihash here @@ -561,7 +570,7 @@ const file_mf_proto_rawDesc = "" + "\n" + "_signatureB\t\n" + "\a_signerB\x10\n" + - "\x0e_signingPubKey\"\xf0\x01\n" + + "\x0e_signingPubKey\"\x85\x02\n" + "\n" + "MFFilePath\x12\x12\n" + "\x04path\x18\x01 \x01(\tR\x04path\x12\x12\n" + @@ -571,7 +580,8 @@ const file_mf_proto_rawDesc = "" + "\x05mtime\x18\xae\x02 \x01(\v2\n" + ".TimestampH\x01R\x05mtime\x88\x01\x01\x12&\n" + "\x05ctime\x18\xaf\x02 \x01(\v2\n" + - ".TimestampH\x02R\x05ctime\x88\x01\x01B\v\n" + + ".TimestampH\x02R\x05ctime\x88\x01\x01\x12\x13\n" + + "\x04mode\x18\xb0\x02 \x01(\rR\x04modeB\v\n" + "\t_mimeTypeB\b\n" + "\x06_mtimeB\b\n" + "\x06_ctime\".\n" + diff --git a/mfer/mf.proto b/mfer/mf.proto index 7ec8992..0d904ca 100644 --- a/mfer/mf.proto +++ b/mfer/mf.proto @@ -59,6 +59,8 @@ message MFFilePath { optional string mimeType = 301; optional Timestamp mtime = 302; optional Timestamp ctime = 303; + // permission bits, at most 0777; 0 when not recorded + uint32 mode = 304; } message MFFileChecksum { diff --git a/mfer/mf.proto.sha256 b/mfer/mf.proto.sha256 index 0a3492b..bd396c7 100644 --- a/mfer/mf.proto.sha256 +++ b/mfer/mf.proto.sha256 @@ -1 +1 @@ -fa6fceaba5553c8667c631994535fe5c307c8adb19f3ad289babf79a0f438650 mf.proto +3d4dcb0b2f4640dd3ae6bb48b833e9f26ae9771e2d3e88bd646e7f1724dda654 mf.proto diff --git a/mfer/mf_test.go b/mfer/mf_test.go index 489f411..83ef2d8 100644 --- a/mfer/mf_test.go +++ b/mfer/mf_test.go @@ -41,6 +41,7 @@ func TestFileEntryFieldsMatchSpec(t *testing.T) { "mimeType": 301, "mtime": 302, "ctime": 303, + "mode": 304, } got := map[string]protoreflect.FieldNumber{} diff --git a/mfer/scanner.go b/mfer/scanner.go index f04c7c6..129e503 100644 --- a/mfer/scanner.go +++ b/mfer/scanner.go @@ -52,6 +52,9 @@ type ScannerOptions struct { // IncludeTimestamps includes a createdAt timestamp in the manifest // (default: omit for determinism). IncludeTimestamps bool + // IncludePermissions records each file's permission bits, 0777 at + // most, in the manifest (default: record 0000). + IncludePermissions bool // Fs is the filesystem to use, defaults to OsFs if nil. Fs afero.Fs // SigningOptions holds GPG signing options (nil = no signing). @@ -69,6 +72,7 @@ type FileEntry struct { Size FileSize // File size in bytes Mtime ModTime // Last modification time Ctime time.Time // Creation time (platform-dependent) + Mode fs.FileMode // Permission bits (Perm() of the file's mode) } // Scanner accumulates files and generates manifests from them. @@ -353,11 +357,18 @@ func (s *Scanner) scanFile( }(scannedBytes, scannedFiles) } + // A mode of 0 records 0000, which means none was recorded. + var mode fs.FileMode + if s.options.IncludePermissions { + mode = entry.Mode + } + // Add to manifest with progress channel bytesRead, err := builder.AddFile( entry.Path, entry.Size, entry.Mtime, + mode, f, fileProgress, ) @@ -461,6 +472,7 @@ func (s *Scanner) enumerateFileWithInfo( AbsPath: AbsFilePath(absPath), Size: FileSize(info.Size()), Mtime: ModTime(info.ModTime()), + Mode: info.Mode().Perm(), // Note: Ctime not available from fs.FileInfo on all platforms // Will need platform-specific code to extract it } diff --git a/mfer/scanner_test.go b/mfer/scanner_test.go index 58e4605..75b4b5b 100644 --- a/mfer/scanner_test.go +++ b/mfer/scanner_test.go @@ -4,6 +4,7 @@ package mfer import ( "bytes" "context" + "os" "testing" "time" @@ -350,6 +351,46 @@ func TestScannerFileEntryFields(t *testing.T) { assert.WithinDuration(t, now, time.Time(entry.Mtime), 2*time.Second) } +// A manifest records every mode as 0000 unless the creator asks for +// permissions; then it records each file's permission bits and never the +// setuid, setgid or sticky bits. +func TestScannerRecordsModeOnlyWhenAsked(t *testing.T) { + t.Parallel() + + fs := afero.NewMemMapFs() + require.NoError(t, afero.WriteFile(fs, "/"+testFile1, []byte("a"), 0o640)) + require.NoError(t, afero.WriteFile(fs, "/run.sh", []byte("b"), 0o755)) + require.NoError(t, afero.WriteFile(fs, "/su", []byte("c"), 0o755)) + require.NoError(t, fs.Chmod("/su", 0o755|os.ModeSetuid|os.ModeSetgid|os.ModeSticky)) + + for _, tc := range []struct { + includePermissions bool + want map[string]uint32 + }{ + {false, map[string]uint32{testFile1: 0, "run.sh": 0, "su": 0}}, + {true, map[string]uint32{testFile1: 0o640, "run.sh": 0o755, "su": 0o755}}, + } { + s := NewScannerWithOptions(&ScannerOptions{ + Fs: fs, + IncludePermissions: tc.includePermissions, + }) + require.NoError(t, s.EnumerateFS(fs, "/", nil)) + + var buf bytes.Buffer + require.NoError(t, s.ToManifest(context.Background(), &buf, nil)) + + m, err := NewManifestFromReader(&buf) + require.NoError(t, err) + + got := map[string]uint32{} + for _, f := range m.Files() { + got[f.GetPath()] = f.GetMode() + } + + assert.Equal(t, tc.want, got, "IncludePermissions: %v", tc.includePermissions) + } +} + func TestScannerLargeFileEnumeration(t *testing.T) { t.Parallel()