From 7c96eea8df378618c508c7bbe120e2c335bf4fa3 Mon Sep 17 00:00:00 2001 From: JasonBuildAI Date: Fri, 30 Oct 2026 13:44:10 +0800 Subject: [PATCH] fix(dsh): mirror every canonical credential shape in the observer The DSH shadow observer is the only writer of `reliability_diagnostics` ledger bytes, and its documented contract is that credential-like tokens fail closed before the first append. It enforced that contract with a hand-written copy of the canonical owner's shapes that carried two of the nine provider-issuer arms and four of the labeled arms, raised the GitHub floor from 16 to 20, and accepted only two directory names under a Windows drive. A 38-value cross-language differential at upstream/main found seventeen values the canonical `loopx/public_safe_text.py` owner refuses and the producer accepted and wrote in cleartext: a GitHub fine-grained PAT and any 16-19 character classic PAT, AWS access and session key ids, Slack bot and app tokens, a Google API key, a Stripe restricted key, npm and PyPI tokens, `api_key:`, `access_token:`, `refresh_token:`, `password:` and `secret:` labels, and any drive path outside `users` / `documents and settings`. Spell every arm byte-identically to the canonical owner and add a parity test that rebuilds the canonical alternation from the mirror's own arms, so an arm added, narrowed or invented on one side fails in the suite that runs on both. No envelope, stats, ledger or activation contract changes. Fixes #7729 Signed-off-by: JasonBuildAI --- packages/dsh-loopx-plugin/src/observer.ts | 40 ++++++++-- .../dsh-loopx-plugin/tests/observer.spec.ts | 59 ++++++++++++++ ...st_reliability_diagnostics_dsh_provider.py | 79 +++++++++++++++++++ 3 files changed, 171 insertions(+), 7 deletions(-) diff --git a/packages/dsh-loopx-plugin/src/observer.ts b/packages/dsh-loopx-plugin/src/observer.ts index bedc06025d..dcfcf3b3c7 100644 --- a/packages/dsh-loopx-plugin/src/observer.ts +++ b/packages/dsh-loopx-plugin/src/observer.ts @@ -71,19 +71,45 @@ export const RUN_IDENTITY_FIELDS = [ const IDENTITY_TOKEN = /^[A-Za-z0-9][A-Za-z0-9_.:-]{0,120}$/u const SUMMARY_TOKEN = /^[A-Za-z0-9][A-Za-z0-9_./:-]{0,79}$/u const LOCAL_PATH_SURFACES = [ - /(?]+/iu, - /(?]+/iu, - /(?]+/iu, + /(?]+/iu, + /(?]+/iu, + /(?]+/iu, + /(?]+/iu, + /(?]+/iu, /\bfile:\/+/iu, ] as const +/** + * One literal per arm of `loopx/public_safe_text.py` SECRET_LIKE_SURFACE_PATTERN, + * byte-identical to the canonical body and case-folded the way its leading + * `(?i)` folds it. The plugin is published standalone and cannot import the + * repository's control-plane modules, so the copy stays — but + * `tests/capabilities/test_reliability_diagnostics_dsh_provider.py` compares + * these bodies against the canonical owner, and fails when an arm is missing, + * narrowed or added on only one side (Refs #5136). + * + * One difference the text comparison cannot see: JavaScript's `\b` is an ASCII + * word boundary under the `u` flag while Python's is Unicode-aware, so a token + * glued to a non-ASCII letter is rejected here and accepted there. That is the + * fail-closed direction, so it stays. + */ const SECRET_LIKE_SURFACES = [ /\bbearer\s+[a-z0-9._~+/=-]{16,}/iu, - /\b(?:access|secret)[_-]?key\s*[=:]\s*[^\s`'"<>]+/iu, - /\b(?:ak|sk)\s*[=:]\s*[^\s`'"<>]+/iu, + /\b(?:access|api|secret)[_-]?key["']?\s*[=:]\s*["']?[^\s`'"<>]+/iu, + /\b(?:ak|sk)["']?\s*[=:]\s*["']?[^\s`'"<>]+/iu, /(?]{12,}/iu, + /\b(?:access|refresh)[_-]?token["']?\s*[=:]\s*["']?[^\s`'"<>]{12,}/iu, + /\b(?:password|secret)["']?\s*[=:]\s*["']?[^\s`'"<>]{12,}/iu, + /-{3,}\s*BEGIN (?:[A-Z]+ )?PRIVATE KEY/iu, + /\btoken["']?\s*[=:]\s*["']?[^\s`'"<>]{12,}/iu, ] as const const CREDENTIAL_FIELD_FAMILIES = new Set([ 'accesskey', 'accesstoken', 'apikey', 'authtoken', 'authorization', diff --git a/packages/dsh-loopx-plugin/tests/observer.spec.ts b/packages/dsh-loopx-plugin/tests/observer.spec.ts index 1fd8ae163d..5cbad823a1 100644 --- a/packages/dsh-loopx-plugin/tests/observer.spec.ts +++ b/packages/dsh-loopx-plugin/tests/observer.spec.ts @@ -304,6 +304,65 @@ describe('shadow observer envelopes', () => { expect(stats.observed_event_count).toBe(envelopes.length + unsafeCount) }) + // Refs #7729: the producer's fail-closed check is a copy of the canonical + // owner's shapes (loopx/public_safe_text.py SECRET_LIKE_SURFACE_PATTERN), and + // the copy has to refuse every arm that owner refuses. Values are assembled + // from parts so no credential-looking literal is committed. + it('refuses every canonical credential arm before the first append', async () => { + const gh = 'gh' + 'p_' + const githubPat = 'github' + '_pat_' + const aws = 'A' + 'KIA' + const awsSession = 'A' + 'SIA' + const slackBot = 'xox' + 'b-' + const slackApp = 'xox' + 'p-' + const googleKey = 'AI' + 'za' + const stripeRestricted = 'r' + 'k_live_' + const npmToken = 'np' + 'm_' + const pypiToken = 'py' + 'pi-' + const drivePath = 'C' + ':/private/notes.txt' + // One value per canonical arm, plus the arm variants the copy used to narrow. + const refused: ReadonlyArray = [ + ['github_classic_floor', gh + '1234567890abcdef'], + ['github_classic_floor_plus_three', gh + '1234567890abcdefghi'], + ['github_fine_grained', githubPat + '11ABCDEFG0abcdefghijklmnopqrstuvwxyz0123456789'], + ['aws_access_key_id', aws + 'IOSFODNN7EXAMPLE'], + ['aws_session_key_id', awsSession + 'IOSFODNN7EXAMPLE'], + ['slack_bot_token', slackBot + '123456789012-123456789012-abcdefghijklmnopqrstuvwx'], + ['slack_app_token', slackApp + '123456789012-123456789012-abcdefghijklmnopqrstuvwx'], + ['google_api_key', googleKey + 'SyA1234567890abcdefghijklmnopqrstuvw'], + ['stripe_restricted_key', stripeRestricted + 'abcdefghijklmnopqrstuvwx'], + ['npm_token', npmToken + 'abcdefghijklmnopqrstuvwxyz0123456789'], + ['pypi_token', pypiToken + 'AgEIcHlwaS5vcmcabcdefghijklmnop'], + ['api_key_label_without_floor', 'api' + '_key:abc'], + ['access_token_label', 'access' + '_token:abcdefghijkl'], + ['refresh_token_label', 'refresh' + '_token:abcdefghijkl'], + ['password_label', 'pass' + 'word:abcdefghijkl'], + ['secret_label', 'sec' + 'ret:abcdefghijkl'], + ['windows_drive_outside_user_profile', drivePath], + ] + for (const [label, value] of refused) { + const { appended, observer } = observerWithCapture() + observer.observeSessionEvent(fakeSession(), sessionEvent('tool/call', 1, 1_756_728_001_000, { + turn: 1, step: 1, callId: 'call-1', name: value, + })) + await observer.flush() + const stats = parsed(appended).at(-1) as ObserverStats + expect(stats.rejected_by_reason, label).toEqual({ public_safety_violation: 1 }) + expect(appended.flat().join('\n'), label).not.toContain(value) + } + const accepted = ['skill-python', 'token_count', 'tool/read-file', 'error-timeout'] + for (const value of accepted) { + const { appended, observer } = observerWithCapture() + observer.observeSessionEvent(fakeSession(), sessionEvent('tool/call', 1, 1_756_728_001_000, { + turn: 1, step: 1, callId: 'call-1', name: value, + })) + await observer.flush() + const stats = parsed(appended).at(-1) as ObserverStats + expect(stats.rejected_by_reason, value).toEqual({}) + expect(appended.flat().join('\n'), value).toContain(value) + } + }) + it('drops with a count while a flush is in flight and the buffer is full', async () => { const appended: string[][] = [] let release: (() => void) | undefined diff --git a/tests/capabilities/test_reliability_diagnostics_dsh_provider.py b/tests/capabilities/test_reliability_diagnostics_dsh_provider.py index b413c114e8..391dd66df6 100644 --- a/tests/capabilities/test_reliability_diagnostics_dsh_provider.py +++ b/tests/capabilities/test_reliability_diagnostics_dsh_provider.py @@ -25,6 +25,12 @@ normalize_observer_envelope, ) from loopx.capabilities.reliability_diagnostics.intake import STATS_FIELDS +from loopx.public_safe_text import ( + HOME_RELATIVE_PATH_PATTERN, + LOCAL_PATH_SURFACE_PATTERN, + PATH_PREFIX_LOCAL_PATTERN, + SECRET_LIKE_SURFACE_PATTERN, +) ROOT = Path(__file__).resolve().parents[2] OBSERVER_TS = ROOT / "packages/dsh-loopx-plugin/src/observer.ts" @@ -160,6 +166,79 @@ def test_shared_counterfactual_allows_safe_source_ref(value: str) -> None: assert normalize_observer_envelope(record).source_refs["tool_call_id"] == value +def _typescript_arms(constant: str) -> list[str]: + """Return the regex bodies of one `observer.ts` shape list, in file order. + + The producer writes one literal per line so each body is the text between + the opening slash and the `/iu` flags. The order is read from the file + because the canonical comparison rebuilds its alternation in that order, so + reordering the arms reads as a failure here even though it changes no + verdict. That is the point: the list is a copy, and a copy is compared as + text. + """ + + source = OBSERVER_TS.read_text(encoding="utf-8") + block = source.split(f"const {constant} = [", 1)[1].split("] as const", 1)[0] + arms = [] + for line in block.splitlines(): + arm = line.strip().rstrip(",") + if not arm: + continue + assert arm.startswith("/") and arm.endswith("/iu"), arm + arms.append(arm[1 : -len("/iu")]) + return arms + + +def _reduced_pattern_source(source: str) -> str: + """Drop the escapes that only one spelling of the same regex needs. + + `public_safe_text.py` writes `\\"` inside raw strings, a redundant escape of + `"`, and a JavaScript regex literal must escape a `/` that appears outside a + character class. Neither escape changes either regex's meaning, so reducing + both lets the two owners be compared as exact text. Only an odd-length + backslash run escapes those characters, which keeps `[\\\\/]` intact. + """ + + return re.sub(r'(? None: + """Refs #7729: the producer's fail-closed check is a copy of the canonical + owner's shapes, and a scan that recognizes only some issuers certifies the + rest as public-safe. Rebuilding the alternation from the mirror's own arms + makes a missing, narrowed, reordered or invented arm fail here -- which is + where the canonical owner's next issuer addition lands. + """ + + rebuilt = "(?i)(?:" + "|".join(_typescript_arms("SECRET_LIKE_SURFACES")) + ")" + assert _reduced_pattern_source(rebuilt) == _reduced_pattern_source( + SECRET_LIKE_SURFACE_PATTERN.pattern + ) + + +def test_typescript_mirror_equals_the_canonical_local_path_surfaces() -> None: + """Refs #7729: the mirror narrowed the drive arm to two directory names, so + every other drive path reached the ledger. The absolute, drive and UNC + alternatives share one lookbehind in the canonical owner, so the mirror + spells that lookbehind on each of the three. + """ + + lookbehind = r"(? None: driver = DRIVER_TS.read_text(encoding="utf-8") observer = OBSERVER_TS.read_text(encoding="utf-8")