From 110728fbb429a6bff83ca6520b75ff1c3992b7bb Mon Sep 17 00:00:00 2001 From: Vladimir Sitnikov Date: Tue, 11 Aug 2026 11:04:54 +0300 Subject: [PATCH 1/2] 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") From ae18c71903fa4dc6236531ad23f7f0bac7aad628 Mon Sep 17 00:00:00 2001 From: Vladimir Sitnikov Date: Tue, 11 Aug 2026 11:09:05 +0300 Subject: [PATCH 2/2] refactor(cfcli): replace the bare exit-code ints with a named type runWithDependencies returned an untyped int, so each of the 21 return sites spelled its exit code as a literal and the only description of what those literals meant was a four-row table in a doc comment. A table in a comment drifts; a constant does not. exitStatus and its four values move that vocabulary into the code, where each value carries the condition it reports and the compiler keeps the name next to the number. run, runWithDependencies, and runExcludeCommand return the type instead of int, main converts once at os.Exit, and the tests compare against the names rather than 0 through 3. Behavior is unchanged: the same four numbers reach the process exit in the same conditions. README.md's exit-0 line gains the exclude command, which it had always omitted. Co-Authored-By: Claude Opus 5 --- cfcli/README.md | 2 +- cfcli/app.go | 51 +++++++++++++++------------------ cfcli/app_pipeline_test.go | 18 ++++++------ cfcli/app_test.go | 18 ++++++------ cfcli/exclude_command.go | 10 +++---- cfcli/exclude_command_test.go | 14 ++++----- cfcli/exit_status.go | 18 ++++++++++++ cfcli/main.go | 2 +- cfcli/regexp_error_path_test.go | 2 +- 9 files changed, 74 insertions(+), 61 deletions(-) create mode 100644 cfcli/exit_status.go diff --git a/cfcli/README.md b/cfcli/README.md index 8e202e2..288b48f 100644 --- a/cfcli/README.md +++ b/cfcli/README.md @@ -155,7 +155,7 @@ the stop. Exit codes are: -- `0`: The scan completes without findings. +- `0`: The scan completes without findings, or the `exclude` command succeeds. - `1`: Arguments, cache access, password lookup, decryption, or another runtime operation fails. - `2`: At least one non-allowed match is found. - `3`: A dictionary expression is incompatible with Go RE2. diff --git a/cfcli/app.go b/cfcli/app.go index b0defc7..d075a0b 100644 --- a/cfcli/app.go +++ b/cfcli/app.go @@ -22,8 +22,8 @@ type appDependencies struct { } // 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 { +// and the wall clock. It returns the exit status documented on [runWithDependencies]. +func run(ctx context.Context, args []string, stdout, stderr io.Writer) exitStatus { return runWithDependencies(ctx, args, stdout, stderr, appDependencies{ refresher: cacheRefresher{ client: &http.Client{}, @@ -37,14 +37,9 @@ 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 +// runWithDependencies performs one cfcli invocation against the given dependencies and returns +// [exitClean], [exitFailure], [exitFindings], or [exitBadExpression], each of which documents the +// condition it reports. // // 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 @@ -54,7 +49,7 @@ func runWithDependencies( args []string, stdout, stderr io.Writer, dependencies appDependencies, -) int { +) exitStatus { if isExcludeCommand(args) { return runExcludeCommand(args, stdout, stderr) } @@ -68,35 +63,35 @@ func runWithDependencies( writeFatal(errorOutput, "%v", err) } _ = writeHelp(errorOutput) - return 1 + return exitFailure } if parsed.printDetails && parsed.mode != modeQuick { if err := output.text( "Warning: --print=details applies only to --mode=quick and will be ignored.", ); err != nil { writeFatal(errorOutput, "Cannot write print option warning: %v", err) - return 1 + return exitFailure } } cache, err := dependencies.refresher.refresh(ctx, errorOutput) if err != nil { writeFatal(errorOutput, "Cannot prepare dictionary cache: %v", err) - return 1 + return exitFailure } cachePath := cache.path if err := output.text("%s", dictionaryStatusMessage(cache.state)); err != nil { writeFatal(errorOutput, "Cannot write dictionary status: %v", err) - return 1 + return exitFailure } if err := output.text("Dictionary path: %s", dictionaryDisplayPath(cache.path, cache.home)); err != nil { writeFatal(errorOutput, "Cannot write dictionary path: %v", err) - return 1 + return exitFailure } encrypted, err := os.ReadFile(cachePath) if err != nil { writeFatal(errorOutput, "Cannot read dictionary cache %q: %v", cachePath, err) - return 1 + return exitFailure } password := "" if dependencies.getenv != nil { @@ -105,7 +100,7 @@ func runWithDependencies( plaintext, err := decryptDictionary(encrypted, password) if err != nil { writeFatal(errorOutput, "%v", err) - return 1 + return exitFailure } loaded, err := loadDictionary(plaintext, errorOutput) @@ -117,14 +112,14 @@ func runWithDependencies( dictionaryPath = absolutePath } writeFatal(errorOutput, "Cannot compile dictionary regexp from \"%s\": %v", dictionaryPath, compileError) - return 3 + return exitBadExpression } writeFatal(errorOutput, "Cannot load dictionary: %v", err) - return 1 + return exitFailure } if err := output.text("Dictionary version: %s", loaded.version); err != nil { writeFatal(errorOutput, "Cannot write dictionary version: %v", err) - return 1 + return exitFailure } now := dependencies.now @@ -134,14 +129,14 @@ func runWithDependencies( startedAt := now() if err := output.text("Scanning is in progress. Please wait."); err != nil { writeFatal(errorOutput, "Cannot write scanning progress: %v", err) - return 1 + return exitFailure } exclusions := loadExclusions(parsed.root, errorOutput) files, err := selectFiles(ctx, parsed.root, parsed.listPath) if err != nil { writeFatal(errorOutput, "%v", err) - return 1 + return exitFailure } result, err := scanFilesConfigured( parsed.root, @@ -156,21 +151,21 @@ func runWithDependencies( ) if err != nil { writeFatal(errorOutput, "Cannot scan files: %v", err) - return 1 + return exitFailure } if err := output.text("Total files scanned %d", result.scannedCount); err != nil { writeFatal(errorOutput, "Cannot write scanned file count: %v", err) - return 1 + return exitFailure } elapsed := now().Sub(startedAt).Seconds() if err := output.text("Scanning is finished in %.3f seconds.", elapsed); err != nil { writeFatal(errorOutput, "Cannot write scanning duration: %v", err) - return 1 + return exitFailure } if result.found { - return 2 + return exitFindings } - return 0 + return exitClean } func writeHelp(output *lineOutput) error { diff --git a/cfcli/app_pipeline_test.go b/cfcli/app_pipeline_test.go index 794e66a..f7f98a9 100644 --- a/cfcli/app_pipeline_test.go +++ b/cfcli/app_pipeline_test.go @@ -22,7 +22,7 @@ func TestRunWithDependenciesRequiresPassword(t *testing.T) { exitCode := runWithDependencies(context.Background(), []string{root}, &stdout, &stderr, dependencies) - if exitCode != 1 || !strings.Contains(stderr.String(), "TEXT: Dictionary password") { + if exitCode != exitFailure || !strings.Contains(stderr.String(), "TEXT: Dictionary password") { t.Fatalf("exit code = %d, stderr = %q", exitCode, stderr.String()) } } @@ -38,7 +38,7 @@ func TestRunWithDependenciesMapsInvalidRegexpToExitCodeThree(t *testing.T) { exitCode := runWithDependencies(context.Background(), []string{root}, &stdout, &stderr, dependencies) - if exitCode != 3 || !strings.Contains(stderr.String(), "TEXT: Cannot compile dictionary regexp") { + if exitCode != exitBadExpression || !strings.Contains(stderr.String(), "TEXT: Cannot compile dictionary regexp") { t.Fatalf("exit code = %d, stderr = %q", exitCode, stderr.String()) } } @@ -55,7 +55,7 @@ func TestRunWithDependenciesQuickReturnsTwoOnFirstFinding(t *testing.T) { exitCode := runWithDependencies(context.Background(), []string{"--mode=quick", root}, &stdout, &stderr, dependencies) - if exitCode != 2 || !strings.Contains( + if exitCode != exitFindings || !strings.Contains( stdout.String(), `JSON: {"type":"found","key":"SECRET","found":"SECRET","line":1,"file":"secret.txt"}`, ) || !strings.Contains( @@ -93,7 +93,7 @@ func TestRunWithDependenciesQuickPrintDetailsPrintsExclusionCommands(t *testing. dependencies, ) - if exitCode != 2 || + if exitCode != exitFindings || !strings.Contains(stdout.String(), "TEXT: POSIX: cfcli exclude ") || !strings.Contains(stdout.String(), "TEXT: PowerShell: cfcli exclude ") || !strings.Contains(stdout.String(), "TEXT: cmd.exe: cfcli exclude ") || @@ -114,7 +114,7 @@ func TestRunWithDependenciesJSONCompletesAndReturnsTwo(t *testing.T) { exitCode := runWithDependencies(context.Background(), []string{root}, &stdout, &stderr, dependencies) - if exitCode != 2 || !strings.Contains(stdout.String(), `JSON: {"type":"found","key":"SECRET"`) || + if exitCode != exitFindings || !strings.Contains(stdout.String(), `JSON: {"type":"found","key":"SECRET"`) || strings.Contains(stdout.String(), "cfcli exclude ") || !strings.HasSuffix( stdout.String(), @@ -142,7 +142,7 @@ func TestRunWithDependenciesPrintDetailsWarnsOutsideQuickMode(t *testing.T) { dependencies, ) - if exitCode != 2 || + if exitCode != exitFindings || !strings.Contains( stdout.String(), "TEXT: Warning: --print=details applies only to --mode=quick and will be ignored.\n", @@ -189,7 +189,7 @@ func TestRunWithDependenciesAcceptsRelativeFolderPath(t *testing.T) { dependencies, ) - if exitCode != 0 || !strings.HasSuffix( + if exitCode != exitClean || !strings.HasSuffix( stdout.String(), "TEXT: Total files scanned 1\nTEXT: Scanning is finished in 1.234 seconds.\n", ) { @@ -211,7 +211,7 @@ func TestRunWithDependenciesAppliesGrandReportExclusions(t *testing.T) { exitCode := runWithDependencies(context.Background(), []string{root}, &stdout, &stderr, dependencies) - if exitCode != 0 || !strings.Contains( + if exitCode != exitClean || !strings.Contains( stdout.String(), `JSON: {"type":"excluded","key":"SECRET","found":"SECRET","line":1,"file":"secret.txt"}`, ) { @@ -237,7 +237,7 @@ func TestRunWithDependenciesEnablesVerboseListOutput(t *testing.T) { dependencies, ) - if exitCode != 0 || !strings.Contains(stdout.String(), `JSON: {"type":"list","file":"safe.txt"}`) { + if exitCode != exitClean || !strings.Contains(stdout.String(), `JSON: {"type":"list","file":"safe.txt"}`) { t.Fatalf("exit code = %d, stdout = %q, stderr = %q", exitCode, stdout.String(), stderr.String()) } } diff --git a/cfcli/app_test.go b/cfcli/app_test.go index e6ec1fe..722c589 100644 --- a/cfcli/app_test.go +++ b/cfcli/app_test.go @@ -20,8 +20,8 @@ func TestRunRejectsInvalidArgumentCounts(t *testing.T) { exitCode := runWithDependencies(context.Background(), args, &stdout, &stderr, appDependencies{}) - if exitCode != 1 { - t.Fatalf("run(%q) exit code = %d, want 1", args, exitCode) + if exitCode != exitFailure { + t.Fatalf("run(%q) exit code = %d, want %d", args, exitCode, exitFailure) } if stdout.Len() != 0 { t.Fatalf("run(%q) stdout = %q, want empty", args, stdout.String()) @@ -51,7 +51,7 @@ func TestRunPrintsExpandedHelpForInsufficientArguments(t *testing.T) { exitCode := runWithDependencies(context.Background(), args, &stdout, &stderr, appDependencies{}) - if exitCode != 1 || stdout.Len() != 0 { + if exitCode != exitFailure || stdout.Len() != 0 { t.Fatalf("run(%q) exit code = %d, stdout = %q", args, exitCode, stdout.String()) } for _, expected := range []string{ @@ -86,8 +86,8 @@ func TestRunReportsTotalFilesScanned(t *testing.T) { testAppDependencies(t, "VERSION=1.0\n", "test-password"), ) - if exitCode != 0 { - t.Fatalf("run() exit code = %d, want 0; stderr = %q", exitCode, stderr.String()) + if exitCode != exitClean { + t.Fatalf("run() exit code = %d, want %d; stderr = %q", exitCode, exitClean, stderr.String()) } want := currentDictionaryOutput() + "TEXT: Dictionary version: 1.0\n" + @@ -138,8 +138,8 @@ func TestRunReportsRuntimeErrors(t *testing.T) { testAppDependencies(t, "VERSION=1.0\n", "test-password"), ) - if exitCode != 1 { - t.Fatalf("run() exit code = %d, want 1", exitCode) + if exitCode != exitFailure { + t.Fatalf("run() exit code = %d, want %d", exitCode, exitFailure) } wantOutput := currentDictionaryOutput() + "TEXT: Dictionary version: 1.0\nTEXT: Scanning is in progress. Please wait.\n" @@ -167,8 +167,8 @@ func TestRunReportsUnavailableGit(t *testing.T) { testAppDependencies(t, "VERSION=1.0\n", "test-password"), ) - if exitCode != 1 { - t.Fatalf("run() exit code = %d, want 1", exitCode) + if exitCode != exitFailure { + t.Fatalf("run() exit code = %d, want %d", exitCode, exitFailure) } wantOutput := currentDictionaryOutput() + "TEXT: Dictionary version: 1.0\nTEXT: Scanning is in progress. Please wait.\n" diff --git a/cfcli/exclude_command.go b/cfcli/exclude_command.go index efca687..ca9e5e2 100644 --- a/cfcli/exclude_command.go +++ b/cfcli/exclude_command.go @@ -100,22 +100,22 @@ func isExcludeCommand(args []string) bool { return len(args) > 0 && args[0] == "exclude" } -func runExcludeCommand(args []string, stdout, stderr io.Writer) int { +func runExcludeCommand(args []string, stdout, stderr io.Writer) exitStatus { errorOutput := newLineOutput(stderr) if len(args) != 3 { _ = writeHelp(errorOutput) - return 1 + return exitFailure } reportPath, err := updateExclusions(args[1], args[2]) if err != nil { writeFatal(errorOutput, "%v", err) - return 1 + return exitFailure } if err := newLineOutput(stdout).text("Exclusions file was updated: %s", reportPath); err != nil { writeFatal(errorOutput, "Cannot write updated exclusions path: %v", err) - return 1 + return exitFailure } - return 0 + return exitClean } type shellCommand struct { diff --git a/cfcli/exclude_command_test.go b/cfcli/exclude_command_test.go index 3d82e11..0353888 100644 --- a/cfcli/exclude_command_test.go +++ b/cfcli/exclude_command_test.go @@ -250,8 +250,8 @@ func TestRunExcludeUpdatesReportWithoutScanDependencies(t *testing.T) { ) reportPath := filepath.Join(root, ".qubership", "grand-report.json") - if exitCode != 0 { - t.Fatalf("exit code = %d, want 0; stderr = %q", exitCode, stderr.String()) + if exitCode != exitClean { + t.Fatalf("exit code = %d, want %d; stderr = %q", exitCode, exitClean, stderr.String()) } if stdout.String() != fmt.Sprintf("TEXT: Exclusions file was updated: %s\n", reportPath) { t.Fatalf("stdout = %q", stdout.String()) @@ -275,7 +275,7 @@ func TestRunExcludeRejectsUnsupportedType(t *testing.T) { ) want := "TEXT: Cannot exclude event type \"allowed\": only \"found\" is supported. No files were changed.\n" - if exitCode != 1 || stdout.Len() != 0 || stderr.String() != want { + if exitCode != exitFailure || stdout.Len() != 0 || stderr.String() != want { t.Fatalf("exit code = %d, stdout = %q, stderr = %q; want stderr %q", exitCode, stdout.String(), stderr.String(), want) } if _, err := os.Stat(filepath.Join(root, ".qubership", "grand-report.json")); !os.IsNotExist(err) { @@ -310,7 +310,7 @@ func TestRunExcludeRejectsInvalidInvocation(t *testing.T) { appDependencies{}, ) - if exitCode != 1 || stdout.Len() != 0 || !strings.HasPrefix(stderr.String(), "TEXT:") { + if exitCode != exitFailure || stdout.Len() != 0 || !strings.HasPrefix(stderr.String(), "TEXT:") { t.Fatalf("exit code = %d, stdout = %q, stderr = %q", exitCode, stdout.String(), stderr.String()) } if _, err := os.Stat(filepath.Join(root, ".qubership", "grand-report.json")); !os.IsNotExist(err) { @@ -335,7 +335,7 @@ func TestRunExcludeLeavesMalformedReportUnchanged(t *testing.T) { appDependencies{}, ) - if exitCode != 1 || stdout.Len() != 0 || !strings.Contains(stderr.String(), "No files were changed.") { + if exitCode != exitFailure || stdout.Len() != 0 || !strings.Contains(stderr.String(), "No files were changed.") { t.Fatalf("exit code = %d, stdout = %q, stderr = %q", exitCode, stdout.String(), stderr.String()) } content, err := os.ReadFile(reportPath) @@ -389,7 +389,7 @@ func TestRunScansWithListFileNamedExclude(t *testing.T) { testAppDependencies(t, "VERSION=1.0\n", "test-password"), ) - if exitCode != 0 || !strings.Contains(stdout.String(), "TEXT: Total files scanned 1\n") { + if exitCode != exitClean || !strings.Contains(stdout.String(), "TEXT: Total files scanned 1\n") { t.Fatalf("exit code = %d, stdout = %q, stderr = %q", exitCode, stdout.String(), stderr.String()) } } @@ -407,7 +407,7 @@ func TestRunRejectsRemovedFolderFirstExcludeSyntax(t *testing.T) { appDependencies{}, ) - if exitCode != 1 || stdout.Len() != 0 || !strings.Contains(stderr.String(), "TEXT: Usage:") { + if exitCode != exitFailure || stdout.Len() != 0 || !strings.Contains(stderr.String(), "TEXT: Usage:") { t.Fatalf("exit code = %d, stdout = %q, stderr = %q", exitCode, stdout.String(), stderr.String()) } if _, err := os.Stat(filepath.Join(root, ".qubership", "grand-report.json")); !os.IsNotExist(err) { diff --git a/cfcli/exit_status.go b/cfcli/exit_status.go new file mode 100644 index 0000000..57e2d8d --- /dev/null +++ b/cfcli/exit_status.go @@ -0,0 +1,18 @@ +package main + +// exitStatus is the process status one cfcli invocation ends with. [run] returns it and main +// passes it to os.Exit, so the values below are a published interface: they are what a script +// calling cfcli branches on, and README.md documents the same four. +type exitStatus int + +const ( + // exitClean reports that the scan found nothing, or that the exclude subcommand succeeded. + exitClean exitStatus = 0 + // exitFailure reports any failure the other three do not name: bad arguments, the dictionary, + // decryption, scanning, an exclusion update, or a write. + exitFailure exitStatus = 1 + // exitFindings reports at least one finding that no allow rule or exclusion covered. + exitFindings exitStatus = 2 + // exitBadExpression reports a dictionary expression that does not compile under Go's RE2 engine. + exitBadExpression exitStatus = 3 +) diff --git a/cfcli/main.go b/cfcli/main.go index eca5d2b..f59867b 100644 --- a/cfcli/main.go +++ b/cfcli/main.go @@ -6,5 +6,5 @@ import ( ) func main() { - os.Exit(run(context.Background(), os.Args[1:], os.Stdout, os.Stderr)) + os.Exit(int(run(context.Background(), os.Args[1:], os.Stdout, os.Stderr))) } diff --git a/cfcli/regexp_error_path_test.go b/cfcli/regexp_error_path_test.go index 4dc3bd9..67ca9db 100644 --- a/cfcli/regexp_error_path_test.go +++ b/cfcli/regexp_error_path_test.go @@ -36,7 +36,7 @@ func TestRegexpCompileErrorIncludesDictionaryPath(t *testing.T) { if err != nil { t.Fatal(err) } - if exitCode != 3 || !strings.Contains(stderr.String(), absolutePath) { + if exitCode != exitBadExpression || !strings.Contains(stderr.String(), absolutePath) { t.Fatalf("exit code = %d, stderr = %q; want dictionary path %q", exitCode, stderr.String(), absolutePath) } }