From e555631fe4eac3afa252e301be6835315955ef6b Mon Sep 17 00:00:00 2001 From: Ben Boyter Date: Tue, 3 Nov 2026 14:52:19 +1100 Subject: [PATCH] Decide own entries through the chosen directory, judge unchosen relative audits through every directory, and judge ties more cheaply An empty file named like the manifest, listed in a manifest kept elsewhere, was dropped as the manifest listing itself because through the manifest's directory it names the manifest, so deleting it passed. Such records are now decided once the directory is chosen. With no directory chosen, a relative sum line naming a deleted file through one directory could be grouped through another with an identical file listed absolutely, hiding the deletion. Without an invoked from directory the audit is now checked through every directory, each grouping lines through its own, and hashdeep files without one only treat records naming the same path through every directory as duplicates. A sum file listing files relatively and absolutely now fails from another directory, as md5sum -c does. Sum lines reaching a named file through a symbolic link are still grouped. A passing tie is judged once per directory rather than once more, and warnings come from the judgement reported. Co-Authored-By: Claude Opus 5.5 --- README.md | 6 +- main_audit_test.go | 47 +++++++++++++++ processor/audit.go | 39 ++++++------ processor/audit_parser.go | 124 ++++++++++++++++++++++++++++---------- processor/audit_test.go | 43 +++++++++++++ processor/formatters.go | 84 ++++++++++++++++---------- test-all.sh | 13 ++-- test-audit.sh | 44 ++++++++++++++ 8 files changed, 310 insertions(+), 90 deletions(-) diff --git a/README.md b/README.md index adc779c..4ba738f 100644 --- a/README.md +++ b/README.md @@ -373,9 +373,9 @@ Some other points worth knowing: - A file which cannot be read, including files in a directory which cannot be read, is reported as could not be read in `-v` output, counted as missing when it is in the audit file and not counted otherwise, and `hashit` exits with `2` rather than `1`. - The same file supplied more than once by the same absolute path, such as through `.` and `./sub`, is only counted once. The same file reached through a symbolic link is a different path, so is counted again. - Modification times are recorded with `--mtime` but are not compared by an audit. -- A `sum` file which lists the same file more than once, such as once with `md5sum` and again with `sha1sum`, or as both `./file` and `file`, has every line checked as `md5sum -c` does. Lines are the same file when they name it through the directory the audit is matched through, including through a symbolic link, and a line is only not reported on its own when another line was checked against that file. A file listed with different values of the same hash is reported as modified with a warning. Lines which only name the same file through another directory name different files, so a `sum` file at the root of a tree listing `./README` and `./sub/README` passes when audited from `sub`. When as many files match through another directory as through the one chosen, lines naming the same file through either must agree, as which the `sum` file is relative to is not known. In a `hashdeep` file only the first entry for a path is used, with a warning. In a SQLite database a row for a path already listed with the same values is not counted, with a warning, while one with other values is reported as missing. Entries are only the same path when they name the same file through the directory the audit is matched through, so after a tree is copied elsewhere an absolute entry into the original is audited on its own. When every file matched an absolute entry no directory is chosen, and entries are the same path when they name the same file through the invoked from directory, or for a `sum` file, which has none, through any of them. A SQLite database whose `file_hashes` table does not have a text `filepath` on every row as its primary key compared exactly, as one not created by hashit may not, is refused rather than audited. +- A `sum` file which lists the same file more than once, such as once with `md5sum` and again with `sha1sum`, or as both `./file` and `file`, has every line checked as `md5sum -c` does. Lines are the same file when they name it through the directory the audit is matched through, including through a symbolic link, and a line is only not reported on its own when another line was checked against that file. A file listed with different values of the same hash is reported as modified with a warning. Lines which only name the same file through another directory name different files, so a `sum` file at the root of a tree listing `./README` and `./sub/README` passes when audited from `sub`. When as many files match through another directory as through the one chosen, lines naming the same file through either must agree, as which the `sum` file is relative to is not known. In a `hashdeep` file only the first entry for a path is used, with a warning. Without an invoked from directory, entries are only the same path when they name it through every directory, as for a `sum` file. In a SQLite database a row for a path already listed with the same values is not counted, with a warning, while one with other values is reported as missing. Entries are only the same path when they name the same file through the directory the audit is matched through, so after a tree is copied elsewhere an absolute entry into the original is audited on its own. When every file matched an absolute entry no directory is chosen, and entries are the same path when they name the same file through the invoked from directory. Without one, as for a `sum` file, nothing decided which directory relative entries are relative to, so the audit is checked through each directory as for a tie. Otherwise a deleted `a` listed beside an identical `s/a` listed absolutely would pass when audited from `s`. So a `sum` file listing files both relatively and absolutely passes in its own directory and fails from another, as `md5sum -c` does. A SQLite database whose `file_hashes` table does not have a text `filepath` on every row as its primary key compared exactly, as one not created by hashit may not, is refused rather than audited. - The audit file of an empty directory passes when audited against an empty directory, as does an audit file which is empty or only contains comments, which `md5sum -c` treats as an error. `hashdeep` reports an error for an audit file with no files in it. -- An audit file written by a shell redirect into the directory being hashed, such as `hashit --format hashdeep . > manifest.txt`, lists itself as an empty file. That entry is ignored when auditing. Without an invoked from directory, as for a `sum` file, an entry is only taken to be the audit file when it names it through the directory the audit is matched through, so another file of the same name, such as `data/data/hashes.md5` listed in `data/hashes.md5` and audited from the parent of `data`, is still audited. `--output manifest.txt` avoids the entry entirely and is recommended, especially for the `sum` format which writes as it goes and so may record part of itself, which then fails when audited from another directory. +- An audit file written by a shell redirect into the directory being hashed, such as `hashit --format hashdeep . > manifest.txt`, lists itself as an empty file. That entry is ignored when auditing. Unless it names the audit file through the invoked from directory of a `hashdeep` file, an entry is only taken to be the audit file when it names it through the directory the audit is matched through, so another file of the same name is still audited. Examples are `data/data/hashes.md5` listed in `data/hashes.md5` and audited from the parent of `data`, or an empty `t/hashes.md5` listed in `m/hashes.md5` written from `t`. `--output manifest.txt` avoids the entry entirely and is recommended, especially for the `sum` format which writes as it goes and so may record part of itself, which then fails when audited from another directory. - Concatenated `hashdeep` files are read, but only the first `## Invoked from:` line is used, so files concatenated from different directories may report moved files. `hashdeep` also fails to audit these. #### Modification Times @@ -394,7 +394,7 @@ hashdeep: Audit passed #### Paths in Audit Files -Relative paths in a `hashdeep` file are resolved against the directory recorded in its `## Invoked from:` line, so an audit file created with `hashit --format hashdeep .` can be audited from anywhere using either a relative or absolute path. When the invoked from directory does not contain the files, such as after they have been copied elsewhere, paths are also resolved against the directory containing the audit file, being where the file really is and then where the path supplied for it is when that is a symbolic link, and then the current directory. An audit is matched through only one of these, the one through which the most files match their records, or the first of those in this order, so a file found only through another, such as a file moved to where the directory containing the audit file places its record, is reported as moved or new. A file counts towards every directory through which a record names it, except a file an absolute record names, and is checked against the record naming it through the chosen directory, so a file which another record also names through a different directory, such as `meta/README` named by `./README` through a manifest kept in `meta`, is still checked against its own record. When as many files match through another directory as through the first of them, which the paths are relative to is not decided by the files on disk, so the audit is checked through each and passes only when it passes through every one, with a warning naming the directory it fails through. Otherwise a tree copied with its manifest kept in `meta`, where `./README` and `./meta/README` both name `meta/README`, could pass after `README` was moved onto `meta/README`. Relative paths in `sum` files have no invoked from directory, so are resolved against the current directory first. Matching is case sensitive on every platform. Paths are always compared as absolute paths, so the path to audit may be supplied as relative or absolute, and `./file` and `file` are the same file. So a manifest stored at the root of a set of files, created from that directory, can be audited from anywhere after the files are copied or shipped elsewhere. Symbolic links in the path to audit are not resolved, so a manifest created through `real/` and audited through a link to it reports every file as moved, as `hashdeep` does. When the current directory was reached through a symbolic link, paths through where it, or a directory containing it, really is are compared through the link, so a manifest of absolute paths through `real/` audited with `.` from the link passes. A path containing `..`, such as `../dir` or `link/../dir`, names the directory the operating system reads, being the parent of the directory a symbolic link points to rather than the directory containing the link, in the paths supplied, the paths recorded in audit files and with `--absolute`. Where removing the `..` names the same directory, such as from a current directory reached through a symbolic link, the path is kept as written, A recorded path through a directory which no longer exists, such as the directory an audit file was created in, has its `..` removed as text, as a missing directory cannot be a symbolic link, while a recorded path with `..` after a link which no longer exists, a file, or a directory which cannot be searched never matches another file. When two files on disk match the same record, such as when the path to audit overlaps a copy of the files, the one matching through the invoked from directory is used and the other is reported as new. +Relative paths in a `hashdeep` file are resolved against the directory recorded in its `## Invoked from:` line, so an audit file created with `hashit --format hashdeep .` can be audited from anywhere using either a relative or absolute path. When the invoked from directory does not contain the files, such as after they have been copied elsewhere, paths are also resolved against the directory containing the audit file, being where the file really is and then where the path supplied for it is when that is a symbolic link, and then the current directory. An audit is matched through only one of these, the one through which the most files match their records, or the first of those in this order, so a file found only through another, such as a file moved to where the directory containing the audit file places its record, is reported as moved or new. A file counts towards every directory through which a record names it, except a file an absolute record names, and is checked against the record naming it through the chosen directory, so a file which another record also names through a different directory, such as `meta/README` named by `./README` through a manifest kept in `meta`, is still checked against its own record. When as many files match through another directory as through the first of them, which the paths are relative to is not decided by the files on disk, so the audit is checked through each and passes only when it passes through every one. The first directory is reported when it fails, otherwise the one it fails through, with a warning naming it. Otherwise a tree copied with its manifest kept in `meta`, where `./README` and `./meta/README` both name `meta/README`, could pass after `README` was moved onto `meta/README`. Relative paths in `sum` files have no invoked from directory, so are resolved against the current directory first. Matching is case sensitive on every platform. Paths are always compared as absolute paths, so the path to audit may be supplied as relative or absolute, and `./file` and `file` are the same file. So a manifest stored at the root of a set of files, created from that directory, can be audited from anywhere after the files are copied or shipped elsewhere. Symbolic links in the path to audit are not resolved, so a manifest created through `real/` and audited through a link to it reports every file as moved, as `hashdeep` does. When the current directory was reached through a symbolic link, paths through where it, or a directory containing it, really is are compared through the link, so a manifest of absolute paths through `real/` audited with `.` from the link passes. A path containing `..`, such as `../dir` or `link/../dir`, names the directory the operating system reads, being the parent of the directory a symbolic link points to rather than the directory containing the link, in the paths supplied, the paths recorded in audit files and with `--absolute`. Where removing the `..` names the same directory, such as from a current directory reached through a symbolic link, the path is kept as written, A recorded path through a directory which no longer exists, such as the directory an audit file was created in, has its `..` removed as text, as a missing directory cannot be a symbolic link, while a recorded path with `..` after a link which no longer exists, a file, or a directory which cannot be searched never matches another file. When two files on disk match the same record, such as when the path to audit overlaps a copy of the files, the one matching through the invoked from directory is used and the other is reported as new. If the audit file is inside the directory being audited it is skipped, as it can never contain its own hash. `hashdeep` and `md5sum` do not do this, so when generating a file for them to verify, write it outside the directory being hashed, otherwise they will report the partially written file as changed. diff --git a/main_audit_test.go b/main_audit_test.go index e99a8e2..d6415bd 100644 --- a/main_audit_test.go +++ b/main_audit_test.go @@ -139,6 +139,11 @@ func newAuditScenario(seed uint64) *auditScenario { if r.Intn(8) == 0 { content = "content 0\n" // identical files } + if name == s.manifestName() && r.Intn(3) == 0 { + // an empty file of the manifest's name, as a manifest written by a shell redirect lists + // itself as an empty file + content = "" + } s.files[p] = content } @@ -799,6 +804,48 @@ var auditRegressions = []struct { files: map[string]string{"README": "top\n", "meta/README": "meta\n"}, changes: []string{"rename meta/README meta/README.old", "rename README meta/README"}}, }, + { + // through the manifest's directory the line naming the empty file names the manifest, so it + // was dropped as the manifest listing itself before the directory was chosen + name: "unchanged empty file of the manifest's name beside it fails, sum", + strict: true, + s: &auditScenario{format: "sum", location: "out", relocation: "none", auditFrom: "root", + files: map[string]string{"hashes.md5": "", "a": "a\n", "sub/b": "b\n"}}, + }, + { + name: "deleted empty file of the manifest's name beside it passes, sum", + s: &auditScenario{format: "sum", location: "out", relocation: "none", auditFrom: "root", + files: map[string]string{"hashes.md5": "", "a": "a\n", "sub/b": "b\n"}, + changes: []string{"delete hashes.md5"}}, + }, + { + // through the manifest's directory the line naming the empty file names the manifest, so it + // was dropped as the manifest listing itself before the directory was chosen + name: "unchanged empty file of the manifest's name beside it fails, hashdeep", + strict: true, + s: &auditScenario{format: "hashdeep", location: "out", relocation: "none", auditFrom: "root", + files: map[string]string{"m.hd": "", "a": "a\n", "sub/b": "b\n"}}, + }, + { + name: "deleted empty file of the manifest's name beside it passes, hashdeep", + s: &auditScenario{format: "hashdeep", location: "out", relocation: "none", auditFrom: "root", + files: map[string]string{"m.hd": "", "a": "a\n", "sub/b": "b\n"}, + changes: []string{"delete m.hd"}}, + }, + { + // through the manifest's directory the line naming the empty file names the manifest, so it + // was dropped as the manifest listing itself before the directory was chosen + name: "unchanged empty file of the manifest's name beside it fails, sqlite", + strict: true, + s: &auditScenario{format: "sqlite", location: "out", relocation: "none", auditFrom: "root", + files: map[string]string{"m.db": "", "a": "a\n", "sub/b": "b\n"}}, + }, + { + name: "deleted empty file of the manifest's name beside it passes, sqlite", + s: &auditScenario{format: "sqlite", location: "out", relocation: "none", auditFrom: "root", + files: map[string]string{"m.db": "", "a": "a\n", "sub/b": "b\n"}, + changes: []string{"delete m.db"}}, + }, } func TestAuditRegressions(t *testing.T) { diff --git a/processor/audit.go b/processor/audit.go index 783cdae..7737316 100644 --- a/processor/audit.go +++ b/processor/audit.go @@ -258,8 +258,9 @@ func sqliteAuditHashes(pathName string) ([]string, error) { // earlier in priority order, see Candidate. Records written as hashit writes them, such as // sub/a or ./sub/a, are looked up directly by sqliteLookup, so only other spellings such as // sub//a are indexed. It is held in a temporary table rather than memory so databases larger -// than memory can be audited -func buildSqlitePathIndex(ctx context.Context, db *sql.DB, bases []string) error { +// than memory can be audited. It reports if any record is a relative path +func buildSqlitePathIndex(ctx context.Context, db *sql.DB, bases []string) (bool, error) { + relative := false if _, err := db.ExecContext(ctx, `create temp table paths ( abs text not null, filepath text not null, @@ -267,25 +268,25 @@ func buildSqlitePathIndex(ctx context.Context, db *sql.DB, bases []string) error rid integer not null, primary key (abs, level) )`); err != nil { - return err + return false, err } tx, err := db.BeginTx(ctx, nil) if err != nil { - return err + return false, err } defer func() { _ = tx.Rollback() }() insert, err := tx.PrepareContext(ctx, `insert into temp.paths (abs, filepath, level, rid) values (?, ?, ?, ?) on conflict (abs, level) do nothing`) if err != nil { - return err + return false, err } defer insert.Close() rows, err := tx.QueryContext(ctx, "select rowid, filepath from file_hashes order by rowid") if err != nil { - return err + return false, err } defer rows.Close() @@ -293,8 +294,9 @@ on conflict (abs, level) do nothing`) var rid int64 var filePath string if err := rows.Scan(&rid, &filePath); err != nil { - return err + return false, err } + relative = relative || !filepath.IsAbs(filePath) direct := isDirectSpelling(filePath) if direct && len(workingDirAncestors()) == 0 { continue @@ -307,15 +309,15 @@ on conflict (abs, level) do nothing`) continue } if _, err := insert.ExecContext(ctx, k, filePath, level, rid); err != nil { - return err + return false, err } } } if err := rows.Err(); err != nil { - return err + return false, err } _ = rows.Close() - return tx.Commit() + return relative, tx.Commit() } // isDirectSpelling reports whether sqliteLookup finds a record by its path without the index, @@ -634,7 +636,8 @@ func doSqliteAudit(input chan Result) (string, bool) { invokedFrom, _ := queries.MetadataGet(ctx, metadataInvokedFrom) bases := auditBases(invokedFrom, AuditFile, cwd) - if err := buildSqlitePathIndex(ctx, db, bases); err != nil { + relative, err := buildSqlitePathIndex(ctx, db, bases) + if err != nil { printError(fmt.Sprintf("failed to index audit database: %s", err.Error())) return "", false } @@ -729,15 +732,17 @@ func doSqliteAudit(input chan Result) (string, bool) { } _ = insertMatch.Close() - chosen := chooseLevel(votes) - tied := tiedLevels(votes, chosen) + chosen, tied := judgedLevels(votes, chooseLevel(votes), bases, invokedFrom == "" && relative) if len(tied) == 0 { return judgeSqliteAudit(ctx, db, tx, stmts, bases, cwd, chosen, newFileCandidates, examinedCount) } - return judgeTied(AuditFile, bases, chosen, tied, func(first bool, level int) (string, bool) { - if first { - return judgeSqliteAudit(ctx, db, tx, stmts, bases, cwd, level, slices.Clone(newFileCandidates), examinedCount) - } + // each judgement has its own transaction, as only one connection is used + stmts.close() + if err := tx.Commit(); err != nil { + printError(fmt.Sprintf("failed to commit audit results: %s", err.Error())) + return "", false + } + return judgeTied(AuditFile, bases, chosen, tied, func(level int) (string, bool) { // the tables of the last judgement, as temp.matches holds every directory for _, table := range []string{"seen", "duplicates", "repeated"} { if _, err := db.ExecContext(ctx, "drop table if exists temp."+table); err != nil { diff --git a/processor/audit_parser.go b/processor/audit_parser.go index 991da2c..366c5f7 100644 --- a/processor/audit_parser.go +++ b/processor/audit_parser.go @@ -269,6 +269,10 @@ func (hdl *Auditor) LookupLevels(file string) []Candidate { // reparse returns the audit file parsed again with nothing yet matched, so an audit can be judged // through another directory, see judgeTied func (hdl *Auditor) reparse() (*Auditor, error) { + // warnings from parsing were given when it was first parsed + muted := warningsMuted + warningsMuted = true + defer func() { warningsMuted = muted }() fresh, err := NewAuditor(hdl.source, hdl.sourceName) if err != nil { return nil, err @@ -323,6 +327,40 @@ func chooseLevel(votes map[int]int) int { return chosen } +// judgedLevels returns the directory an audit is matched through and the directories it must also +// pass through, see judgeTied. These are those tied for the most files matched, see tiedLevels, or +// when no directory was chosen and a record is relative, every directory, as no file on disk +// decided which directory relative records are relative to, which the caller only asks for when +// there is no invoked from directory, as that decides it. Then a relative line naming a deleted +// file through one directory can name another file listed absolutely through another, which would +// hide the deletion +func judgedLevels(votes map[int]int, chosen int, bases []string, relative bool) (int, []int) { + if chosen != -1 { + return chosen, tiedLevels(votes, chosen) + } + if !relative || len(bases) == 0 { + return -1, nil + } + if len(bases) == 1 { + return 0, nil + } + var levels []int + for level := range bases { + levels = append(levels, level) + } + return 0, levels +} + +// hasRelative reports if any record is a relative path +func (hdl *Auditor) hasRelative() bool { + for _, r := range hdl.records { + if !filepath.IsAbs(r.Filename) { + return true + } + } + return false +} + // tiedLevels returns every directory as many files matched through as the one chosen, see // chooseLevel, when there is more than one, as then which the audit is matched through is not // decided by the files on disk @@ -720,9 +758,9 @@ func (hdl *Auditor) Exclude(path string) { for _, r := range hdl.records { switch { case hdl.isOwnEntry(r, target): - case hdl.primaryBase == "" && target.matches([]string{hdl.primaryKey(r)}): - // without the directory paths were created relative to, the working directory may not - // be the directory the audit is matched through, see settleOwnEntries + case hdl.mayBeOwnEntry(r, target): + // the directory the audit is matched through may not be one it names the audit file + // through, see settleOwnEntries hdl.ownEntries = append(hdl.ownEntries, r) records = append(records, r) default: @@ -756,24 +794,29 @@ func (hdl *Auditor) recordKeys(r *AuditRecord) []string { } // isOwnEntry reports if the record is the audit file listing itself, which happens when it is -// written by a shell redirect into the directory being hashed. The record must resolve to the -// audit file using the invoked from directory of a hashdeep file. A record which only resolves to -// it some other way, such as through the audit file's own directory after a tree has been copied, -// must also describe an empty file, as the audit file was when it was hashed. Otherwise it may be -// a different file which happens to share the audit file's name, and must still be audited. Without -// an invoked from directory a record resolving to it through the working directory is decided once -// the directory the audit is matched through is chosen, see settleOwnEntries +// written by a shell redirect into the directory being hashed, being a record resolving to the +// audit file using the invoked from directory of a hashdeep file func (hdl *Auditor) isOwnEntry(r *AuditRecord, target auditFileTarget) bool { - if hdl.primaryBase != "" && target.matches([]string{hdl.primaryKey(r)}) { + return hdl.primaryBase != "" && target.matches([]string{hdl.primaryKey(r)}) +} + +// mayBeOwnEntry reports if the record may be the audit file listing itself, which is decided once +// the directory the audit is matched through is chosen, see settleOwnEntries. Without an invoked +// from directory that is a record resolving to it through the working directory. A record which +// only resolves to it some other way, such as through the audit file's own directory after a tree +// has been copied, must also describe an empty file, as the audit file was when it was hashed. +// Otherwise it may be a different file which happens to share the audit file's name, such as an +// empty t/hashes.md5 listed in m/hashes.md5 written from t, and must still be audited +func (hdl *Auditor) mayBeOwnEntry(r *AuditRecord, target auditFileTarget) bool { + if hdl.primaryBase == "" && target.matches([]string{hdl.primaryKey(r)}) { return true } - return target.matches(hdl.recordKeys(r)) && isEmptyRecord(r) + return isEmptyRecord(r) && target.matches(hdl.recordKeys(r)) } -// settleOwnEntries drops each record which names the audit file through the working directory, when -// there is no invoked from directory, if it also names the audit file through the directory the -// audit is matched through and no file on disk was checked against it, as it is the audit file -// listing itself. Otherwise it names another file, such as data/data/hashes.md5 for a sum file +// settleOwnEntries drops each record which may be the audit file listing itself, see mayBeOwnEntry, +// if it names the audit file through the directory the audit is matched through and no file on +// disk was checked against it. Otherwise it names another file, such as data/data/hashes.md5 for a sum file // data/hashes.md5 written in data and audited from its parent, which is audited as any other func (hdl *Auditor) settleOwnEntries(chosen int, claimed func(*AuditRecord) bool) { base := hdl.bases[max(chosen, 0)] @@ -817,9 +860,11 @@ func (hdl *Auditor) removeDuplicates() { // compare the resolved path, so the same file listed by relative and absolute path is a // duplicate too key := hdl.primaryKey(r) - if hdl.sumFile { - // only lines naming the same path through every directory, as a relative line can be - // found through the sum file's directory while an absolute one cannot, see GroupSameFile + if hdl.sumFile || hdl.primaryBase == "" { + // without an invoked from directory, only records naming the same path through every + // directory, as a relative record can name another file through the audit file's + // directory than an absolute one, see GroupSameFile. Dropping the absolute record + // would let the relative one decide the directory alone key = strings.Join(hdl.recordKeys(r), "\x00") } if first, ok := seen[key]; ok { @@ -898,24 +943,18 @@ func (hdl *Auditor) warnDuplicates() { // such as ./a, a and its absolute path, through the directory the audit is matched through, or every // directory when none was chosen, see judgingBases. Lines naming the same file only through another // directory, such as ./README through the working directory sub and ./sub/README through the sum -// file's directory, name different files. When as many files matched through other directories as -// through the chosen one, see tiedLevels, lines are grouped through each of them, as either could be -// the directory the sum file is relative to, so lines naming a file differently through them cannot -// both be right. Each line is kept, as its own path may be the one which is scanned, and a line is -// only not reported on its own when another line was checked against the same file. Lines listing -// different values of a hash for the file can never match, which is warned of once grouped -func (hdl *Auditor) GroupSameFile(chosen int, tied []int) { +// file's directory, name different files. When the audit is judged through several directories, +// see judgeTied, each groups lines through its own, so lines naming a file differently through +// another are judged by that directory. Each line is kept, as its own path may be the one which is +// scanned, and a line is only not reported on its own when another line was checked against the +// same file. Lines listing different values of a hash for the file can never match, which is +// warned of once grouped +func (hdl *Auditor) GroupSameFile(chosen int, scanned map[string]bool) { if !hdl.sumFile { return } defer hdl.warnConflicts() bases := judgingBases(hdl.bases, chosen) - if len(tied) != 0 { - bases = nil - for _, level := range tied { - bases = append(bases, hdl.bases[level]) - } - } keysOf := func(r *AuditRecord) []string { var keys []string for _, base := range bases { @@ -934,11 +973,30 @@ func (hdl *Auditor) GroupSameFile(chosen int, tied []int) { } } + // a path which was not scanned may be a symbolic link to a file another line names, such as a + // through the working directory to s/a, so is the same file as it. Scanned paths are files + // themselves, so only the paths of lines which are missing or outside the paths audited, usually + // few, are looked up + links := map[string]bool{} + for _, r := range hdl.records { + for _, k := range keysOf(r) { + if counts[k] > 1 || scanned[k] { + continue + } + if fi, err := os.Lstat(k); err == nil && fi.Mode()&os.ModeSymlink != 0 { + if real := realPath(k); real != "" { + links[k] = true + links[real] = true + } + } + } + } + groups := map[string][]*AuditRecord{} for _, r := range hdl.records { keys := keysOf(r) for _, k := range keys { - if counts[k] > 1 { + if counts[k] > 1 || links[k] { // the first file it names, by where it really is, as that is the file it is checked // against for _, k := range keys { diff --git a/processor/audit_test.go b/processor/audit_test.go index bc0c629..b2663ad 100644 --- a/processor/audit_test.go +++ b/processor/audit_test.go @@ -3,6 +3,7 @@ package processor import ( + "fmt" "os" "path/filepath" "runtime" @@ -329,3 +330,45 @@ func TestHasCollateClause(t *testing.T) { } } } + +func TestJudgeTied(t *testing.T) { + bases := []string{"/b0", "/b1", "/b2"} + cases := []struct { + name string + chosen int + tied []int + fails map[int]bool + reported int + calls int + }{ + {name: "passing two way tie judges each once", chosen: 0, tied: []int{0, 1}, reported: 0, calls: 2}, + {name: "passing three way tie judges each once", chosen: 1, tied: []int{2, 0, 1}, reported: 1, calls: 3}, + {name: "chosen failing is reported", chosen: 0, tied: []int{0, 1}, fails: map[int]bool{0: true, 1: true}, reported: 0, calls: 3}, + {name: "chosen failing alone is reported", chosen: 0, tied: []int{0, 1}, fails: map[int]bool{0: true}, reported: 0, calls: 2}, + {name: "other failing is reported", chosen: 0, tied: []int{0, 1, 2}, fails: map[int]bool{2: true}, reported: 2, calls: 4}, + {name: "first other failing is reported", chosen: 2, tied: []int{0, 1, 2}, fails: map[int]bool{0: true, 1: true}, reported: 0, calls: 3}, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + var calls []int + var last int + summary, passed := judgeTied("m", bases, c.chosen, c.tied, func(level int) (string, bool) { + calls = append(calls, level) + last = level + return fmt.Sprintf("judged %d", level), !c.fails[level] + }) + if len(calls) != c.calls { + t.Errorf("expected %d judgements got %v", c.calls, calls) + } + if last != c.reported || summary != fmt.Sprintf("judged %d", c.reported) { + t.Errorf("expected %d reported got %d %q", c.reported, last, summary) + } + if passed == (len(c.fails) != 0) { + t.Errorf("expected passed %v got %v", len(c.fails) == 0, passed) + } + if warningsMuted || stdout.w != os.Stdout { + t.Errorf("output not restored") + } + }) + } +} diff --git a/processor/formatters.go b/processor/formatters.go index ff3f69a..82084c2 100644 --- a/processor/formatters.go +++ b/processor/formatters.go @@ -268,16 +268,18 @@ func doAudit(input chan Result) (string, bool) { votes[level]++ } } - chosen := chooseLevel(votes) - tied := tiedLevels(votes, chosen) + chosen, tied := judgedLevels(votes, chooseLevel(votes), auditor.bases, auditor.primaryBase == "" && auditor.hasRelative()) if len(tied) == 0 { - return judgeAudit(founds, newFileCandidates, len(results), chosen, nil) + return judgeAudit(founds, newFileCandidates, len(results), chosen) } original := auditor defer func() { auditor = original }() - return judgeTied(original.sourceName, original.bases, chosen, tied, func(first bool, level int) (string, bool) { - if first { - return judgeAudit(founds, newFileCandidates, len(results), level, tied) + // the first judgement uses the audit file as already parsed, and each after it parses it again + used := false + return judgeTied(original.sourceName, original.bases, chosen, tied, func(level int) (string, bool) { + if !used { + used = true + return judgeAudit(founds, newFileCandidates, len(results), level) } fresh, err := original.reparse() if err != nil { @@ -286,7 +288,7 @@ func doAudit(input chan Result) (string, bool) { } auditor = fresh founds, newFiles := lookupResults(results) - return judgeAudit(founds, newFiles, len(results), level, tied) + return judgeAudit(founds, newFiles, len(results), level) }) } @@ -294,48 +296,56 @@ func doAudit(input chan Result) (string, bool) { // chosen, see tiedLevels, as which the paths were written relative to is then not decided by the // files on disk. Judged only through the first, a file modified through one directory can match // another record through the first, with the record it should have matched reported as moved, so -// the audit passes only when it passes through every one, reporting the first it fails through. -// judge is called with each directory in turn, the chosen one first with first set, then again -// for the one reported with first unset, so it starts again from the audit file -func judgeTied(auditName string, bases []string, chosen int, tied []int, judge func(first bool, level int) (string, bool)) (string, bool) { - levels := []int{chosen} - sorted := slices.Clone(tied) - slices.Sort(sorted) - for _, level := range sorted { +// the audit passes only when it passes through every one. The chosen directory is reported when it +// fails, otherwise the first which does. The other directories are judged first with their report +// and warnings discarded, so no report is held in memory, and when every one passes the chosen +// directory is judged once, writing its report. Otherwise the one reported is judged again writing +// it, so its warnings are its own. judge must start from the audit file each time it is called +func judgeTied(auditName string, bases []string, chosen int, tied []int, judge func(level int) (string, bool)) (string, bool) { + var others []int + for _, level := range tied { if level != chosen { - levels = append(levels, level) + others = append(others, level) } } + slices.Sort(others) - // each is judged with its report discarded, so no report is held in memory, then the one - // reported is judged again writing it, from the start as judging changes the audit's state out := stdout defer func() { stdout, warningsMuted = out, false }() - reported := chosen - for i, level := range levels { - // warnings were given when judged through the chosen directory - warningsMuted = i != 0 - stdout = &checkedWriter{w: io.Discard} - summary, passed := judge(i == 0, level) + quietly := func(level int) (string, bool) { + stdout, warningsMuted = &checkedWriter{w: io.Discard}, true + defer func() { stdout, warningsMuted = out, false }() + return judge(level) + } + + failing := -1 + for _, level := range others { + summary, passed := quietly(level) if summary == "" { // an error, which was printed return "", false } if !passed { - reported = level + failing = level break } } - stdout, warningsMuted = out, false - if reported != chosen { - printWarn(fmt.Sprintf("the paths in %s name as many files through %s as through %s, so the audit is checked through each, and fails through %s", auditName, bases[chosen], bases[reported], bases[reported])) + reported := chosen + if failing != -1 { + summary, passed := quietly(chosen) + if summary == "" { + return "", false + } + if passed { + reported = failing + printWarn(fmt.Sprintf("the paths in %s name as many files through %s as through %s, so the audit is checked through each, and fails through %s", auditName, bases[chosen], bases[reported], bases[reported])) + } } - warningsMuted = true - return judge(false, reported) + return judge(reported) } // judgeAudit matches files on disk to records through the chosen directory and reports the result -func judgeAudit(founds []auditFound, newFileCandidates []Result, examinedCount int, chosen int, tied []int) (string, bool) { +func judgeAudit(founds []auditFound, newFileCandidates []Result, examinedCount int, chosen int) (string, bool) { matched := 0 filesModified := 0 moved := 0 @@ -353,7 +363,17 @@ func judgeAudit(founds []auditFound, newFileCandidates []Result, examinedCount i r *AuditRecord level int } - auditor.GroupSameFile(chosen, tied) + var scanned map[string]bool + if auditor.sumFile { + scanned = make(map[string]bool, len(founds)+len(newFileCandidates)) + for _, f := range founds { + scanned[absFrom(auditor.cwd, f.res.File)] = true + } + for _, res := range newFileCandidates { + scanned[absFrom(auditor.cwd, res.File)] = true + } + } + auditor.GroupSameFile(chosen, scanned) // Two files on disk can resolve to the same record, so which claims it does not depend on the // order files were processed in. A file matching through a directory earlier in priority order diff --git a/test-all.sh b/test-all.sh index d11ad26..21654ea 100755 --- a/test-all.sh +++ b/test-all.sh @@ -1062,7 +1062,7 @@ mkdir -p "$AUDITDIR/r12/sb/t" echo a > "$AUDITDIR/r12/sb/t/a" && echo c > "$AUDITDIR/r12/sb/t/c" (cd "$AUDITDIR/r12/sb/t" && { md5sum c | sed 's/ c$/ a/'; md5sum "$PWD/a"; } > s.md5) check "conflicting sum lines audited from outside its directory are not a move test" \ - "cd '$AUDITDIR/r12/sb' && ! '$HASHIT' --allow-moved -a t/s.md5 t > /dev/null 2>&1 && '$HASHIT' -a t/s.md5 t 2>&1 | grep -q 'different hashes'" + "cd '$AUDITDIR/r12/sb' && ! '$HASHIT' -a t/s.md5 t > /dev/null 2>&1 && ! '$HASHIT' --allow-moved -a t/s.md5 t > /dev/null 2>&1 && '$HASHIT' --allow-moved -a t/s.md5 t 2>&1 | grep -q 'different hashes'" mkdir -p "$AUDITDIR/r12/u" echo u > "$AUDITDIR/r12/u/u" (cd "$AUDITDIR/r12/u" && "$HASHIT" -f sqlite -o ../u.db . > /dev/null) @@ -1346,15 +1346,18 @@ printf A > "$AUDITDIR/r21/D/a" && printf A > "$AUDITDIR/r21/T/a" (cd "$AUDITDIR/r21/D" && "$HASHIT" -f sqlite -o ../M/m.db a "$AUDITDIR/r21/T/a" > /dev/null) rm "$AUDITDIR/r21/D/a" check "sqlite relative row is not a duplicate of an absolute row through the working directory test" "cd '$AUDITDIR/r21/T' && ! '$HASHIT' -a ../M/m.db . > /dev/null 2>&1" -# a sum file listing every file by relative and absolute path passes in place from another directory +# a sum file listing every file by relative and absolute path passes in place, and fails from +# another directory, as md5sum -c does, as every file matches an absolute line so the relative +# lines are judged through every directory mkdir -p "$AUDITDIR/r20/t/s" echo a > "$AUDITDIR/r20/t/a" && echo b > "$AUDITDIR/r20/t/s/b" (cd "$AUDITDIR/r20/t" && md5sum a s/b "$AUDITDIR/r20/t/a" "$AUDITDIR/r20/t/s/b" > s.md5) -check "sum file listing files by relative and absolute path passes from parent test" "cd '$AUDITDIR/r20' && '$HASHIT' -a t/s.md5 t | grep -q 'Audit passed'" +check "sum file listing files by relative and absolute path passes in place test" "cd '$AUDITDIR/r20/t' && '$HASHIT' -a s.md5 . | grep -q 'Audit passed' && md5sum --quiet -c s.md5" +check "sum file listing files by relative and absolute path fails from parent test" "cd '$AUDITDIR/r20' && ! '$HASHIT' -a t/s.md5 t > /dev/null 2>&1 && ! md5sum --quiet -c t/s.md5 > /dev/null 2>&1" cp "$AUDITDIR/r20/t/a" "$AUDITDIR/r20/t/extra" -check "sum file listing files by relative and absolute path fails with a copy allowing moves test" "cd '$AUDITDIR/r20' && ! '$HASHIT' --allow-moved -a t/s.md5 t > /dev/null 2>&1" +check "sum file listing files by relative and absolute path fails with a copy allowing moves test" "cd '$AUDITDIR/r20/t' && ! '$HASHIT' --allow-moved -a s.md5 . > /dev/null 2>&1" rm "$AUDITDIR/r20/t/extra" "$AUDITDIR/r20/t/s/b" -check "sum file listing files by relative and absolute path fails when one is deleted test" "cd '$AUDITDIR/r20' && ! '$HASHIT' -a t/s.md5 t > /dev/null 2>&1" +check "sum file listing files by relative and absolute path fails when one is deleted test" "cd '$AUDITDIR/r20/t' && ! '$HASHIT' -a s.md5 . > /dev/null 2>&1" # the directory most files match through is used, so the file which moved is the one reported mkdir -p "$AUDITDIR/r19/maj/t" for F in a b c d e; do echo $F > "$AUDITDIR/r19/maj/t/$F"; done diff --git a/test-audit.sh b/test-audit.sh index c16beeb..beaa8da 100755 --- a/test-audit.sh +++ b/test-audit.sh @@ -164,6 +164,50 @@ expect_rc "deleted file of the sum file's name fails test" 1 "'$HASHIT' -a hashe expect_rc "md5sum fails a deleted file of the sum file's name test" 1 "md5sum --quiet -c hashes.md5" expect_rc "deleted file of the sum file's name fails from the parent test" 1 "cd .. && '$HASHIT' -a data/hashes.md5 data" +# A manifest kept outside the tree it was written from, which holds an empty file of the +# manifest's name. Through the manifest's directory that line names the manifest itself, but the +# audit is matched through the tree, where it is another file which must still be audited +mkdir -p "$WORK/own/sum/t" "$WORK/own/sum/m" "$WORK/own/hd/t" "$WORK/own/hd/m" +cd "$WORK/own/sum/t" || exit 1 +: > hashes.md5; echo a > a; echo b > b +md5sum hashes.md5 a b > ../m/hashes.md5 +expect_rc "empty file of the sum file's name kept elsewhere passes test" 0 "'$HASHIT' -a ../m/hashes.md5 ." +expect_rc "md5sum passes an empty file of the sum file's name kept elsewhere test" 0 "md5sum --quiet -c ../m/hashes.md5" +echo changed > hashes.md5 +expect_rc "modified empty file of the sum file's name fails with --allow-new test" 1 "'$HASHIT' --allow-new -a ../m/hashes.md5 ." +rm hashes.md5 +expect_rc "deleted empty file of the sum file's name fails test" 1 "'$HASHIT' -a ../m/hashes.md5 ." +expect_rc "md5sum fails a deleted empty file of the sum file's name test" 1 "md5sum --quiet -c ../m/hashes.md5" +cd "$WORK/own/hd/t" || exit 1 +: > m.hd; echo a > a +"$HASHIT" -c md5 -f hashdeep -o ../m/m.hd . > /dev/null +expect_rc "empty file of the hashdeep file's name kept elsewhere passes test" 0 "'$HASHIT' -a ../m/m.hd ." +expect_rc "hashdeep passes an empty file of the hashdeep file's name kept elsewhere test" 0 "hashdeep -l -r -a -k ../m/m.hd ." +rm m.hd +expect_rc "deleted empty file of the hashdeep file's name fails test" 1 "'$HASHIT' -a ../m/m.hd ." +expect_rc "hashdeep fails a deleted empty file of the hashdeep file's name test" 1 "hashdeep -l -r -a -k ../m/m.hd ." + +# A manifest listing a relative a and an absolute s/a, identical files. With a deleted, audited +# from s, the only file matching is named absolutely so no directory is chosen, and a must not be +# taken as the same file as s/a through the working directory s +for fmt in sum hashdeep; do + mkdir -p "$WORK/nochosen-$fmt/t/s" + cd "$WORK/nochosen-$fmt/t" || exit 1 + echo same > a; echo same > s/a + if [ $fmt == sum ]; then + M=hashes.md5; { md5sum a; md5sum "$PWD/s/a"; } > $M + else + M=m.hd; H=$(md5sum < a | cut -c1-32) + printf '%%%%%%%% HASHDEEP-1.0\n%%%%%%%% size,md5,filename\n##\n5,%s,./a\n5,%s,%s\n' "$H" "$H" "$PWD/s/a" > $M + fi + expect_rc "$fmt relative and absolute identical files pass from a subdirectory test" 0 "cd s && '$HASHIT' -a ../$M .." + rm a + expect_rc "$fmt deleted file with an identical file listed absolutely fails from a subdirectory test" 1 "cd s && '$HASHIT' -a ../$M .." + expect_rc "$fmt deleted file with an identical file listed absolutely fails test" 1 "'$HASHIT' -a $M ." +done +expect_rc "md5sum fails a deleted file with an identical file listed absolutely test" 1 "cd '$WORK/nochosen-sum/t' && md5sum --quiet -c hashes.md5" +expect_rc "hashdeep fails a deleted file with an identical file listed absolutely test" 1 "cd '$WORK/nochosen-hashdeep/t' && hashdeep -l -r -a -k m.hd ." + # A sum file at the root of the files it lists, audited from a subdirectory, with a name repeated # in the subdirectory mkdir -p "$WORK/root-sum/R/sub"