diff --git a/app/cli/internal/trace/README.md b/app/cli/internal/trace/README.md index 3df2f4458..6072463ae 100644 --- a/app/cli/internal/trace/README.md +++ b/app/cli/internal/trace/README.md @@ -85,6 +85,7 @@ How the evidence is protected before it leaves the machine. |---------|---------------|-------------|--------|--------------|------------| | Secret redaction | Secrets in the session evidence and in the spec materials are replaced with a placeholder before upload. | Yes | Yes | Yes | Yes | | Media skipped by redaction | The secret redaction does not scan base64 media (images, PDFs) in the transcript, so it is faster and does not break the media. | Yes | Unknown | Not applicable | Not applicable | +| Allow markers in redaction | A secret on a line that carries an inline `gitleaks:allow` or `betterleaks:allow` marker, such as a test fixture the agent read, is not redacted. The marker covers only its own line. | Yes | Yes | Yes | Yes | Why not Yes: diff --git a/internal/redaction/betterleaks.go b/internal/redaction/betterleaks.go index 357312c33..439952cb0 100644 --- a/internal/redaction/betterleaks.go +++ b/internal/redaction/betterleaks.go @@ -18,6 +18,7 @@ package redaction import ( "context" "crypto/sha256" + _ "embed" "fmt" "runtime" "slices" @@ -25,6 +26,7 @@ import ( "strings" "sync" + "github.com/betterleaks/betterleaks/config" "github.com/betterleaks/betterleaks/detect" betterleaksregexp "github.com/betterleaks/betterleaks/regexp" "github.com/betterleaks/betterleaks/regexp/re2" @@ -67,6 +69,18 @@ type betterleaksScanner struct { chunkSize int } +// rulesConfig is the betterleaks default ruleset, extended with allowlists for +// values the generic rules match but which are not credentials. +// +//go:embed betterleaks.toml +var rulesConfig string + +// RulesetVersion identifies what the scanner detects. Bump it whenever +// betterleaks.toml, the betterleaks version, or the handling of findings in +// this file changes what gets redacted, so that policies can tell placeholders +// from different rulesets apart. Version 1 is the plain default ruleset. +const RulesetVersion = 2 + var ( defaultScanner = sync.OnceValues(newBetterleaksScanner) // useRE2 selects the regex engine. It is process-wide and only affects @@ -88,21 +102,30 @@ func DefaultScanner() (Scanner, error) { } func newBetterleaksScanner() (*betterleaksScanner, error) { + return newScannerWithRules(rulesConfig) +} + +// newScannerWithRules builds a scanner from a betterleaks TOML configuration. +func newScannerWithRules(rules string) (*betterleaksScanner, error) { // The library defaults to the standard library engine, which is several // times slower on this ruleset; the betterleaks CLI defaults to RE2 for the // same reason. useRE2() - // Validation stays off, which is what the default constructor gives us: - // validating would reach out to third-party APIs to check whether a candidate - // credential is live, and crafting a material must not do that. - d, err := detect.NewDetectorDefaultConfig() + cfg, err := config.ParseTOMLString(rules, "") if err != nil { - return nil, fmt.Errorf("loading the default secret scanning rules: %w", err) + return nil, fmt.Errorf("loading the secret scanning rules: %w", err) } - // A session transcript is text the model was free to write, so an in-band - // "betterleaks:allow" or "gitleaks:allow" must not switch redaction off. + // Validation stays off: validating would reach out to third-party APIs to + // check whether a candidate credential is live, and crafting a material + // must not do that. + d := detect.NewDetectorContext(context.Background(), cfg, detect.ValidationOptions{}) + + // The detector checks an allow marker against the whole line of its match. + // A string leaf of the scanned document is a single line, so one marker + // would suppress every finding in a whole tool result. Redactor checks the + // marker against the line of each occurrence instead, when asked to. d.IgnoreGitleaksAllow = true // Findings must carry the verbatim secret: it is what we search for in the // document in order to replace it. @@ -169,10 +192,12 @@ func (s *betterleaksScanner) scanChunks(ctx context.Context, text string, chunks // and the rule for that component is marked as not independently // reportable — so the primary finding names the harmless public // identifier while the actual credential arrives only as a component. - // Both halves have to be redacted. + // Both halves have to be redacted. An optional component is context + // that only raises the confidence of the finding, such as the username + // next to a generic password, and is not a secret. for _, set := range result.Finding.ComponentSets { for _, component := range set.Components { - if component == nil { + if component == nil || component.Optional { continue } perChunk[chunk] = appendSecret(perChunk[chunk], component.RuleID, component.Secret) @@ -332,8 +357,11 @@ func byteSpans(text string, ranges []lineSpan) []span { // appendSecret records a locatable secret. A finding without one cannot be // searched for in the document, so it is dropped rather than reported. func appendSecret(dst []Finding, ruleID, secret string) []Finding { - if ruleID == jwtRuleID { + switch ruleID { + case jwtRuleID: secret = cutAtTerminatorEscape(secret) + case genericPasswordRuleID: + secret = cutAtLineEscape(secret) } if secret == "" { return dst @@ -341,8 +369,17 @@ func appendSecret(dst []Finding, ruleID, secret string) []Finding { return append(dst, Finding{RuleID: ruleID, Secret: secret}) } -// jwtRuleID is the default ruleset's rule for JSON Web Tokens. -const jwtRuleID = "jwt" +const ( + // jwtRuleID is the default ruleset's rule for JSON Web Tokens. + jwtRuleID = "jwt" + // genericPasswordRuleID is the default ruleset's rule for a password + // assigned to a password key. Its unquoted value stops at whitespace in + // source text, but in JSON-encoded text a line break is the escape `\n`, so + // the match runs into the next line. That swallowed, for instance, the line + // break after a placeholder the transcript already showed, and reported the + // placeholder as a new secret. + genericPasswordRuleID = "generic-password" +) // cutAtTerminatorEscape removes the text from the first whitespace or quote // escape onwards. @@ -357,12 +394,24 @@ const jwtRuleID = "jwt" // Escapes are read as units from the left, so the second backslash of a `\\` // escape is never taken as the start of another escape. func cutAtTerminatorEscape(secret string) string { + return cutAtEscape(secret, `"nrt`) +} + +// cutAtLineEscape removes the text from the first whitespace escape onwards. A +// password is not cut at a quote escape: its value often starts with one, +// `password: \"...\"`, which is part of it. +func cutAtLineEscape(secret string) string { + return cutAtEscape(secret, "nrt") +} + +// cutAtEscape removes the text from the first escape of one of the given +// characters onwards. +func cutAtEscape(secret, terminators string) string { for i := 0; i+1 < len(secret); i++ { if secret[i] != '\\' { continue } - switch secret[i+1] { - case '"', 'n', 'r', 't': + if strings.IndexByte(terminators, secret[i+1]) >= 0 { return secret[:i] } // Skip the escaped character, so that it is not read as the start of diff --git a/internal/redaction/betterleaks.toml b/internal/redaction/betterleaks.toml new file mode 100644 index 000000000..2e10355b8 --- /dev/null +++ b/internal/redaction/betterleaks.toml @@ -0,0 +1,64 @@ +# Secret scanning rules for redaction: the betterleaks default ruleset, with +# allowlists for values the generic rules match in AI coding sessions but which +# are not credentials. +# +# Every allowlist targets one or more low-precision rules and tests the found +# secret, or the rule's match. None tests the whole line: a string leaf of the +# scanned document is a single line, so a line allowlist would drop every +# finding in a whole tool result. + +[extend] +useDefault = true + +[[allowlists]] +description = "UUIDs are identifiers, not credentials" +targetRules = ["generic-api-key", "generic-password"] +regexes = ['''^[0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{12}$'''] + +[[allowlists]] +description = "Content digests, such as sha256:" +targetRules = ["generic-api-key"] +regexTarget = "match" +regexes = ['''(?i)\bsha(?:1|224|256|384|512):[0-9a-f]{40,128}\b'''] + +[[allowlists]] +description = "Code expressions, test durations and the bare key word, which are not password values" +targetRules = ["generic-password"] +# The scanned text is JSON-encoded, so a match of this rule can run on through +# an escaped line break, or through a \n written in source code, which reaches +# the scanner as \\n. The scanner cuts the secret at the first, but only after +# these regexes ran, so each one also accepts such a tail. +regexes = [ + # An expression such as t.password||n.password in minified JavaScript: + # property accesses, and only those, joined by logical operators. + '''^[A-Za-z_$][\w$]*(?:\.[A-Za-z_$][\w$]*)+(?:(?:\|\||&&|\?\?)[A-Za-z_$][\w$]*(?:\.[A-Za-z_$][\w$]*)+)+(?:\\+[nrt].*)?$''', + # A property access such as e.password or config.db_password, alone or + # followed by logical operators and identifiers, as in e.password||n. + '''^[A-Za-z_$][\w$]*(?:\.[A-Za-z_$][\w$]*)*\.\w*(?i:passw(?:or)?d|pwd)\w*(?:(?:\|\||&&|\?\?)[A-Za-z_$][\w$]*(?:\.[A-Za-z_$][\w$]*)*)*(?:\\+[nrt].*)?$''', + # A redaction placeholder the transcript shows again, in the current or + # the legacy format. + '''^\[(?:CHAINLOOP_TRACE_)?REDACTED(?::[^\]\s]*)?\](?:\\+[nrt].*)?$''', + # Documentation such as (password: `password`). + '''(?i)^(?:password|passwd|pwd)(?:\\+[nrt].*)?$''', + # The duration of a Go test, as in "--- PASS: TestPassword: (0.01s)". + '''^\([0-9]+(?:\.[0-9]+)?(?:ns|us|µs|ms|s|m|h)\)(?:\\+[nrt].*)?$''', +] + +[[allowlists]] +description = "Placeholders in documentation, such as Bearer YOUR_API_TOKEN" +targetRules = ["curl-auth-header"] +regexes = ['''^[A-Z][A-Z0-9]*(?:_[A-Z0-9]+)+$'''] + +[[allowlists]] +description = "A host and port read as the user and password of a URI, such as http://localhost:8000@example.com" +targetRules = ["generic-credential-uri"] +regexTarget = "match" +# The "user" must be localhost, an IPv4 address or a dotted host name, so a +# numeric password next to a plain user name, as in admin:12345@, is still +# redacted. +regexes = ['''(?i)://(?:localhost|[0-9]{1,3}(?:\.[0-9]{1,3}){3}|[a-z0-9-]+(?:\.[a-z0-9-]+)+):[0-9]{1,5}@'''] + +[[allowlists]] +description = "An elided password in documentation, such as http://user:…@example.com" +targetRules = ["generic-credential-uri"] +regexes = ['''^(?:…|\.{3})$'''] diff --git a/internal/redaction/betterleaks_test.go b/internal/redaction/betterleaks_test.go index 6101ed724..62dbca7c7 100644 --- a/internal/redaction/betterleaks_test.go +++ b/internal/redaction/betterleaks_test.go @@ -79,8 +79,9 @@ func TestDefaultScannerDetects(t *testing.T) { wantRule: "github-pat", }, { - // A session transcript is attacker-influenceable text, so neither - // in-band bypass marker may suppress a finding. + // The detector would apply a marker to its whole line, which is a + // whole string leaf of the document. Redactor applies it to the + // line of each occurrence instead, so the scanner reports it. name: "gitleaks:allow must not suppress the finding", text: `"content": "` + awsPair + ` // gitleaks:allow"`, wantRule: "aws-access-token", diff --git a/internal/redaction/parity_test.go b/internal/redaction/parity_test.go new file mode 100644 index 000000000..7c4f03b91 --- /dev/null +++ b/internal/redaction/parity_test.go @@ -0,0 +1,277 @@ +// +// Copyright 2026 The Chainloop Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package redaction + +import ( + "context" + "encoding/json" + "strings" + "testing" + + "github.com/betterleaks/betterleaks/config" + "github.com/betterleaks/betterleaks/detect" + "github.com/betterleaks/betterleaks/sources" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// fakePrivateKey is a private key block the private-key rule matches. Its +// lines are assembled from fragments, like the other synthetic credentials in +// this package. +var fakePrivateKey = "-----BEGIN RSA " + "PRIVATE KEY-----\n" + + strings.Repeat("MIIEvQIBADANBgkqhkiG9w0BAQEFAASC"+"BKcwggSjAgEAAoIBAQC7\n", 3) + + "-----END RSA " + "PRIVATE KEY-----" + +// fileVisible scans each text as its own file, the way betterleaks scans a +// directory, with the ruleset of the session scanner and allow markers +// honoured. It returns, for each of secrets in order, how many of its occurrences the +// scan does not report: the ones it does not detect and the ones an allow +// marker keeps. +func fileVisible(t *testing.T, texts, secrets []string) []int { + t.Helper() + useRE2() + cfg, err := config.ParseTOMLString(rulesConfig, "") + require.NoError(t, err) + d := detect.NewDetectorContext(context.Background(), cfg, detect.ValidationOptions{}) + d.Redact = 0 + d.MaxDecodeDepth = 0 + d.SkipFindingAppend = true + + visible := make([]int, len(secrets)) + for _, text := range texts { + // An occurrence is identified by its line, so that a secret two rules + // report on the same line counts once. + reported := map[string]map[int]struct{}{} + report := func(secret string, line int) { + if reported[secret] == nil { + reported[secret] = map[int]struct{}{} + } + reported[secret][line] = struct{}{} + } + for r := range d.Run(context.Background(), fileSource{text: text}) { + require.NoError(t, r.Err) + report(r.Finding.Secret, r.Finding.StartLine) + for _, set := range r.Finding.ComponentSets { + for _, c := range set.Components { + if c != nil && !c.Optional { + report(c.Secret, c.StartLine) + } + } + } + } + for i, s := range secrets { + visible[i] += strings.Count(text, s) - len(reported[s]) + } + } + return visible +} + +// fileSource is a single file's content. +type fileSource struct{ text string } + +func (s fileSource) Fragments(_ context.Context, yield sources.FragmentsFunc) error { + return yield(sources.Fragment{Raw: s.text}, nil) +} + +// sessionVisible redacts a session document whose string leaves are texts, the +// way an AI coding session is redacted, and returns how many occurrences of +// each of secrets, in order, are left in it. +func sessionVisible(t *testing.T, texts, secrets []string) []int { + t.Helper() + scanner, err := DefaultScanner() + require.NoError(t, err) + + doc, err := json.Marshal(map[string][]string{"content": texts}) + require.NoError(t, err) + + out, _, err := New(scanner, WithAllowMarkers()).Redact(context.Background(), doc) + require.NoError(t, err) + + var got map[string][]string + require.NoError(t, json.Unmarshal(out, &got)) + visible := make([]int, len(secrets)) + for _, leaf := range got["content"] { + for i, s := range secrets { + visible[i] += strings.Count(leaf, s) + } + } + return visible +} + +// TestSessionRedactionMatchesFileScan compares the real detector over files +// with the session redaction of the same text as string leaves. The ruleset is +// the same on both sides, so the comparison is about where the text is: a file +// has real line breaks, a session leaf is a JSON string where a line break is +// the escape \n and a quote is \". +// +// Each case gives, per secret, the occurrences left visible. A case with a +// difference documents why the session does not do what a file scan does. +func TestSessionRedactionMatchesFileScan(t *testing.T) { + testCases := []struct { + name string + // texts are the files of the file scan, and the leaves of the session. + texts []string + secrets []string + // wantFile and wantSession are the occurrences of each of secrets left + // visible, in the same order. + wantFile []int + wantSession []int + // difference explains why wantSession is not wantFile. + difference string + }{ + { + name: "a secret without a marker is reported", + texts: []string{"export GITHUB_TOKEN=" + fakeGitHubPAT + "\n"}, + secrets: []string{fakeGitHubPAT}, + wantFile: []int{0}, + }, + { + name: "a marker on the line of the secret keeps it", + texts: []string{"const pat = '" + fakeGitHubPAT + "' // gitleaks:allow\n"}, + secrets: []string{fakeGitHubPAT}, + wantFile: []int{1}, + }, + { + name: "the betterleaks marker keeps it too", + texts: []string{"token: " + fakeGitHubPAT + " # betterleaks:allow\n"}, + secrets: []string{fakeGitHubPAT}, + wantFile: []int{1}, + }, + { + name: "a marker on the next line does not keep it", + texts: []string{"const pat = '" + fakeGitHubPAT + "'\n// gitleaks:allow\n"}, + secrets: []string{fakeGitHubPAT}, + wantFile: []int{0}, + }, + { + name: "a marker on the previous line does not keep it", + texts: []string{"// gitleaks:allow\nconst pat = '" + fakeGitHubPAT + "'\n"}, + secrets: []string{fakeGitHubPAT}, + wantFile: []int{0}, + }, + { + name: "a marker keeps only the occurrence on its line", + texts: []string{"a = '" + fakeGitHubPAT + "' # gitleaks:allow\nexport GITHUB_TOKEN=" + fakeGitHubPAT + "\n"}, + secrets: []string{fakeGitHubPAT}, + wantFile: []int{1}, + }, + { + name: "a marker in another file or leaf does not keep it", + texts: []string{"export GITHUB_TOKEN=" + fakeGitHubPAT + "\n", "x // gitleaks:allow\n"}, + secrets: []string{fakeGitHubPAT}, + wantFile: []int{0}, + }, + { + name: "an unquoted password", + texts: []string{"password: " + samplePassword + "\n"}, + secrets: []string{samplePassword}, + wantFile: []int{0}, + }, + { + name: "a single-quoted key", + texts: []string{"api_key = '" + sampleAPIKey + "'\n"}, + secrets: []string{sampleAPIKey}, + wantFile: []int{0}, + }, + { + name: "an AWS key pair", + texts: []string{"AWS_ACCESS_KEY_ID=" + fakeAWSKey + "\nAWS_SECRET_ACCESS_KEY=" + fakeAWSSecret + "\n"}, + secrets: []string{fakeAWSKey, fakeAWSSecret}, + wantFile: []int{0, 0}, + }, + { + // The file scan drops the whole pair, then reports the secret key with + // the generic rule. The session keeps only the key id. Either way + // only the id is left. + name: "a marker on the AWS key id line keeps the id only", + texts: []string{"AWS_ACCESS_KEY_ID=" + fakeAWSKey + " # gitleaks:allow\nAWS_SECRET_ACCESS_KEY=" + fakeAWSSecret + "\n"}, + secrets: []string{fakeAWSKey, fakeAWSSecret}, + wantFile: []int{1, 0}, + }, + { + name: "a private key", + texts: []string{fakePrivateKey + "\n"}, + secrets: []string{fakePrivateKey}, + wantFile: []int{0}, + }, + { + name: "a marker on the last line of a private key", + texts: []string{fakePrivateKey + " # gitleaks:allow\n"}, + secrets: []string{fakePrivateKey}, + wantFile: []int{1}, + wantSession: []int{0}, + difference: "betterleaks checks the marker against every line of a match, so one marker " + + "keeps a whole multi-line secret; the session redacts a secret that spans lines", + }, + { + name: "a double-quoted key", + texts: []string{"const api_key = \"" + sampleAPIKey + "\"\n"}, + secrets: []string{sampleAPIKey}, + wantFile: []int{0}, + wantSession: []int{1}, + difference: "known gap: the session scans JSON, where the quote is \\\" and the generic rules do not allow a backslash before the value", + }, + { + name: "a double-quoted password", + texts: []string{"password = \"" + samplePassword + "\"\n"}, + secrets: []string{samplePassword}, + wantFile: []int{0}, + wantSession: []int{1}, + difference: "known gap: the session scans JSON, where the quote is \\\"", + }, + { + name: "a value on the line after its key", + texts: []string{"api_key:\n " + sampleAPIKey + "\n"}, + secrets: []string{sampleAPIKey}, + wantFile: []int{0}, + wantSession: []int{1}, + difference: "known gap: in JSON the line break is the escape \\n, which the rules do not read as whitespace", + }, + { + name: "an import on another line of the text", + texts: []string{"import { a } from 'b'\napi_key = " + sampleAPIKey + "\n"}, + secrets: []string{sampleAPIKey}, + wantFile: []int{0}, + wantSession: []int{1}, + difference: "known gap: the generic-api-key rule drops a finding whose line is an import, " + + "and a whole session leaf is one line", + }, + { + name: "an AWS key id at the start of a line", + texts: []string{"x\n" + fakeAWSKey + "\naws_secret = " + fakeAWSSecret + "\n"}, + secrets: []string{fakeAWSKey, fakeAWSSecret}, + wantFile: []int{0, 0}, + wantSession: []int{1, 0}, + difference: "known gap: after the escape \\n the id follows the letter n, so the rule's word boundary does not match", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + wantSession := tc.wantSession + if tc.difference == "" { + wantSession = tc.wantFile + } else { + require.NotEqual(t, tc.wantFile, wantSession, "a case with a difference expects one") + } + + // Compared as counts by position, so that a failure does not print a + // secret. + assert.Equal(t, tc.wantFile, fileVisible(t, tc.texts, tc.secrets), "file scan") + assert.Equal(t, wantSession, sessionVisible(t, tc.texts, tc.secrets), "session") + }) + } +} diff --git a/internal/redaction/precision_test.go b/internal/redaction/precision_test.go new file mode 100644 index 000000000..ef0d2b292 --- /dev/null +++ b/internal/redaction/precision_test.go @@ -0,0 +1,273 @@ +// +// Copyright 2026 The Chainloop Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package redaction + +import ( + "context" + "strings" + "testing" + + betterleaksconfig "github.com/betterleaks/betterleaks/config" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// Values shaped like the ones the generic rules matched in recorded AI coding +// sessions, and real secrets of the same shape. Assembled from fragments, like +// the other synthetic credentials in this package. +const ( + sampleUUID = "3bf79921-3c03-81b6-" + "afff-cb246849866f" + sampleDigest = "9f8e7d6c5b4a39281706f5e4d3c2b1a0" + "f9e8d7c69f8e7d6c5b4a39281706f5e4" + // samplePassword is a password with enough entropy to look real. + samplePassword = "Xk9mQ2vL" + "p7wRt4zN" + // sampleAPIKey is a random-looking key the generic-api-key rule matches. + sampleAPIKey = "q8Zr2LmX7v" + "Pk4TnW9sYb" + sampleEmail = "sarah@chainloop.local" + + ruleGenericAPIKey = "generic-api-key" + ruleGitHubPAT = "github-pat" + ruleCredentialURI = "generic-credential-uri" +) + +// defaultRulesScanner returns a scanner with the betterleaks default ruleset and +// none of the allowlists of rulesConfig. It shows that a value the allowlists +// keep is one the default ruleset matches, so that a test of an allowlist +// cannot pass for the wrong reason. +func defaultRulesScanner(t *testing.T) *betterleaksScanner { + t.Helper() + s, err := newScannerWithRules(betterleaksconfig.DefaultConfig) + require.NoError(t, err) + return s +} + +// TestRulesConfigKeepsNonSecrets covers the values that the generic rules of +// the default ruleset replace in AI coding sessions but which are not secrets. +// Each case is a line of a tool result or a message, followed by a line break, +// which is the shape it has in a transcript. +func TestRulesConfigKeepsNonSecrets(t *testing.T) { + scanner, err := DefaultScanner() + require.NoError(t, err) + unfiltered := defaultRulesScanner(t) + + testCases := []struct { + name string + text string + // wantDefaultRule is the rule of the default ruleset that replaces part + // of the text. + wantDefaultRule string + }{ + { + name: "uuid as the value of a key", + text: "workflow key: " + sampleUUID + "\n", + wantDefaultRule: ruleGenericAPIKey, + }, + { + name: "uuid as an idempotency key", + text: "idempotency_key=" + sampleUUID + "\n", + wantDefaultRule: ruleGenericAPIKey, + }, + { + name: "sha256 evidence digest in cli output", + text: "attestation pushed, access sha256:" + sampleDigest + "\n", + wantDefaultRule: ruleGenericAPIKey, + }, + { + name: "minified javascript in a tool result", + text: "host: localhost\nuser: admin\n!function(e){var t={};e.password=t.password||n.password,e.user=u}\n", + wantDefaultRule: genericPasswordRuleID, + }, + { + name: "documentation of development credentials", + text: "- **Dex OIDC**: localhost:5556, users " + sampleEmail + " / john@chainloop.local (password: `password`)\n- **Database**: host: localhost, user: postgres\n", + wantDefaultRule: genericPasswordRuleID, + }, + { + name: "key word as the value, followed by another line", + text: "host: localhost\nuser: postgres\npassword: password\nport: 5432\n", + wantDefaultRule: genericPasswordRuleID, + }, + { + name: "key word as the value in a go string literal", + text: `text: "host: localhost\nuser: postgres\npassword: password\nport: 5432\n",` + "\n", + wantDefaultRule: genericPasswordRuleID, + }, + { + name: "minified javascript with a plain identifier operand", + text: "host: localhost\nuser: admin\n!function(e){e.password=e.password||n,e.user=u}\n", + wantDefaultRule: genericPasswordRuleID, + }, + { + name: "legacy placeholder shown again", + text: "host: localhost\nuser: admin\npassword: [REDACTED:generic-password]\nport: 5432\n", + wantDefaultRule: genericPasswordRuleID, + }, + { + name: "placeholder shown again in a go string literal", + text: `text: "host: localhost\nuser: admin\npassword: [CHAINLOOP_TRACE_REDACTED:generic-password]\nport: 5432\n",` + "\n", + wantDefaultRule: genericPasswordRuleID, + }, + { + name: "elided password of a uri in documentation", + text: "the issuer is `http://localhost:…@chainloop.local`\n", + wantDefaultRule: ruleCredentialURI, + }, + { + name: "duration of a go test", + text: "=== RUN TestRedactPassword\n--- PASS: TestRedactPassword: (0.01s)\nPASS\n", + wantDefaultRule: genericPasswordRuleID, + }, + { + name: "placeholder token in a curl example of a document", + text: "curl -H 'Authorization: Bearer REPLACE_ME' https://api.example.com\n", + wantDefaultRule: "curl-auth-header", + }, + { + name: "port read as the password of a uri", + text: "issuer: http://localhost:8000@chainloop.local\n", + wantDefaultRule: ruleCredentialURI, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + _, report, err := New(unfiltered).RedactText(context.Background(), tc.text) + require.NoError(t, err) + require.Contains(t, report.RuleIDs(), tc.wantDefaultRule, "the default ruleset must replace part of this text, or this case tests nothing") + + got, report, err := New(scanner).RedactText(context.Background(), tc.text) + require.NoError(t, err) + assert.Equal(t, tc.text, got) + assert.False(t, report.Changed(), "replaced by %v", report.RuleIDs()) + }) + } +} + +// TestRulesConfigStillRedactsSecrets is the counterpart of +// TestRulesConfigKeepsNonSecrets: a real secret of each shape is still +// replaced. +func TestRulesConfigStillRedactsSecrets(t *testing.T) { + scanner, err := DefaultScanner() + require.NoError(t, err) + + testCases := []struct { + name string + text string + secret string + wantRule string + // keep is text next to the secret that must survive. + keep string + }{ + { + name: "api key that is not a uuid", + text: "workflow key: " + sampleAPIKey + "\n", + secret: sampleAPIKey, + wantRule: ruleGenericAPIKey, + }, + { + name: "hex key without a digest prefix", + text: "access_token: " + sampleDigest + "\n", + secret: sampleDigest, + wantRule: ruleGenericAPIKey, + }, + { + name: "password next to an email username", + text: "host: db.internal\nuser: " + sampleEmail + "\npassword: " + samplePassword + "\n", + secret: samplePassword, + wantRule: genericPasswordRuleID, + keep: sampleEmail, + }, + { + name: "bearer token in a curl command", + text: "curl -H 'Authorization: Bearer " + sampleAPIKey + "' https://api.example.com\n", + secret: sampleAPIKey, + wantRule: "curl-auth-header", + }, + { + name: "password with a logical operator in it", + text: "host: db.internal\nuser: " + sampleEmail + "\npassword: Xk9m||Q2vL" + "p7wRt4z\n", + secret: "Xk9m||Q2vL" + "p7wRt4z", + wantRule: genericPasswordRuleID, + }, + { + name: "numeric password in a connection uri", + text: "dsn: postgres://admin:" + "12345@db.internal:5432/app\n", + secret: "admin:" + "12345@", + wantRule: ruleCredentialURI, + }, + { + name: "password in a connection uri", + text: "dsn: postgres://admin:" + samplePassword + "@db.internal:5432/app\n", + secret: samplePassword, + wantRule: ruleCredentialURI, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + got, report, err := New(scanner).RedactText(context.Background(), tc.text) + require.NoError(t, err) + assert.NotContains(t, got, tc.secret) + assert.Contains(t, report.RuleIDs(), tc.wantRule) + if tc.keep != "" { + assert.Contains(t, got, tc.keep) + } + }) + } +} + +// TestRedactKeepsEchoedPlaceholders covers transcripts that show placeholders +// of an earlier redaction: test assertions, a pull request check table, or a +// policy result. They are left as they are, and not counted as replacements. +func TestRedactKeepsEchoedPlaceholders(t *testing.T) { + scanner, err := DefaultScanner() + require.NoError(t, err) + + texts := []string{ + "wantURI := \"https://tracker.example.com/issue/1?token=[CHAINLOOP_TRACE_REDACTED:github-pat]\"\n", + "assert.Contains(t, string(content), \"[CHAINLOOP_TRACE_REDACTED:aws-access-token]\")\n", + "| ai-config-no-secrets | fail | Secret (generic-password) detected [CHAINLOOP_TRACE_REDACTED:generic-password] |\n", + "password: [CHAINLOOP_TRACE_REDACTED:generic-password]\napi_key: [CHAINLOOP_TRACE_REDACTED:generic-api-key]\n", + // The log line the CLI prints after a redaction. + "10:04AM INF redacted secrets from the AI coding session before upload count=3 rules=[\"generic-password\",\"generic-username\"]\n", + } + + for _, text := range texts { + t.Run(strings.SplitN(text, " ", 2)[0], func(t *testing.T) { + got, report, err := New(scanner).RedactText(context.Background(), text) + require.NoError(t, err) + assert.Equal(t, text, got) + assert.False(t, report.Changed(), "replaced by %v", report.RuleIDs()) + }) + } +} + +// TestRedactAllowMarkerWithDefaultRules checks an allow marker against the real +// ruleset: a test fixture whose line carries one is kept, the same value on a +// line without one is not. +func TestRedactAllowMarkerWithDefaultRules(t *testing.T) { + scanner, err := DefaultScanner() + require.NoError(t, err) + + fixture := "\tconst pat = \"" + fakeGitHubPAT + "\" // gitleaks:allow\n" + text := "119" + fixture + "120\twantURI := \"https://tracker.example.com/issue/1?token=\" + pat\nexport GITHUB_TOKEN=" + fakeGitHubPAT + "\n" + + got, report, err := New(scanner, WithAllowMarkers()).RedactText(context.Background(), text) + require.NoError(t, err) + assert.Contains(t, got, fixture, "the line with the marker is kept") + assert.Contains(t, got, "export GITHUB_TOKEN=[CHAINLOOP_TRACE_REDACTED:github-pat]\n") + assert.Equal(t, map[string]int{ruleGitHubPAT: 1}, report.Allowed) + assert.Equal(t, map[string]int{ruleGitHubPAT: 1}, report.ByRule) +} diff --git a/internal/redaction/redaction.go b/internal/redaction/redaction.go index 382a35687..cd0687f9f 100644 --- a/internal/redaction/redaction.go +++ b/internal/redaction/redaction.go @@ -32,6 +32,7 @@ import ( "fmt" "io" "regexp" + "slices" "sort" "strconv" "strings" @@ -106,6 +107,9 @@ type Report struct { // lies in a path the filter protects, or when it spans the JSON punctuation // between two adjacent leaves. Unlocated map[string]int + // Allowed counts, per rule id, occurrences left in place because their line + // carries an allow marker. Empty unless the Redactor honours markers. + Allowed map[string]int // Passes is the number of detection passes performed. Passes int } @@ -136,6 +140,7 @@ type Redactor struct { maxPasses int placeholder func(ruleID string) string isPlaceholder func(string) bool + allowMarkers bool } // Option customises a Redactor. @@ -159,6 +164,19 @@ func WithOpaque(f OpaqueFilter) Option { } } +// WithAllowMarkers leaves in place a secret whose line carries an inline +// "gitleaks:allow" or "betterleaks:allow" marker, as a repository scan with +// either tool would. The default redacts it. +// +// A line is a line of the decoded string, such as one line of a file in a tool +// result, so the marker only covers the secrets next to it. An occurrence that +// spans more than one line is always redacted. +func WithAllowMarkers() Option { + return func(r *Redactor) { + r.allowMarkers = true + } +} + // WithMaxPasses bounds the convergence loop. func WithMaxPasses(n int) Option { return func(r *Redactor) { @@ -273,7 +291,7 @@ func (r *Redactor) Redact(ctx context.Context, doc []byte) ([]byte, *Report, err detachOpaque(root, "", r.opaque, &detached) } - report := &Report{ByRule: map[string]int{}, Unlocated: map[string]int{}} + report := &Report{ByRule: map[string]int{}, Unlocated: map[string]int{}, Allowed: map[string]int{}} // Secrets no eligible leaf contains, so that the loop stops chasing them. skip := make(map[string]struct{}) converged := false @@ -308,23 +326,40 @@ func (r *Redactor) Redact(ctx context.Context, doc []byte) ([]byte, *Report, err } w := &rewriter{ - pathFilter: r.pathFilter, - placeholder: r.placeholder, - secrets: pending, - byRule: map[string]int{}, - located: map[string]struct{}{}, + pathFilter: r.pathFilter, + placeholder: r.placeholder, + allowMarkers: r.allowMarkers, + secrets: pending, + byRule: map[string]int{}, + allowed: map[string]int{}, + located: map[string]struct{}{}, + kept: map[string]struct{}{}, + protected: map[string]struct{}{}, } w.rewriteMap(root, "") for _, s := range pending { - if _, ok := w.located[s.secret]; !ok { + _, located := w.located[s.secret] + _, kept := w.kept[s.secret] + _, protected := w.protected[s.secret] + switch { + // A copy in a protected leaf stays in the document whatever happened + // to the others, so it is reported even when another was kept. + case protected || (!located && !kept): skip[s.secret] = struct{}{} report.Unlocated[s.ruleID]++ + case kept: + // It is only left on allowed lines after this pass, so a further + // pass would only find it again. + skip[s.secret] = struct{}{} } } for rule, n := range w.byRule { report.ByRule[rule] += n } + for rule, n := range w.allowed { + report.Allowed[rule] += n + } report.Replacements += w.count if w.count == 0 { @@ -449,12 +484,20 @@ func trimPartialEscape(secret string) string { // rewriter walks a decoded JSON value tree replacing secrets in eligible string // leaves. type rewriter struct { - pathFilter PathFilter - placeholder func(string) string - secrets []secretRule - byRule map[string]int - located map[string]struct{} - count int + pathFilter PathFilter + placeholder func(string) string + allowMarkers bool + secrets []secretRule + byRule map[string]int + // allowed counts, per rule id, the occurrences kept on allowed lines. + allowed map[string]int + located map[string]struct{} + // kept holds the secrets with an occurrence kept on an allowed line. + kept map[string]struct{} + // protected holds the secrets with an occurrence in a leaf the path filter + // protects, which therefore stays in the document. + protected map[string]struct{} + count int } func (w *rewriter) rewriteMap(m map[string]any, path string) { @@ -488,8 +531,9 @@ func (w *rewriter) rewrite(node any, path string) any { // characters `\n`. // // The path filter is consulted only once the leaf is known to hold a secret, and -// nothing is recorded for a leaf it protects. Nearly every leaf holds none, and -// the filter runs for each leaf on every pass otherwise. +// a leaf it protects is left as it is, with its secrets recorded as protected. +// Nearly every leaf holds none, and the filter runs for each leaf on every pass +// otherwise. func (w *rewriter) redactLeaf(s, path string) string { body, err := encodeStringBody(s) if err != nil { @@ -498,15 +542,28 @@ func (w *rewriter) redactLeaf(s, path string) string { var ( n int + eligible bool lastRule string + // Replacing a secret never adds or removes a marker, so this holds for + // the whole leaf. + marked = w.allowMarkers && hasAllowMarker(body) ) - for _, sr := range w.secrets { + for i, sr := range w.secrets { c := strings.Count(body, sr.secret) if c == 0 { continue } - if n == 0 && !w.pathFilter(path) { - return s + if !eligible { + if !w.pathFilter(path) { + // The secrets before this one are not in the leaf. + for _, rest := range w.secrets[i:] { + if strings.Contains(body, rest.secret) { + w.protected[rest.secret] = struct{}{} + } + } + return s + } + eligible = true } // The placeholder goes into the escaped form of the string, so it is // escaped too. A raw quote or backslash in it would otherwise break @@ -515,9 +572,22 @@ func (w *rewriter) redactLeaf(s, path string) string { if err != nil { return w.placeholder(sr.ruleID) } - body = strings.ReplaceAll(body, sr.secret, placeholder) - n += c - w.byRule[sr.ruleID] += c + + replaced, kept := c, 0 + if marked { + body, replaced, kept = replaceOutsideAllowedLines(body, sr.secret, placeholder) + } else { + body = strings.ReplaceAll(body, sr.secret, placeholder) + } + if kept > 0 { + w.allowed[sr.ruleID] += kept + w.kept[sr.secret] = struct{}{} + } + if replaced == 0 { + continue + } + n += replaced + w.byRule[sr.ruleID] += replaced w.located[sr.secret] = struct{}{} lastRule = sr.ruleID } @@ -535,6 +605,73 @@ func (w *rewriter) redactLeaf(s, path string) string { return out } +// allowSignatures are the inline markers that exempt a line from secret +// scanning in betterleaks and gitleaks. +var allowSignatures = []string{"betterleaks:allow", "gitleaks:allow"} + +// hasAllowMarker reports whether s carries one of allowSignatures. +func hasAllowMarker(s string) bool { + return slices.ContainsFunc(allowSignatures, func(sig string) bool { return strings.Contains(s, sig) }) +} + +// replaceOutsideAllowedLines substitutes placeholder for every occurrence of +// secret in body, the JSON-encoded form of a leaf, except the occurrences that +// lie within a line carrying an allow marker. It returns the new body and the +// number of occurrences replaced and kept. +func replaceOutsideAllowedLines(body, secret, placeholder string) (string, int, int) { + allowed := allowedLines(body) + + var ( + b strings.Builder + replaced, kept int + pos int + ) + for { + i := strings.Index(body[pos:], secret) + if i < 0 { + break + } + start, end := pos+i, pos+i+len(secret) + b.WriteString(body[pos:start]) + if within(allowed, start, end) { + b.WriteString(secret) + kept++ + } else { + b.WriteString(placeholder) + replaced++ + } + pos = end + } + b.WriteString(body[pos:]) + return b.String(), replaced, kept +} + +// allowedLines returns the lines of body, the JSON-encoded form of a string, +// that carry an allow marker. Lines are those of the decoded string: they end at +// an escaped line feed or carriage return. +func allowedLines(body string) []span { + var out []span + for start := 0; start <= len(body); { + line := cutAtEscape(body[start:], "nr") + if hasAllowMarker(line) { + out = append(out, span{start: start, end: start + len(line)}) + } + // Past the line and the two characters of its line break. + start += len(line) + 2 + } + return out +} + +// within reports whether [start, end) lies inside one of spans. +func within(spans []span, start, end int) bool { + for _, s := range spans { + if s.start <= start && end <= s.end { + return true + } + } + return false +} + // decodeObject parses doc into a value tree, keeping numbers in their original // textual form so re-encoding does not reformat them. func decodeObject(doc []byte) (map[string]any, error) { diff --git a/internal/redaction/redaction_test.go b/internal/redaction/redaction_test.go index abe76fc6b..8cea6ef43 100644 --- a/internal/redaction/redaction_test.go +++ b/internal/redaction/redaction_test.go @@ -470,6 +470,144 @@ func TestRedactText(t *testing.T) { } } +// TestRedactAllowMarkers checks that an allow marker keeps only the secrets on +// its own line, the line of the decoded string, and nothing else. +func TestRedactAllowMarkers(t *testing.T) { + const ( + secret = "s3cr3t-value-0123456789" + placeholder = "[CHAINLOOP_TRACE_REDACTED:test-token]" + ) + + testCases := []struct { + name string + // leaves are the string leaves of the document, in keys a, b, ... + leaves []string + opts []Option + want []string + wantByRule map[string]int + wantAllowed map[string]int + }{ + { + name: "marker on the line of the secret keeps it", + leaves: []string{"token := \"" + secret + "\" // gitleaks:allow"}, + opts: []Option{WithAllowMarkers()}, + want: []string{"token := \"" + secret + "\" // gitleaks:allow"}, + wantByRule: map[string]int{}, + wantAllowed: map[string]int{"test-token": 1}, + }, + { + name: "betterleaks marker keeps it too", + leaves: []string{"token: " + secret + " # betterleaks:allow"}, + opts: []Option{WithAllowMarkers()}, + want: []string{"token: " + secret + " # betterleaks:allow"}, + wantByRule: map[string]int{}, + wantAllowed: map[string]int{"test-token": 1}, + }, + { + name: "marker on another line of the same leaf does not", + leaves: []string{"1\ttoken := \"" + secret + "\"\n2\tother := x // gitleaks:allow"}, + opts: []Option{WithAllowMarkers()}, + want: []string{"1\ttoken := \"" + placeholder + "\"\n2\tother := x // gitleaks:allow"}, + wantByRule: map[string]int{"test-token": 1}, + wantAllowed: map[string]int{}, + }, + { + name: "carriage returns end a line too", + leaves: []string{"token := " + secret + "\r\nx // gitleaks:allow"}, + opts: []Option{WithAllowMarkers()}, + want: []string{"token := " + placeholder + "\r\nx // gitleaks:allow"}, + wantByRule: map[string]int{"test-token": 1}, + wantAllowed: map[string]int{}, + }, + { + name: "the same secret without a marker elsewhere in the leaf is replaced", + leaves: []string{"a := \"" + secret + "\" // gitleaks:allow\nexport TOKEN=" + secret}, + opts: []Option{WithAllowMarkers()}, + want: []string{"a := \"" + secret + "\" // gitleaks:allow\nexport TOKEN=" + placeholder}, + wantByRule: map[string]int{"test-token": 1}, + wantAllowed: map[string]int{"test-token": 1}, + }, + { + name: "marker in another leaf does not", + leaves: []string{"export TOKEN=" + secret, "x // gitleaks:allow"}, + opts: []Option{WithAllowMarkers()}, + want: []string{"export TOKEN=" + placeholder, "x // gitleaks:allow"}, + wantByRule: map[string]int{"test-token": 1}, + wantAllowed: map[string]int{}, + }, + { + name: "an escaped backslash before n is not a line break", + leaves: []string{`path C:\new ` + secret + ` // gitleaks:allow`}, + opts: []Option{WithAllowMarkers()}, + want: []string{`path C:\new ` + secret + ` // gitleaks:allow`}, + wantByRule: map[string]int{}, + wantAllowed: map[string]int{"test-token": 1}, + }, + { + name: "markers are ignored unless enabled", + leaves: []string{"token := \"" + secret + "\" // gitleaks:allow"}, + want: []string{"token := \"" + placeholder + "\" // gitleaks:allow"}, + wantByRule: map[string]int{"test-token": 1}, + wantAllowed: map[string]int{}, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + scanner := &fakeScanner{findings: []Finding{{RuleID: "test-token", Secret: secret}}, requirePresent: true} + doc := map[string]string{} + for i, leaf := range tc.leaves { + doc[string(rune('a'+i))] = leaf + } + in, err := json.Marshal(doc) + require.NoError(t, err) + + out, report, err := New(scanner, tc.opts...).Redact(context.Background(), in) + require.NoError(t, err) + + var got map[string]string + require.NoError(t, json.Unmarshal(out, &got)) + for i, want := range tc.want { + assert.Equal(t, want, got[string(rune('a'+i))]) + } + assert.Equal(t, tc.wantByRule, report.ByRule) + assert.Equal(t, tc.wantAllowed, report.Allowed) + assert.Empty(t, report.Unlocated, "a kept secret is not an unlocated one") + }) + } +} + +// TestRedactAllowMarkerDoesNotHideProtectedCopy checks that a copy of a secret +// in a protected leaf is still reported when another copy was kept by an allow +// marker. +func TestRedactAllowMarkerDoesNotHideProtectedCopy(t *testing.T) { + scanner := &fakeScanner{findings: []Finding{{RuleID: "r1", Secret: "SEC-0123456789"}}, requirePresent: true} + doc := `{"keepme":"SEC-0123456789","other":"x SEC-0123456789 // gitleaks:allow"}` + + _, report, err := New(scanner, WithAllowMarkers(), WithPathFilter(func(p string) bool { return p != "/keepme" })). + Redact(context.Background(), []byte(doc)) + + require.NoError(t, err) + assert.Equal(t, map[string]int{"r1": 1}, report.Allowed) + assert.Equal(t, map[string]int{"r1": 1}, report.Unlocated) +} + +// TestRedactTextAllowMarker checks a kept secret next to a replaced one in plain +// text, the shape of a captured spec. +func TestRedactTextAllowMarker(t *testing.T) { + scanner := &fakeScanner{findings: []Finding{ + {RuleID: "kept", Secret: "KEPT-0123456789"}, + {RuleID: "other", Secret: "OTHER-0123456789"}, + }, requirePresent: true} + + out, report, err := New(scanner, WithAllowMarkers()). + RedactText(context.Background(), "a KEPT-0123456789 // gitleaks:allow\nb OTHER-0123456789") + + require.NoError(t, err) + assert.Equal(t, "a KEPT-0123456789 // gitleaks:allow\nb [CHAINLOOP_TRACE_REDACTED:other]", out) + assert.Equal(t, 1, report.Replacements) +} + // TestRedactPlaceholderNeedingEscape checks that a placeholder is inserted in // its JSON-escaped form. A raw quote or backslash in it would break the string // the leaf is rebuilt from, and the whole leaf would then be lost to the diff --git a/pkg/attestation/crafter/api/attestation/v1/crafting_state.go b/pkg/attestation/crafter/api/attestation/v1/crafting_state.go index cb623d993..07ea4766e 100644 --- a/pkg/attestation/crafter/api/attestation/v1/crafting_state.go +++ b/pkg/attestation/crafter/api/attestation/v1/crafting_state.go @@ -73,6 +73,11 @@ var ( // AnnotationMaterialRedactionRules lists the detection rules that matched, // so a policy can act on the kind of credential that was present. AnnotationMaterialRedactionRules = CreateAnnotation("material.redaction.rules") + // AnnotationMaterialRedactionRuleset is the version of the detection rules + // the scan ran with, so a policy can tell placeholders from an older, less + // precise ruleset apart from current ones. Absent on materials redacted + // before it was introduced, which used version 1. + AnnotationMaterialRedactionRuleset = CreateAnnotation("material.redaction.ruleset") // AnnotationMaterialRedactionSkipped marks a material uploaded without // redaction because the operator explicitly asked for it. Recorded so the // bypass is visible to policies rather than silent. diff --git a/pkg/attestation/crafter/crafter_test.go b/pkg/attestation/crafter/crafter_test.go index 842f1bf39..4ab4b0700 100644 --- a/pkg/attestation/crafter/crafter_test.go +++ b/pkg/attestation/crafter/crafter_test.go @@ -27,6 +27,7 @@ import ( "os" "path/filepath" "slices" + "strconv" "strings" "testing" "time" @@ -35,6 +36,7 @@ import ( schemaapi "github.com/chainloop-dev/chainloop/app/controlplane/api/workflowcontract/v1" "github.com/chainloop-dev/chainloop/app/controlplane/pkg/unmarshal" + "github.com/chainloop-dev/chainloop/internal/redaction" "github.com/chainloop-dev/chainloop/pkg/attestation/crafter" v1 "github.com/chainloop-dev/chainloop/pkg/attestation/crafter/api/attestation/v1" "github.com/chainloop-dev/chainloop/pkg/attestation/crafter/materials" @@ -838,6 +840,7 @@ func (s *crafterSuite) TestAddMaterialCleanSessionIsMarkedScanned() { assert.Equal(s.T(), v1.AnnotationValueTrue, m.Annotations[v1.AnnotationMaterialRedacted]) assert.Equal(s.T(), "0", m.Annotations[v1.AnnotationMaterialRedactionCount]) + assert.Equal(s.T(), strconv.Itoa(redaction.RulesetVersion), m.Annotations[v1.AnnotationMaterialRedactionRuleset]) assert.NotContains(s.T(), m.Annotations, v1.AnnotationMaterialRedactionSkipped) // Nothing was replaced, so the digest is still that of the source file. diff --git a/pkg/attestation/crafter/materials/aicodingsession/redact.go b/pkg/attestation/crafter/materials/aicodingsession/redact.go index 0e0d20df8..8d07c96a7 100644 --- a/pkg/attestation/crafter/materials/aicodingsession/redact.go +++ b/pkg/attestation/crafter/materials/aicodingsession/redact.go @@ -76,13 +76,19 @@ var protectedPaths = []string{ // its original digest. The result is guaranteed to still validate against the AI // coding session schema; if it does not, redaction fails rather than uploading // either an invalid document or an unredacted one. +// +// A secret on a line with an inline gitleaks:allow or betterleaks:allow marker +// is kept, as a repository scan would keep it: an agent reading a test fixture +// marked that way is the common case. The marker is text in the transcript, so +// the agent could also write one next to a real secret; it covers only its +// own line, which is the same trust a repository scan gives it. func Redact(ctx context.Context, evidence []byte) ([]byte, *redaction.Report, error) { scanner, err := redaction.DefaultScanner() if err != nil { return nil, nil, fmt.Errorf("initialising the secret scanner: %w", err) } - redacted, report, err := redaction.New(scanner, redaction.WithPathFilter(eligible), redaction.WithOpaque(isMedia)).Redact(ctx, evidence) + redacted, report, err := redaction.New(scanner, redaction.WithPathFilter(eligible), redaction.WithOpaque(isMedia), redaction.WithAllowMarkers()).Redact(ctx, evidence) if err != nil { return nil, nil, err } @@ -99,8 +105,8 @@ func Redact(ctx context.Context, evidence []byte) ([]byte, *redaction.Report, er // RedactSpecText removes detected secrets from the text of one captured spec. // // A spec is stored as its own material rather than inside the session -// document, so it does not pass through Redact. It gets the same scanner and -// the same placeholders here, so a credential pasted into a ticket body is +// document, so it does not pass through Redact. It gets the same scanner, the +// same allow markers and the same placeholders here, so a credential pasted into a ticket body is // removed exactly as it would be from the transcript. Text with no detected // secrets is returned unchanged. func RedactSpecText(ctx context.Context, text string) (string, *redaction.Report, error) { @@ -109,7 +115,7 @@ func RedactSpecText(ctx context.Context, text string) (string, *redaction.Report return "", nil, fmt.Errorf("initialising the secret scanner: %w", err) } - return redaction.New(scanner).RedactText(ctx, text) + return redaction.New(scanner, redaction.WithAllowMarkers()).RedactText(ctx, text) } // eligible reports whether the string leaf at path may be rewritten. diff --git a/pkg/attestation/crafter/materials/aicodingsession/testdata/session-fp-shaped.json b/pkg/attestation/crafter/materials/aicodingsession/testdata/session-fp-shaped.json index 2f780f9a3..bc47ac637 100644 --- a/pkg/attestation/crafter/materials/aicodingsession/testdata/session-fp-shaped.json +++ b/pkg/attestation/crafter/materials/aicodingsession/testdata/session-fp-shaped.json @@ -79,6 +79,36 @@ "type": "assistant", "message": {"role": "assistant", "content": "Reading pkg/attestation/crafter/materials/aicodingsession/aicodingsession.go and running `go test ./... -run TestRedact`. Base64 of hello: aGVsbG8=. Escaped: & \"quotes\" and a tab\there."}, "timestamp": "2026-03-25T15:13:40.100Z" + }, + { + "type": "user", + "message": {"role": "user", "content": "Pushed the attestation.\n digest: sha256:9f8e7d6c5b4a39281706f5e4d3c2b1a0f9e8d7c69f8e7d6c5b4a39281706f5e4\n access sha256:9f8e7d6c5b4a39281706f5e4d3c2b1a0f9e8d7c69f8e7d6c5b4a39281706f5e4\nworkflow key: 3bf79921-3c03-81b6-afff-cb246849866f\nidempotency_key=3bf79921-3c03-81b6-afff-cb246849866f\n"}, + "timestamp": "2026-03-25T15:14:02.000Z" + }, + { + "type": "user", + "message": {"role": "user", "content": "- **Dex OIDC**: localhost:5556, users sarah@chainloop.local / john@chainloop.local (password: `password`)\n- **Database**: host: localhost, user: postgres\n"}, + "timestamp": "2026-03-25T15:14:03.000Z" + }, + { + "type": "user", + "message": {"role": "user", "content": "host: localhost\nuser: admin\n!function(e){var t={};e.password=t.password||n.password,e.user=u}\n"}, + "timestamp": "2026-03-25T15:14:04.000Z" + }, + { + "type": "assistant", + "message": {"role": "assistant", "content": "curl -H 'Authorization: Bearer REPLACE_ME' https://api.example.com\nissuer: http://localhost:8000@chainloop.local\n"}, + "timestamp": "2026-03-25T15:14:05.000Z" + }, + { + "type": "user", + "message": {"role": "user", "content": "| ai-config-no-secrets | fail | Secret (generic-password) detected [CHAINLOOP_TRACE_REDACTED:generic-password] |\npassword: [CHAINLOOP_TRACE_REDACTED:generic-password]\napi_key: [CHAINLOOP_TRACE_REDACTED:generic-api-key]\n"}, + "timestamp": "2026-03-25T15:14:06.000Z" + }, + { + "type": "user", + "message": {"role": "user", "content": "119\tconst pat = \"__GITHUB_PAT__\" // gitleaks:allow\n120\twantURI := \"https://tracker.example.com/issue/1?token=\" + pat\n"}, + "timestamp": "2026-03-25T15:14:07.000Z" } ] }, diff --git a/pkg/attestation/crafter/materials/chainloop_ai_coding_session.go b/pkg/attestation/crafter/materials/chainloop_ai_coding_session.go index 891592063..852e8b986 100644 --- a/pkg/attestation/crafter/materials/chainloop_ai_coding_session.go +++ b/pkg/attestation/crafter/materials/chainloop_ai_coding_session.go @@ -200,6 +200,7 @@ func (c *ChainloopAICodingSessionCrafter) annotateRedaction(material *api.Attest material.Annotations[api.AnnotationMaterialRedacted] = api.AnnotationValueTrue material.Annotations[api.AnnotationMaterialRedactionCount] = strconv.Itoa(report.Replacements) + material.Annotations[api.AnnotationMaterialRedactionRuleset] = strconv.Itoa(redaction.RulesetVersion) if !report.Changed() { return diff --git a/pkg/attestation/crafter/materials/chainloop_ai_coding_session_redaction_test.go b/pkg/attestation/crafter/materials/chainloop_ai_coding_session_redaction_test.go index 3837cb16f..331d02150 100644 --- a/pkg/attestation/crafter/materials/chainloop_ai_coding_session_redaction_test.go +++ b/pkg/attestation/crafter/materials/chainloop_ai_coding_session_redaction_test.go @@ -27,6 +27,7 @@ import ( "testing" schemaapi "github.com/chainloop-dev/chainloop/app/controlplane/api/workflowcontract/v1" + "github.com/chainloop-dev/chainloop/internal/redaction" api "github.com/chainloop-dev/chainloop/pkg/attestation/crafter/api/attestation/v1" "github.com/chainloop-dev/chainloop/pkg/casclient" mUploader "github.com/chainloop-dev/chainloop/pkg/casclient/mocks" @@ -285,6 +286,7 @@ func TestChainloopAICodingSessionCrafterRedaction(t *testing.T) { if tc.skipRedaction { assert.Equal(t, "true", got.Annotations[api.AnnotationMaterialRedactionSkipped]) assert.NotContains(t, got.Annotations, api.AnnotationMaterialRedacted) + assert.NotContains(t, got.Annotations, api.AnnotationMaterialRedactionRuleset) assert.Contains(t, string(stored), awsKey) assert.Equal(t, sha256Digest(string(original)), got.GetArtifact().Digest) // Nothing was transformed, so nothing is held in memory for the @@ -293,6 +295,7 @@ func TestChainloopAICodingSessionCrafterRedaction(t *testing.T) { } else { // Marked redacted whenever the scan ran, even when it found nothing. assert.Equal(t, "true", got.Annotations[api.AnnotationMaterialRedacted]) + assert.Equal(t, strconv.Itoa(redaction.RulesetVersion), got.Annotations[api.AnnotationMaterialRedactionRuleset]) assert.NotContains(t, got.Annotations, api.AnnotationMaterialRedactionSkipped) // The scanned copy is what policies must be handed, since a