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")