From b35808fc83d334a25366c85507e422ce62390bc0 Mon Sep 17 00:00:00 2001 From: Nguyen Thanh Dat Date: Wed, 19 Aug 2026 16:22:52 +0700 Subject: [PATCH 1/2] fix(share): scrub credentials that carry no upper-case character MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The last-resort secret pattern requires an upper-case letter so it does not fire on prose: (?=[A-Za-z0-9+/_-]*[A-Z])(?=...[a-z])(?=...[0-9]) That makes it blind to credentials whose body is lower-case and digits only, and those reached the share card intact. Measured against buildFallbackHeadline on main: xoxb-123456789012-987654321098-abcdefghijklmnop verbatim xoxp-987654321098-123456789012-zyxwvutsrqponmlk verbatim glpat-abcdefghij1234567890 verbatim hf_abcdefghijklmnopqrstuvwxyz1234 verbatim https://discord.com/api/webhooks// verbatim The module states its own bar -- "nothing private must leak into a shared card" -- and this is the card text that gets posted publicly. Adds explicit patterns for those shapes rather than relaxing the last-resort rule, which would trade this for over-redacting prose; that trade is yours to make, not mine to assume. One detail worth keeping: `_` is optional in the huggingface pattern because stripMarkdown strips emphasis characters before redaction runs, so `hf_abc…` arrives as `hfabc…`. A pattern insisting on the underscore would never match. Pinned by a test. Eight tests. Six are red without the src change. The Slack webhook case and the prose guard pass either way -- the first because that URL's path happens to contain upper-case, the second by design. 62 tests green across src/features/share. prettier, eslint and tsc all exit 0. --- app/src/features/share/shareContent.test.ts | 42 +++++++++++++++++++++ app/src/features/share/shareContent.ts | 13 +++++++ 2 files changed, 55 insertions(+) diff --git a/app/src/features/share/shareContent.test.ts b/app/src/features/share/shareContent.test.ts index db6497fd0b8..f2fa5665ff2 100644 --- a/app/src/features/share/shareContent.test.ts +++ b/app/src/features/share/shareContent.test.ts @@ -48,6 +48,48 @@ describe('redactSensitive', () => { expect(redactSensitive('mail jane.doe@example.com now')).toContain('[email]'); }); + // The last-resort rule requires an upper-case character so it does not fire on + // prose. Everything below is lower-case + digits, so it reached the card intact + // until these patterns were added. Each string is a shape-accurate dummy. + // + // The Slack ones are assembled from parts on purpose: written as one literal they + // match GitHub's Slack-token detector well enough that push protection rejects the + // commit, which would block this file for anyone pushing it. + const slackToken = (prefix: string, ...rest: string[]) => [prefix, ...rest].join('-'); + + test.each([ + ['slack bot token', slackToken('xoxb', '123456789012', '987654321098', 'abcdefghijklmnop')], + ['slack user token', slackToken('xoxp', '987654321098', '123456789012', 'zyxwvutsrqponmlk')], + ['gitlab personal access token', 'glpat-abcdefghij1234567890'], + ['huggingface token', 'hf_abcdefghijklmnopqrstuvwxyz1234'], + ])('scrubs an all-lower-case %s', (_label, secret) => { + const out = redactSensitive(`saved ${secret} for you`); + expect(out).toContain('[redacted]'); + expect(out).not.toContain(secret); + }); + + test.each([ + ['slack', 'https://hooks.slack.com/services/T00000000/B00000000/abcdefghijklmnopqrst'], + [ + 'discord', + 'https://discord.com/api/webhooks/123456789012345678/abcdefghijklmnopqrstuvwxyz012345', + ], + ])('scrubs a %s webhook URL, which carries its secret in the path', (_label, url) => { + expect(redactSensitive(`posting to ${url} now`)).not.toContain(url); + }); + + test('survives stripMarkdown running first, as buildFallbackHeadline runs it', () => { + // stripMarkdown removes `_`, so a pattern that insisted on it would never match. + const head = buildFallbackHeadline('Done. Saved hf_abcdefghijklmnopqrstuvwxyz1234 for you.'); + expect(head).not.toContain('abcdefghijklmnopqrstuvwxyz'); + expect(head).toContain('[redacted]'); + }); + + test('does not fire on ordinary lower-case prose', () => { + const clean = 'renamed the folder to archive and moved twelve files into it'; + expect(redactSensitive(clean)).toBe(clean); + }); + test('is idempotent and leaves clean prose untouched', () => { const clean = 'Summarised three months of emails in twelve seconds'; expect(redactSensitive(clean)).toBe(clean); diff --git a/app/src/features/share/shareContent.ts b/app/src/features/share/shareContent.ts index f7efa84475a..63c6f144e06 100644 --- a/app/src/features/share/shareContent.ts +++ b/app/src/features/share/shareContent.ts @@ -36,6 +36,19 @@ const REDACTIONS: ReadonlyArray<{ re: RegExp; with: string }> = [ { re: /\bBearer\s+[A-Za-z0-9._-]{12,}\b/gi, with: '[redacted]' }, // AWS access key ids. { re: /\bAKIA[0-9A-Z]{16}\b/g, with: '[redacted]' }, + // Vendor-prefixed credentials whose body is lower-case and digits only. The + // last-resort rule below cannot reach these: it requires an upper-case letter so + // it does not fire on prose, and an all-lower-case token slips straight past it. + // The `_` is optional because `stripMarkdown` strips emphasis characters before + // this runs, so `hf_abc…` arrives here as `hfabc…`. + { re: /\b(?:xox[abeprs]|xapp)-[A-Za-z0-9-]{10,}/g, with: '[redacted]' }, + { re: /\bglpat-[A-Za-z0-9_-]{16,}/g, with: '[redacted]' }, + { re: /\bhf_?[a-z0-9]{20,}\b/g, with: '[redacted]' }, + // Webhook URLs carry the secret in the path, so the whole URL has to go. + { + re: /\bhttps?:\/\/(?:hooks\.slack\.com|discord(?:app)?\.com\/api\/webhooks)\/[^\s"'`)]+/gi, + with: '[redacted]', + }, // Long opaque hex runs (>= 32 chars) that look like secrets, not prose. { re: /\b[A-Fa-f0-9]{32,}\b/g, with: '[redacted]' }, // Long opaque base64/base64url runs (>= 24 chars) that mix upper/lower/digit, From 7850e5cb646c792a3693861c5aab0f4a389d9174 Mon Sep 17 00:00:00 2001 From: Nguyen Thanh Dat Date: Wed, 19 Aug 2026 17:26:19 +0700 Subject: [PATCH 2/2] test(share): assemble every vendor token fixture instead of writing it whole Review point from CodeRabbit on the glpat fixture, and it holds for the huggingface one too: a shape-exact literal is what a secret scanner keys on, so writing one into a test file risks blocking the push for whoever touches it next. GitHub push protection already rejected the Slack form on the first attempt at this branch, which is why those two were assembled and the other two were not -- an inconsistency, not a judgement. Generalises the existing helper to take the separator, so `hf_` is covered by the same mechanism as the dashed prefixes rather than a second one: const assembled = (separator: string, ...parts: string[]) => parts.join(separator); All four values are unchanged, so the evidence table in the PR description still describes exactly what these tests feed in. Every fixture here is a synthetic dummy; none is or ever was a live credential. The webhook URLs are left as literals on purpose. Their secret components are `T00000000` / `B00000000` / a straight alphabet run -- visibly placeholder, and no scanner has flagged them. 62 tests green across src/features/share. prettier, eslint and tsc exit 0. --- app/src/features/share/shareContent.test.ts | 26 +++++++++++++-------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/app/src/features/share/shareContent.test.ts b/app/src/features/share/shareContent.test.ts index f2fa5665ff2..e202d0cdb20 100644 --- a/app/src/features/share/shareContent.test.ts +++ b/app/src/features/share/shareContent.test.ts @@ -50,18 +50,23 @@ describe('redactSensitive', () => { // The last-resort rule requires an upper-case character so it does not fire on // prose. Everything below is lower-case + digits, so it reached the card intact - // until these patterns were added. Each string is a shape-accurate dummy. + // until these patterns were added. Each value is a shape-accurate dummy, not a + // real credential. // - // The Slack ones are assembled from parts on purpose: written as one literal they - // match GitHub's Slack-token detector well enough that push protection rejects the - // commit, which would block this file for anyone pushing it. - const slackToken = (prefix: string, ...rest: string[]) => [prefix, ...rest].join('-'); + // Every vendor-prefixed value is assembled from parts on purpose: written as a + // single literal it carries the vendor's exact token shape, which is what a + // secret scanner keys on. GitHub push protection already rejects the Slack form + // outright, and that would block this file for anyone pushing it. + const assembled = (separator: string, ...parts: string[]) => parts.join(separator); test.each([ - ['slack bot token', slackToken('xoxb', '123456789012', '987654321098', 'abcdefghijklmnop')], - ['slack user token', slackToken('xoxp', '987654321098', '123456789012', 'zyxwvutsrqponmlk')], - ['gitlab personal access token', 'glpat-abcdefghij1234567890'], - ['huggingface token', 'hf_abcdefghijklmnopqrstuvwxyz1234'], + ['slack bot token', assembled('-', 'xoxb', '123456789012', '987654321098', 'abcdefghijklmnop')], + [ + 'slack user token', + assembled('-', 'xoxp', '987654321098', '123456789012', 'zyxwvutsrqponmlk'), + ], + ['gitlab personal access token', assembled('-', 'glpat', 'abcdefghij1234567890')], + ['huggingface token', assembled('_', 'hf', 'abcdefghijklmnopqrstuvwxyz1234')], ])('scrubs an all-lower-case %s', (_label, secret) => { const out = redactSensitive(`saved ${secret} for you`); expect(out).toContain('[redacted]'); @@ -80,7 +85,8 @@ describe('redactSensitive', () => { test('survives stripMarkdown running first, as buildFallbackHeadline runs it', () => { // stripMarkdown removes `_`, so a pattern that insisted on it would never match. - const head = buildFallbackHeadline('Done. Saved hf_abcdefghijklmnopqrstuvwxyz1234 for you.'); + const hfToken = assembled('_', 'hf', 'abcdefghijklmnopqrstuvwxyz1234'); + const head = buildFallbackHeadline(`Done. Saved ${hfToken} for you.`); expect(head).not.toContain('abcdefghijklmnopqrstuvwxyz'); expect(head).toContain('[redacted]'); });