From 110728fbb429a6bff83ca6520b75ff1c3992b7bb Mon Sep 17 00:00:00 2001 From: Vladimir Sitnikov Date: Tue, 11 Aug 2026 11:04:54 +0300 Subject: [PATCH] docs(cfcli): document the contracts the signatures do not carry cfcli carried 2 comment lines across 3764 lines of Go, so every fact not expressed by a type had to be re-derived from the code on each reading. This adds doc comments to the declarations whose contract is not evident from the signature, and inline comments where a line is surprising. The facts recovered here are the ones that cost the most to re-derive: nil-tolerance of appDependencies.getenv and .now, the same-filesystem constraint replaceFile inherits from os.Rename, the four exit codes behind runWithDependencies' bare int return, and the format coupling between encryptDictionaryForTest and decryptDictionary. Comments only: the packages parse to identical ASTs with comments dropped, and gofmt, go vet, and go test are unchanged. Co-Authored-By: Claude Opus 5 --- cfcli/app.go | 24 +++++++++++++ cfcli/app_pipeline_test.go | 26 ++++++++++++++ cfcli/app_test.go | 15 +++++++++ cfcli/atomic_replace_other.go | 3 ++ cfcli/atomic_replace_windows.go | 4 +++ cfcli/decrypt.go | 11 ++++++ cfcli/decrypt_test.go | 4 +++ cfcli/dictionary.go | 60 ++++++++++++++++++++++++++++++--- cfcli/dictionary_cache.go | 23 +++++++++++++ cfcli/dictionary_cache_test.go | 6 ++++ cfcli/exclude_command.go | 32 ++++++++++++++++++ cfcli/exclusions.go | 14 ++++++++ cfcli/git_files.go | 7 ++++ cfcli/git_files_test.go | 7 ++++ cfcli/options.go | 18 ++++++++++ cfcli/output.go | 7 ++++ cfcli/output_test.go | 2 ++ cfcli/paths.go | 16 +++++++++ cfcli/paths_test.go | 5 +++ cfcli/regexp_error_path_test.go | 3 ++ cfcli/scanner.go | 45 +++++++++++++++++++++++++ cfcli/scanner_test.go | 10 ++++++ 22 files changed, 337 insertions(+), 5 deletions(-) diff --git a/cfcli/app.go b/cfcli/app.go index e9002a7..b0defc7 100644 --- a/cfcli/app.go +++ b/cfcli/app.go @@ -11,12 +11,18 @@ import ( "time" ) +// appDependencies holds the process-level services runWithDependencies would otherwise reach for +// directly, so a test can supply a temporary home, a fixed clock, and a known password. A nil now +// falls back to [time.Now]. A nil getenv leaves the dictionary password empty, which decryption +// then rejects. type appDependencies struct { refresher cacheRefresher getenv func(string) string now func() time.Time } +// run performs one cfcli invocation with the live dictionary endpoint, the process environment, +// and the wall clock. It returns the exit code documented on [runWithDependencies]. func run(ctx context.Context, args []string, stdout, stderr io.Writer) int { return runWithDependencies(ctx, args, stdout, stderr, appDependencies{ refresher: cacheRefresher{ @@ -31,6 +37,18 @@ func run(ctx context.Context, args []string, stdout, stderr io.Writer) int { }) } +// runWithDependencies performs one cfcli invocation against the given dependencies and returns the +// process exit code: +// +// - 0: the scan found nothing, or the exclude subcommand succeeded +// - 1: any other failure — bad arguments, the dictionary, decryption, scanning, an exclusion +// update, or a write +// - 2: the scan reported at least one finding that no allow rule or exclusion covered +// - 3: a dictionary expression does not compile under Go's RE2 engine +// +// Scan output goes to stdout. The usage help, every fatal message, and the dictionary warnings go +// to stderr; the --print=details warning is the one warning written to stdout. An "exclude" first +// argument is dispatched to [runExcludeCommand] before any option is parsed. func runWithDependencies( ctx context.Context, args []string, @@ -173,6 +191,9 @@ func writeHelp(output *lineOutput) error { return nil } +// writeFatal writes a message for a failure the caller exits on. It discards the write error: +// every caller already returns a nonzero exit code, and the only stream left to report a failed +// write on is the one that just failed. func writeFatal(output *lineOutput, format string, args ...any) { _ = output.text(format, args...) } @@ -185,6 +206,9 @@ func dictionaryStatusMessage(state cacheState) string { }[state] } +// dictionaryDisplayPath renders path for a human reader: a path inside home becomes "~/" followed +// by the remainder, and anything else is cleaned and left as it is. The result always uses "/" +// separators, including on Windows. func dictionaryDisplayPath(path, home string) string { relative, err := filepath.Rel(home, path) outsideHome := relative == ".." || strings.HasPrefix(relative, ".."+string(filepath.Separator)) diff --git a/cfcli/app_pipeline_test.go b/cfcli/app_pipeline_test.go index d54fda0..794e66a 100644 --- a/cfcli/app_pipeline_test.go +++ b/cfcli/app_pipeline_test.go @@ -27,6 +27,9 @@ func TestRunWithDependenciesRequiresPassword(t *testing.T) { } } +// A dictionary pattern that Go's RE2 engine rejects ends the run with exit code 3 rather than the +// generic 1, so a caller can tell a broken dictionary from a broken environment. The case here is +// a lookbehind, which RE2 has no equivalent for. func TestRunWithDependenciesMapsInvalidRegexpToExitCodeThree(t *testing.T) { root := initRepository(t) dependencies := testAppDependencies(t, "BAD(regexp)=(?<=token)value\n", "test-password") @@ -40,6 +43,9 @@ func TestRunWithDependenciesMapsInvalidRegexpToExitCodeThree(t *testing.T) { } } +// Quick mode stops at the first finding: it emits that one found event, exits 2, and prints the +// hint that names the flag combination for exclusion commands rather than the commands themselves. +// It still prints the scan summary the other modes print. func TestRunWithDependenciesQuickReturnsTwoOnFirstFinding(t *testing.T) { root := initRepository(t) writeTestFile(t, root, "secret.txt", "contains SECRET") @@ -70,6 +76,8 @@ func TestRunWithDependenciesQuickReturnsTwoOnFirstFinding(t *testing.T) { } } +// Quick mode with --print=details prints a ready-to-run exclude command for each supported shell +// in place of the hint, never both. func TestRunWithDependenciesQuickPrintDetailsPrintsExclusionCommands(t *testing.T) { root := initRepository(t) writeTestFile(t, root, "secret.txt", "contains SECRET") @@ -94,6 +102,9 @@ func TestRunWithDependenciesQuickPrintDetailsPrintsExclusionCommands(t *testing. } } +// The default JSON mode reports a finding and keeps going, so the scan summary and the dictionary +// status lines are all present alongside the found event, and exit code 2 arrives at the end +// rather than in place of the rest of the output. func TestRunWithDependenciesJSONCompletesAndReturnsTwo(t *testing.T) { root := initRepository(t) writeTestFile(t, root, "secret.txt", "SECRET") @@ -114,6 +125,8 @@ func TestRunWithDependenciesJSONCompletesAndReturnsTwo(t *testing.T) { assertCurrentDictionaryOutput(t, stdout.String()) } +// Outside quick mode --print=details is ignored with a warning on stdout, and no exclude command +// is printed. The scan itself runs to completion and still reports the finding through exit 2. func TestRunWithDependenciesPrintDetailsWarnsOutsideQuickMode(t *testing.T) { root := initRepository(t) writeTestFile(t, root, "secret.txt", "SECRET") @@ -147,6 +160,8 @@ func assertCurrentDictionaryOutput(t *testing.T, output string) { } } +// FOLDER_PATH may be given relative to the working directory. The test changes the process working +// directory to reach that case, so it cannot run in parallel with anything else in the package. func TestRunWithDependenciesAcceptsRelativeFolderPath(t *testing.T) { root := initRepository(t) writeTestFile(t, root, "safe.txt", "safe") @@ -182,6 +197,9 @@ func TestRunWithDependenciesAcceptsRelativeFolderPath(t *testing.T) { } } +// A match covered by a grand-report exclusion is reported as an excluded event instead of a found +// one, so the run still exits 0. The report file itself stays eligible for scanning, which is why +// the count is two for one written file. func TestRunWithDependenciesAppliesGrandReportExclusions(t *testing.T) { root := initRepository(t) writeTestFile(t, root, "secret.txt", "SECRET") @@ -224,6 +242,11 @@ func TestRunWithDependenciesEnablesVerboseListOutput(t *testing.T) { } } +// testAppDependencies builds dependencies for an offline run. The dictionary cache it writes under +// a temporary home is fresh, so the refresher serves it without touching the nil HTTP client and +// the run reports the dictionary as up to date. The clock returns a fixed instant, then one +// 1234 ms later for every call after the first, which is the "1.234 seconds" the expected +// transcripts contain. func testAppDependencies(t *testing.T, plaintext, password string) appDependencies { t.Helper() home := t.TempDir() @@ -253,6 +276,9 @@ func testAppDependencies(t *testing.T, plaintext, password string) appDependenci } } +// encryptDictionaryForTest produces what decryptDictionary expects: PKCS#7-padded AES-CBC under a +// key derived from password with the same salt, iteration count, and IV, then Base64. Change it in +// step with decryptDictionary, or every test that loads a dictionary fails while decrypting. func encryptDictionaryForTest(t *testing.T, plaintext []byte, password string) string { t.Helper() key := deriveKey([]byte(password), []byte("bsd87918hediu"), 65536, 32) diff --git a/cfcli/app_test.go b/cfcli/app_test.go index 6d138f6..e6ec1fe 100644 --- a/cfcli/app_test.go +++ b/cfcli/app_test.go @@ -9,6 +9,9 @@ import ( "testing" ) +// A scan invocation with no positional argument, or with more than two, exits 1 with the usage +// text on stderr and nothing on stdout. The zero-value dependencies are part of the rule: the +// argument count has to be rejected before anything calls the nil homeDir behind the refresher. func TestRunRejectsInvalidArgumentCounts(t *testing.T) { tests := [][]string{{}, {"root", "list", "extra"}} for _, args := range tests { @@ -32,6 +35,9 @@ func TestRunRejectsInvalidArgumentCounts(t *testing.T) { } } +// A wrong argument count for either command form prints the whole usage block, scan form and +// exclude form together, so a user who mistyped one still sees the other. Add an entry to the +// expected list here when writeHelp gains a line. func TestRunPrintsExpandedHelpForInsufficientArguments(t *testing.T) { tests := [][]string{ nil, @@ -62,6 +68,9 @@ func TestRunPrintsExpandedHelpForInsufficientArguments(t *testing.T) { } } +// A clean scan exits 0 and prints the dictionary status, path, and version, then the progress +// line, the file count, and the elapsed time, with nothing on stderr. The check compares the whole +// stdout transcript, so a line added anywhere in the run has to be added to want as well. func TestRunReportsTotalFilesScanned(t *testing.T) { root := initRepository(t) writeTestFile(t, root, "a.txt", "a") @@ -93,6 +102,10 @@ func TestRunReportsTotalFilesScanned(t *testing.T) { } } +// Every invocation whose file set cannot be established exits 1: a missing directory, a file in +// place of a directory, a directory outside any Git repository, and a file list that cannot be +// read. The stdout transcript stops after the progress line, and stderr names the operation that +// failed. func TestRunReportsRuntimeErrors(t *testing.T) { nonGitDirectory := t.TempDir() rootFile := filepath.Join(t.TempDir(), "root.txt") @@ -167,6 +180,8 @@ func TestRunReportsUnavailableGit(t *testing.T) { } } +// Every cacheState maps to its own status line. Add a case here when a state joins the cacheState +// constants, or the run prints a bare "TEXT: " line for it. func TestDictionaryStatusMessage(t *testing.T) { tests := []struct { state cacheState diff --git a/cfcli/atomic_replace_other.go b/cfcli/atomic_replace_other.go index 4d3ad98..37d8899 100644 --- a/cfcli/atomic_replace_other.go +++ b/cfcli/atomic_replace_other.go @@ -4,6 +4,9 @@ package main import "os" +// replaceFile renames source onto destination, replacing an existing destination in one step. +// Both paths must live on the same filesystem, because [os.Rename] has no copy fallback. Windows +// builds get the MoveFileExW implementation instead. func replaceFile(source, destination string) error { return os.Rename(source, destination) } diff --git a/cfcli/atomic_replace_windows.go b/cfcli/atomic_replace_windows.go index 6ffb599..02d78b8 100644 --- a/cfcli/atomic_replace_windows.go +++ b/cfcli/atomic_replace_windows.go @@ -8,6 +8,7 @@ import ( "unsafe" ) +// Values of the Win32 MOVEFILE_* flags that MoveFileExW takes in dwFlags. const ( moveFileReplaceExisting = 0x1 moveFileWriteThrough = 0x8 @@ -15,6 +16,9 @@ const ( var moveFileEx = syscall.NewLazyDLL("kernel32.dll").NewProc("MoveFileExW") +// replaceFile moves source onto destination with MoveFileExW, replacing an existing destination +// and returning once the change has reached the disk rather than the cache. Both paths must live +// on the same volume, because the call omits MOVEFILE_COPY_ALLOWED. func replaceFile(source, destination string) error { sourcePointer, err := syscall.UTF16PtrFromString(source) if err != nil { diff --git a/cfcli/decrypt.go b/cfcli/decrypt.go index 3a39bfc..6f31aca 100644 --- a/cfcli/decrypt.go +++ b/cfcli/decrypt.go @@ -13,8 +13,15 @@ import ( "unicode/utf8" ) +// dictionaryIV is the fixed AES-CBC initialization vector the published dictionary is encrypted +// with. It, the salt and iteration count in [decryptDictionary], and the padding scheme all mirror +// the constants the Java class PasswordBasedEncryption decrypts the same dictionaries with; a change +// on either side makes every already published dictionary undecryptable. var dictionaryIV = []byte{0, 2, 3, 4, 5, 4, 3, 2, 1, 0, 1, 2, 3, 4, 5, 0} +// decryptDictionary decodes ciphertext as Base64 text, ignoring surrounding whitespace, and +// decrypts it under a key derived from password. The plaintext has to be valid UTF-8; anything else +// is an error, as is an empty password. The returned slice is a fresh copy. func decryptDictionary(ciphertext []byte, password string) ([]byte, error) { if password == "" { return nil, fmt.Errorf("Dictionary password environment variable CYBER_FERRET_PASSWORD is not set") @@ -48,6 +55,7 @@ func deriveKey(password, salt []byte, iterations, keyLength int) []byte { return pbkdf2(password, salt, iterations, keyLength, sha256.New) } +// pbkdf2 derives keyLength bytes, not bits, from password and salt using newHash as the HMAC hash. func pbkdf2(password, salt []byte, iterations, keyLength int, newHash func() hash.Hash) []byte { hashLength := newHash().Size() blocks := (keyLength + hashLength - 1) / hashLength @@ -74,6 +82,9 @@ func pbkdf2(password, salt []byte, iterations, keyLength int, newHash func() has return derived[:keyLength] } +// removePKCS7Padding strips the trailing padding and returns a prefix of plaintext that shares its +// backing array. It reports an error when the final byte is not a padding length within blockSize, +// or when the bytes it covers do not all repeat it. func removePKCS7Padding(plaintext []byte, blockSize int) ([]byte, error) { if len(plaintext) == 0 { return nil, fmt.Errorf("invalid PKCS padding: plaintext is empty") diff --git a/cfcli/decrypt_test.go b/cfcli/decrypt_test.go index 7dcd51b..83bc045 100644 --- a/cfcli/decrypt_test.go +++ b/cfcli/decrypt_test.go @@ -6,6 +6,10 @@ import ( "testing" ) +// The fixed ciphertext decrypts under the same parameters the Java class PasswordBasedEncryption +// decrypts published dictionaries with. A failure here means a crypto parameter drifted from that +// class, so published dictionaries no longer decrypt; compare the IV, salt, iteration count, and key +// length against the Java constants rather than adjusting the vector. func TestDecryptDictionaryMatchesJavaVector(t *testing.T) { const ciphertext = "nzn8A/OYyg6yMGSNTqYRzsRQ4G/x0BkR2x9W7Wqq8Kw=" diff --git a/cfcli/dictionary.go b/cfcli/dictionary.go index 28b1602..73b334e 100644 --- a/cfcli/dictionary.go +++ b/cfcli/dictionary.go @@ -10,19 +10,36 @@ import ( "unicode/utf16" ) +// signature is one dictionary key together with the pattern a scan matches file contents against. type signature struct { - key string - expression *regexp.Regexp + // key is the dictionary key, reported as the key of every finding this signature produces. + key string + // expression carries the (?is) flags, so it matches case-insensitively and its dot spans + // newlines. + expression *regexp.Regexp + // excludedExtensions holds lowercase extensions without the leading dot. A file whose + // extension is present is never matched against expression. excludedExtensions map[string]struct{} } +// dictionary is the compiled form of the decrypted dictionary file, as built by [loadDictionary]. type dictionary struct { - version string - signatures []signature - allowed map[string]struct{} + // version is the value of the VERSION key, empty when the file carries none. + version string + // signatures keep the order of the dictionary file, which is the order a scan applies them + // in. + signatures []signature + // allowed holds exact values that suppress a finding, lowercased, so a lookup has to + // lowercase too. + allowed map[string]struct{} + // allowedPatterns holds the allowed values that carried a wildcard, compiled by + // [compileAllowedPattern]. allowedPatterns []*regexp.Regexp } +// regexpCompileError reports a dictionary expression that Go's RE2 engine rejects, such as one +// using a lookbehind. It is a distinct type so that a caller can tell a broken dictionary from +// other load failures with errors.As; cfcli exits with code 3 for it. type regexpCompileError struct { key string expression string @@ -37,6 +54,9 @@ func (e *regexpCompileError) Unwrap() error { return e.cause } +// dictionaryEntry is one key/value line of the dictionary file, decoded but not yet interpreted. +// live is false once a later line repeats the key, so only a key's last occurrence reaches the +// [dictionary]. type dictionaryEntry struct { key string value string @@ -48,6 +68,17 @@ type signatureSpec struct { expression string } +// loadDictionary compiles the decrypted dictionary text into a [dictionary]. A parenthesized suffix +// on the key decides what its value means: +// +// - (regexp) holds a regular expression to match file contents against; +// - (allowed) holds a value that suppresses a finding, exact or, with a '*', a wildcard; +// - (exclude-ext) holds a comma-separated list of extensions the key before the suffix skips. +// +// A key without a suffix is a literal phrase, VERSION sets the version, and any other key holding a +// parenthesis is an error. Duplicate keys are reported through output and the last value wins. +// +// loadDictionary returns a *regexpCompileError when a signature does not compile. func loadDictionary(plaintext []byte, output *lineOutput) (dictionary, error) { entries, err := parseDictionaryEntries(plaintext, output) if err != nil { @@ -114,6 +145,9 @@ func loadDictionary(plaintext []byte, output *lineOutput) (dictionary, error) { return result, nil } +// compileAllowedPattern compiles an allowed value that carries a wildcard. Each '*' matches one or +// more non-whitespace characters, every other character is literal, and the pattern has to match a +// detected value whole, ignoring case. func compileAllowedPattern(value string) (*regexp.Regexp, error) { parts := strings.Split(value, "*") for index := range parts { @@ -122,6 +156,8 @@ func compileAllowedPattern(value string) (*regexp.Regexp, error) { return regexp.Compile(`(?i)^` + strings.Join(parts, `\S+`) + `\z`) } +// isAllowed reports whether exact, the text a signature matched, is one the dictionary declares +// harmless: an exact allowed value ignoring case, or a match of one of the wildcard patterns. func (d dictionary) isAllowed(exact string) bool { if _, allowed := d.allowed[strings.ToLower(exact)]; allowed { return true @@ -134,6 +170,12 @@ func (d dictionary) isAllowed(exact string) bool { return false } +// parseDictionaryEntries reads plaintext as key=value lines in the Java properties escaping +// convention: it skips blank lines and lines whose first non-blank character is '#', splits every +// other line at its first '=', and unescapes both halves with [unescapeProperty]. A line without +// '=' and a line with an empty key are both errors. The key is trimmed and the value is not, so a +// value keeps the spaces around it. The entries keep file order, and a repeated key clears live on +// the earlier entry and writes a warning through output. func parseDictionaryEntries(plaintext []byte, output *lineOutput) ([]dictionaryEntry, error) { entries := make([]dictionaryEntry, 0) positions := make(map[string]int) @@ -177,6 +219,9 @@ func parseDictionaryEntries(plaintext []byte, output *lineOutput) ([]dictionaryE return entries, nil } +// unescapeProperty decodes the escapes a Java properties file may carry: \t, \n, \r, \f, and \uXXXX +// including a surrogate pair. A backslash before any other character yields that character, and a +// trailing backslash stays a backslash. func unescapeProperty(value string) (string, error) { var result strings.Builder for index := 0; index < len(value); index++ { @@ -220,6 +265,8 @@ func unescapeProperty(value string) (string, error) { return result.String(), nil } +// parseUnicodePropertyEscape decodes the four hex digits after the 'u' at markerIndex. It returns +// the rune, which may be an unpaired surrogate, and the index of the last digit it consumed. func parseUnicodePropertyEscape(value string, markerIndex int) (rune, int, error) { if markerIndex+4 >= len(value) { return 0, markerIndex, fmt.Errorf("incomplete Unicode escape") @@ -232,6 +279,9 @@ func parseUnicodePropertyEscape(value string, markerIndex int) (rune, int, error return rune(parsed), markerIndex + 4, nil } +// literalExpression turns a literal dictionary value into a regular expression: punctuation is +// quoted so it keeps no regexp meaning, each single space becomes \s+ so any whitespace matches, +// and word boundaries at both ends keep the phrase from matching inside a longer word. func literalExpression(value string) string { parts := strings.Split(value, " ") for index := range parts { diff --git a/cfcli/dictionary_cache.go b/cfcli/dictionary_cache.go index b038214..4632155 100644 --- a/cfcli/dictionary_cache.go +++ b/cfcli/dictionary_cache.go @@ -10,6 +10,10 @@ import ( "time" ) +// Fixed parameters of the dictionary cache. cacheFileName is user-facing: the README documents the +// cache as ~/.qubership/sensitive-signatures.encrypted and cfcli prints that path, so renaming it +// leaves the previous cache behind unread. maxCacheSize is a byte count, and a download above it is +// discarded rather than truncated. const ( dictionaryURL = "https://raw.githubusercontent.com/exadmin/CyberFerretDictionary/main/dictionary-latest.encrypted" cacheFileName = "sensitive-signatures.encrypted" @@ -18,6 +22,9 @@ const ( maxCacheSize = 16 * 1024 * 1024 ) +// cacheRefresher keeps the encrypted dictionary in the user's home directory up to date. client, +// now, and homeDir have to be set; an empty url falls back to [dictionaryURL] and a non-positive +// timeout to [refreshTimeout]. type cacheRefresher struct { client *http.Client now func() time.Time @@ -26,6 +33,9 @@ type cacheRefresher struct { timeout time.Duration } +// cacheState records how a [cacheRefresher.refresh] call ended: the cache was young enough to use +// as it was, the download replaced it, or the download failed and the cache on disk serves on past +// its age limit. type cacheState int const ( @@ -34,12 +44,21 @@ const ( cacheFallback ) +// cacheResult locates the cache a scan should read. home is the resolved home directory, carried +// alongside path so a caller can abbreviate it to a leading tilde when it prints the path. type cacheResult struct { path string home string state cacheState } +// refresh returns the cache a scan should read, downloading a new copy first when the file is +// missing or older than [cacheMaxAge]. A cache within that age costs no request at all. +// +// A failed or timed-out download is not fatal while a cache exists: refresh writes the reason +// through output and returns the existing file with state [cacheFallback], however stale. It fails +// when the home directory cannot be resolved, when the cache path cannot be inspected, or when the +// download failed with no cache to fall back on. func (r cacheRefresher) refresh(ctx context.Context, output *lineOutput) (cacheResult, error) { home, err := r.homeDir() if err != nil { @@ -84,6 +103,10 @@ func (r cacheRefresher) refresh(ctx context.Context, output *lineOutput) (cacheR return cacheResult{}, fmt.Errorf("dictionary cache is unavailable after refresh: %w", refreshErr) } +// download fetches the dictionary into destination, creating its directory when needed. The body +// goes to a temporary file in that same directory, readable only by its owner, and replaces +// destination in one rename once it has arrived whole, so a cancelled or failed download leaves the +// previous cache untouched and nothing behind. A body over [maxCacheSize] bytes is rejected. func (r cacheRefresher) download(ctx context.Context, destination string) error { url := r.url if url == "" { diff --git a/cfcli/dictionary_cache_test.go b/cfcli/dictionary_cache_test.go index 47a0a42..38911da 100644 --- a/cfcli/dictionary_cache_test.go +++ b/cfcli/dictionary_cache_test.go @@ -13,6 +13,8 @@ import ( "time" ) +// The cache file keeps the name the README documents and cfcli prints. Renaming it is a +// user-visible change rather than a refactor, so update the documentation and this test together. func TestDictionaryCacheFilename(t *testing.T) { if cacheFileName != "sensitive-signatures.encrypted" { t.Fatalf("cacheFileName = %q, want sensitive-signatures.encrypted", cacheFileName) @@ -131,6 +133,10 @@ func TestCacheRefresherFailsWithoutCache(t *testing.T) { } } +// A download that outlives the refresh timeout never touches the cache afterwards, and leaves no +// temporary file behind. The server here blocks until the request context is cancelled, so a +// failure means the download goroutine still wrote to the cache directory once refresh had already +// returned the stale file. func TestCacheRefresherTimeoutDoesNotReplaceStaleCacheLater(t *testing.T) { home := t.TempDir() cache := writeCacheFile(t, home, "stale") diff --git a/cfcli/exclude_command.go b/cfcli/exclude_command.go index aafed09..efca687 100644 --- a/cfcli/exclude_command.go +++ b/cfcli/exclude_command.go @@ -11,12 +11,18 @@ import ( "strings" ) +// excludeEvent is the part of a [finding] the exclude command acts on. The remaining finding fields +// are ignored, so an event copied verbatim out of the scanner's JSON output is a valid argument. type excludeEvent struct { Type string `json:"type"` Found string `json:"found"` File string `json:"file"` } +// strictGrandReport mirrors [grandReport] for decoding only. Its Exclusions pointer, and the +// pointer fields of [strictGrandReportExclusion], tell an absent key from a present one, which is +// what lets [decodeGrandReport] reject a report missing "exclusions", "t-hash", or "f-hash" +// instead of substituting a zero value. type strictGrandReport struct { Exclusions *[]strictGrandReportExclusion `json:"exclusions"` } @@ -26,6 +32,9 @@ type strictGrandReportExclusion struct { FileHash *string `json:"f-hash"` } +// UnmarshalJSON decodes one exclusion, requiring the object to hold "t-hash" followed by "f-hash" +// and nothing else. Field order carries no meaning in JSON otherwise, so a report that spells the +// two keys in the other order is rejected rather than accepted. func (e *strictGrandReportExclusion) UnmarshalJSON(content []byte) error { decoder := json.NewDecoder(bytes.NewReader(content)) opening, err := decoder.Token() @@ -81,6 +90,8 @@ func (e *unsupportedExcludeEventTypeError) Error() string { ) } +// excludeCommandError reports a failure that left every file as it was. Error appends that promise +// to message, so message must not repeat it and must end with its own punctuation. type excludeCommandError struct { message string } @@ -120,6 +131,9 @@ func quotePowerShellArgument(value string) string { return "'" + strings.ReplaceAll(value, "'", "''") + "'" } +// quoteCmdArgument quotes value for a cmd.exe command line under the CommandLineToArgvW rules: a +// run of backslashes is doubled only when a quote or the end of the argument follows it, and a +// quote is escaped by that doubled run plus one more backslash. func quoteCmdArgument(value string) string { var quoted strings.Builder quoted.WriteByte('"') @@ -144,6 +158,9 @@ func quoteCmdArgument(value string) string { return quoted.String() } +// formatExcludeCommands renders the cfcli exclude invocation that would suppress event, once for +// each supported shell: POSIX, PowerShell, then cmd.exe, always in that order and whatever +// operating system cfcli runs on. func formatExcludeCommands(root string, event finding) ([]shellCommand, error) { absoluteRoot, err := filepath.Abs(root) if err != nil { @@ -178,6 +195,11 @@ func (e *excludeCommandError) Error() string { return e.message + " No files were changed." } +// updateExclusions adds the event encoded in encodedEvent to root/.qubership/grand-report.json, +// creating the directory and the file when they are absent, and returns the absolute path of the +// report it wrote. The new content goes to a temporary file in the same directory and is renamed +// over the old one, and every rejection happens before that rename, so a failure leaves an existing +// report byte-for-byte unchanged. func updateExclusions(root, encodedEvent string) (string, error) { event, err := parseExcludeEvent(encodedEvent) if err != nil { @@ -260,6 +282,10 @@ func updateExclusions(root, encodedEvent string) (string, error) { return reportPath, nil } +// decodeGrandReport parses a report strictly: an empty object decodes as an empty report, and +// anything else must be an "exclusions" array whose objects carry "t-hash" and "f-hash" and nothing +// more. Unknown fields, duplicate keys, and trailing content are errors rather than omissions, +// because [updateExclusions] rewrites the whole file and whatever this decoder drops would be lost. func decodeGrandReport(content []byte) (grandReport, error) { var compact bytes.Buffer if json.Compact(&compact, content) == nil && compact.String() == "{}" { @@ -301,6 +327,9 @@ func decodeGrandReport(content []byte) (grandReport, error) { return report, nil } +// validateUniqueJSONKeys rejects content that repeats a field name in any of its objects. +// encoding/json keeps the last of a repeated key without complaining, which would silently drop the +// earlier value on the next rewrite. func validateUniqueJSONKeys(content []byte) error { decoder := json.NewDecoder(bytes.NewReader(content)) decoder.UseNumber() @@ -359,6 +388,9 @@ func validateJSONValue(decoder *json.Decoder) error { return err } +// parseExcludeEvent decodes encoded into an [excludeEvent], tolerating the "JSON:" line prefix that +// cfcli itself prints so a finding can be pasted straight from the scan output. Only a "found" +// event with a nonempty "found" and "file" is accepted. func parseExcludeEvent(encoded string) (excludeEvent, error) { encoded = strings.TrimSpace(encoded) if strings.HasPrefix(encoded, "JSON:") { diff --git a/cfcli/exclusions.go b/cfcli/exclusions.go index ad0b1fc..40a0328 100644 --- a/cfcli/exclusions.go +++ b/cfcli/exclusions.go @@ -8,8 +8,14 @@ import ( "path/filepath" ) +// fullPathExclusionHash stands where a text hash would stand in grand-report.json and excludes the +// paired path outright: the file itself, or every file beneath it when the path is a directory. It +// is a literal marker rather than the SHA-256 of anything. const fullPathExclusionHash = "00000000" +// exclusionSet holds the exclusions loaded from grand-report.json. The outer key is the SHA-256 hex +// of a path normalized by [normalizeRelativePath], the inner key the SHA-256 hex of an exact match +// or [fullPathExclusionHash]; hashing the match is what makes text exclusions case-sensitive. type exclusionSet struct { textHashesByFileHash map[string]map[string]struct{} } @@ -23,6 +29,9 @@ type grandReportExclusion struct { FileHash string `json:"f-hash"` } +// loadExclusions reads the exclusions from root/.qubership/grand-report.json. A missing file yields +// an empty set silently; an unreadable or malformed one yields an empty set after a warning naming +// the absolute report path, so a broken report never stops a scan. func loadExclusions(root string, warnings *lineOutput) exclusionSet { loaded := exclusionSet{textHashesByFileHash: make(map[string]map[string]struct{})} reportPath, err := filepath.Abs(filepath.Join(root, ".qubership", "grand-report.json")) @@ -58,6 +67,9 @@ func (e exclusionSet) excludesPath(relativePath string) bool { return len(e.excludedPaths(relativePath)) > 0 } +// excludedPaths returns the whole-path exclusions that cover relativePath: the path itself and each +// of its ancestors up to the repository root, outermost first. The result is empty when no +// exclusion covers the path. func (e exclusionSet) excludedPaths(relativePath string) []string { candidates := []string{normalizeRelativePath(relativePath)} for candidates[len(candidates)-1] != "" { @@ -87,6 +99,8 @@ func (e exclusionSet) contains(textHash, fileHash string) bool { return found } +// normalizeRelativePath returns path in the cleaned, slash-separated form the exclusion hashes are +// computed over. The repository root becomes the empty string. func normalizeRelativePath(path string) string { cleaned := filepath.ToSlash(filepath.Clean(filepath.FromSlash(path))) if cleaned == "." { diff --git a/cfcli/git_files.go b/cfcli/git_files.go index ede5d5a..93adacb 100644 --- a/cfcli/git_files.go +++ b/cfcli/git_files.go @@ -10,6 +10,13 @@ import ( "strings" ) +// enumerateGitFiles returns every file git accounts for under root: all tracked +// files, plus the untracked files that survive git's standard exclude rules. A +// tracked file stays in the set even when an ignore rule matches it. +// +// Keys are paths relative to root, using the separator of the running OS. A +// regular ~/.gitignore_global also excludes untracked files, whether or not +// git's own configuration references it. func enumerateGitFiles(ctx context.Context, root string) (map[string]struct{}, error) { selected := make(map[string]struct{}) untrackedArgs := []string{"ls-files", "--others", "--exclude-standard", "-z"} diff --git a/cfcli/git_files_test.go b/cfcli/git_files_test.go index 53496bd..60c418e 100644 --- a/cfcli/git_files_test.go +++ b/cfcli/git_files_test.go @@ -28,6 +28,9 @@ func TestEnumerateGitFilesIncludesTrackedAndNonIgnoredUntracked(t *testing.T) { } } +// An untracked file stays out of the result when any exclude source git +// consults as standard hides it: a .gitignore, .git/info/exclude, or the file +// core.excludesFile names. func TestEnumerateGitFilesHonorsStandardExclusions(t *testing.T) { root := initRepository(t) writeTestFile(t, root, ".gitignore", "repository-ignored.txt\n") @@ -70,6 +73,8 @@ func TestEnumerateGitFilesHonorsNegatedRules(t *testing.T) { } } +// A ~/.gitignore_global excludes untracked files even when no git configuration +// references it: [enumerateGitFiles] passes it to git as an extra exclude file. func TestEnumerateGitFilesHonorsDetectedHomeGitignoreGlobal(t *testing.T) { root := initRepository(t) home := t.TempDir() @@ -123,6 +128,8 @@ func runTestGit(t *testing.T, root string, args ...string) { } } +// writeTestFile writes content to relativePath under root, creating any missing +// parent directories, and returns the path it wrote. func writeTestFile(t *testing.T, root, relativePath, content string) string { t.Helper() path := filepath.Join(root, filepath.FromSlash(relativePath)) diff --git a/cfcli/options.go b/cfcli/options.go index 8b7cbe1..6588432 100644 --- a/cfcli/options.go +++ b/cfcli/options.go @@ -5,6 +5,10 @@ import ( "strings" ) +// scanMode selects how far a scan runs and what it reports. modeQuick stops at +// the first finding that is neither allowed nor excluded; modeJSON scans every +// file and reports allowed and excluded matches alongside the findings. +// parseOptions accepts no other value. type scanMode string const ( @@ -12,6 +16,10 @@ const ( modeJSON scanMode = "json" ) +// options holds a parsed command line. listPath is nil unless a second +// positional argument named a file listing the paths to scan. printDetails +// takes effect only in modeQuick; [runWithDependencies] warns when another mode +// is in force. type options struct { mode scanMode printDetails bool @@ -20,12 +28,22 @@ type options struct { listPath *string } +// usageError reports a positional argument count other than the one or two +// parseOptions accepts. [runWithDependencies] matches it with errors.As and +// answers with the help text alone, so its message never reaches the user. type usageError struct{} func (e *usageError) Error() string { return "invalid argument count" } +// parseOptions reads the leading --mode, --print, and --verbose flags, then the +// positional arguments: the root to scan and, optionally, a file listing the +// paths to scan. The mode defaults to modeJSON. +// +// The flags may appear in any order but not after a positional argument, and an +// unrecognized flag value is rejected. A positional count other than one or two +// returns a [usageError]. func parseOptions(args []string) (options, error) { parsed := options{mode: modeJSON} diff --git a/cfcli/output.go b/cfcli/output.go index 2b2776e..99f9416 100644 --- a/cfcli/output.go +++ b/cfcli/output.go @@ -8,6 +8,9 @@ import ( "sync" ) +// lineOutput writes cfcli's output protocol: one complete line per call, +// prefixed with TEXT: or JSON: and flushed before the call returns. Safe for +// concurrent use by multiple goroutines. type lineOutput struct { writer *bufio.Writer mu sync.Mutex @@ -17,6 +20,8 @@ func newLineOutput(writer io.Writer) *lineOutput { return &lineOutput{writer: bufio.NewWriter(writer)} } +// text writes one TEXT: line. format is a [fmt.Printf] format string, so data +// such as a file path or a matched value goes in an argument. func (o *lineOutput) text(format string, args ...any) error { o.mu.Lock() defer o.mu.Unlock() @@ -27,6 +32,8 @@ func (o *lineOutput) text(format string, args ...any) error { return o.writer.Flush() } +// json writes value as one JSON: line, encoded by [json.Marshal]. Nothing +// reaches the writer when value cannot be encoded. func (o *lineOutput) json(value any) error { encoded, err := json.Marshal(value) if err != nil { diff --git a/cfcli/output_test.go b/cfcli/output_test.go index d9a31db..8a81ce9 100644 --- a/cfcli/output_test.go +++ b/cfcli/output_test.go @@ -19,6 +19,8 @@ func TestLineOutputWritesAndFlushesText(t *testing.T) { } } +// An encoded value occupies a single flushed line, so a reader that splits the +// output on newlines gets whole JSON documents. func TestLineOutputWritesAndFlushesJSON(t *testing.T) { var destination bytes.Buffer output := newLineOutput(&destination) diff --git a/cfcli/paths.go b/cfcli/paths.go index 55a1b40..e0d0d84 100644 --- a/cfcli/paths.go +++ b/cfcli/paths.go @@ -10,8 +10,17 @@ import ( "strings" ) +// maxListedPathLength is the longest line [readListedPaths] accepts from a list +// file, in bytes. A longer line fails the read instead of being truncated. const maxListedPathLength = 1024 * 1024 +// selectFiles returns the sorted absolute paths of the files to scan under +// rootArg, which must name a directory. [enumerateGitFiles] decides which paths +// are eligible: everything Git tracks, plus the untracked files no ignore rule +// covers. A non-nil listArg names a file of paths relative to rootArg that +// narrows the selection further: an entry Git does not report is skipped, while +// an absolute entry or one escaping rootArg fails the call. Only regular files +// survive, so a symbolic link to a directory is neither returned nor traversed. func selectFiles(ctx context.Context, rootArg string, listArg *string) ([]string, error) { root, err := filepath.Abs(rootArg) if err != nil { @@ -67,6 +76,9 @@ func selectFiles(ctx context.Context, rootArg string, listArg *string) ([]string return result, nil } +// readListedPaths returns the non-empty lines of the file at path, each with a +// trailing carriage return removed so a CRLF list reads like an LF one. A line +// longer than [maxListedPathLength] fails the read. func readListedPaths(path string) ([]string, error) { file, err := os.Open(path) if err != nil { @@ -89,6 +101,10 @@ func readListedPaths(path string) ([]string, error) { return paths, nil } +// validateRelativeGitPath rejects a listed path that cannot name a file inside +// FOLDER_PATH: an absolute path, one that resolves to the folder itself, and one +// that escapes it through "..". The error names the rule that was broken and +// reaches the user, wrapped by [selectFiles]. func validateRelativeGitPath(path string) error { nativePath := filepath.FromSlash(path) if filepath.IsAbs(nativePath) { diff --git a/cfcli/paths_test.go b/cfcli/paths_test.go index b94dfab..e3785e8 100644 --- a/cfcli/paths_test.go +++ b/cfcli/paths_test.go @@ -28,6 +28,9 @@ func TestSelectFilesReturnsSortedAbsoluteGitSelection(t *testing.T) { } } +// A listed path is selected only when Git also reports it, so an entry that is +// ignored or nonexistent drops out. Blank lines, repeated entries, and CRLF line +// endings leave the selection unchanged. func TestSelectFilesRestrictsResultsToListedPaths(t *testing.T) { root := initRepository(t) included := writeTestFile(t, root, "dir/included.txt", "included") @@ -102,6 +105,8 @@ func TestSelectFilesRejectsInvalidRoot(t *testing.T) { } } +// A tracked symbolic link to a file is selected; one to a directory is not, and +// neither are the files behind it. func TestSelectFilesHandlesSymbolicLinksWithoutTraversingDirectories(t *testing.T) { root := initRepository(t) targetFile := writeTestFile(t, root, "target.txt", "target") diff --git a/cfcli/regexp_error_path_test.go b/cfcli/regexp_error_path_test.go index 7dc7a2d..4dc3bd9 100644 --- a/cfcli/regexp_error_path_test.go +++ b/cfcli/regexp_error_path_test.go @@ -9,6 +9,9 @@ import ( "time" ) +// A dictionary expression Go RE2 cannot compile ends the run with exit code 3, +// and the message on stderr carries the absolute path of the cache file that +// holds the expression. func TestRegexpCompileErrorIncludesDictionaryPath(t *testing.T) { root := initRepository(t) home := t.TempDir() diff --git a/cfcli/scanner.go b/cfcli/scanner.go index 95ab2a6..c8842cc 100644 --- a/cfcli/scanner.go +++ b/cfcli/scanner.go @@ -7,8 +7,17 @@ import ( "strings" ) +// maxFindingsPerSignaturePerFile caps the found events one file may report for +// one signature key. Signatures that share a key share the cap, and allowed and +// excluded matches do not count against it; reaching the cap ends the file's +// reporting for that key altogether. const maxFindingsPerSignaturePerFile = 5 +// finding is the JSON event carrying one signature match. +// +// Type is found, allowed, or excluded, and only a found event makes the scan +// report a failure. Line is the 1-based line holding the match, and File is +// relative to the scan root with / separators. type finding struct { Type string `json:"type"` Key string `json:"key"` @@ -17,17 +26,29 @@ type finding struct { File string `json:"file"` } +// excludedPathEvent is the JSON event for a file or folder the grand report +// excludes whole. A single match suppressed by an exact-text exclusion is +// reported as a [finding] with Type excluded instead. type excludedPathEvent struct { Type string `json:"type"` File string `json:"file"` } +// listPathEvent is the JSON event verbose mode emits for a folder or file before +// the scan reaches it. Exactly one of File and Folder is set, and a folder is +// reported once however many of its files follow. type listPathEvent struct { Type string `json:"type"` File string `json:"file,omitempty"` Folder string `json:"folder,omitempty"` } +// scanResult summarizes one scan pass. +// +// found is true when at least one match survived both the dictionary allowed +// list and the grand-report exclusions. scannedCount counts the files that were +// read, so it excludes files skipped by a path exclusion or a read error, and in +// quick mode it stops at the file holding the first finding. type scanResult struct { found bool scannedCount int @@ -54,6 +75,21 @@ func scanFilesWithExclusions( return scanFilesConfigured(root, files, loaded, mode, exclusions, false, false, output, errors) } +// scanFilesConfigured scans files for dictionary signature matches and reports +// them to output as JSON. files holds absolute paths under root, and root anchors +// the relative paths that appear in the events and in the exclusion lookups. A +// file that cannot be read produces a warning on errors and is not counted as +// scanned; the returned error covers only path resolution and failed writes. +// +// modeJSON also reports the matches the dictionary allowed list or the grand +// report suppresses, until the key hits its [maxFindingsPerSignaturePerFile] +// cap. modeQuick reports neither and returns as soon as one match survives +// both, leaving the remaining files unread; printDetails then adds the +// copy-ready cfcli exclude commands for that match, and without it output +// carries a one-line hint. verbose emits a [listPathEvent] for each folder and +// file the scan walks, in either mode; a path the exclusions name is listed and +// then reported as an [excludedPathEvent], and below an excluded folder only +// such paths are listed. func scanFilesConfigured( root string, files []string, @@ -241,6 +277,12 @@ func newFinding(key, exact string, line int, relativePath string) finding { } } +// lineAt advances from cursor to offset over content and reports the 1-based +// number of the line holding offset. LF, CRLF, and a lone CR each end one line. +// +// Calls have to walk content forward: cursor and line come from the previous call +// on the same content, or from 0 and 1 at the start, and offset must not be +// before cursor. The returned cursor is offset, ready to be passed back. func lineAt(content []byte, offset, cursor, line int) (int, int) { for cursor < offset { switch content[cursor] { @@ -260,6 +302,9 @@ func lineAt(content []byte, offset, cursor, line int) (int, int) { return cursor, line } +// relativeParentPaths returns the ancestor folders of a slash-separated relative +// path, outermost first, excluding both the scan root and the path itself. A path +// directly under the root yields an empty slice. func relativeParentPaths(relativePath string) []string { parents := make([]string, 0) for parent := filepath.ToSlash(filepath.Dir(filepath.FromSlash(relativePath))); parent != "." && parent != ""; parent = filepath.ToSlash(filepath.Dir(filepath.FromSlash(parent))) { diff --git a/cfcli/scanner_test.go b/cfcli/scanner_test.go index 28a45b1..3000196 100644 --- a/cfcli/scanner_test.go +++ b/cfcli/scanner_test.go @@ -78,6 +78,8 @@ func TestScanFilesJSONEmitsCompleteFindingsAndTotal(t *testing.T) { } } +// Line numbers count from 1, and LF, CRLF, and a lone CR each end exactly one +// line. Two matches on one line both report that line. func TestScanFilesReportsOneBasedMatchLines(t *testing.T) { root := t.TempDir() file := writeTestFile(t, root, "lines.txt", "SECRET\né SECRET\r\nx SECRET\ry SECRET SECRET") @@ -114,6 +116,9 @@ func TestScanFilesReportsOneBasedMatchLines(t *testing.T) { } } +// A signature's excluded extensions are compared against the lowercased file +// extension, so a .ZIP file skips a signature that lists zip. The file is still +// read and counted as scanned. func TestScanFilesHonorsExcludedExtensionsCaseInsensitively(t *testing.T) { root := t.TempDir() file := writeTestFile(t, root, "archive.ZIP", "SECRET") @@ -375,6 +380,9 @@ func TestScanFilesWithExclusionsSuppressesOnlyExactFileMatch(t *testing.T) { } } +// Quick mode reports nothing about the exclusions it applies: neither a skipped +// path nor a suppressed match produces an event. It walks past both and stops at +// the first match the grand report leaves alone. func TestScanFilesWithExclusionsQuickOutputIsSilent(t *testing.T) { root := t.TempDir() excludedFile := writeTestFile(t, root, "excluded.txt", "SECRET") @@ -410,6 +418,8 @@ func TestScanFilesWithExclusionsQuickOutputIsSilent(t *testing.T) { } } +// A match that the dictionary allows and the grand report also excludes is +// reported as excluded, not as allowed, and does not count as a finding. func TestScanFilesWithExclusionsReportsAllowedExcludedMatch(t *testing.T) { root := t.TempDir() file := writeTestFile(t, root, "allowed.txt", "SECRET")