fix(agent-core): keep dotted file names readable in session titles - #2872
fix(agent-core): keep dotted file names readable in session titles#2872sailist wants to merge 13 commits into
Conversation
🦋 Changeset detectedLatest commit: e023c6a The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
commit: |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6ef1f84adc
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| /\bsk-[A-Za-z0-9_-]{12,}\b(?![A-Za-z0-9_-]*\.[A-Za-z0-9]{1,8}(?![A-Za-z0-9_]))/g, | ||
| '[redacted]', | ||
| ) | ||
| .replaceAll( | ||
| /\b[A-Za-z0-9][A-Za-z0-9+/=_-]{39,}\b(?![A-Za-z0-9+/=_-]*\.[A-Za-z0-9]{1,8}(?![A-Za-z0-9_]))/g, |
There was a problem hiding this comment.
When a prompt contains a token-looking path segment such as sk-abcdefghijklmnop1234.env or a 40+ character access token followed by .json, these new negative lookaheads classify it as a filename and skip both catch-all redactions, so the full value is persisted in lastPrompt/title and shown in session lists. Since this sanitizer is the metadata privacy boundary, keep known secret/token-shaped strings redacted even when a short extension follows, or require a safer filename context and mirror the fix in the v1 copy.
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9454699d0e
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
|
||
| function isFileNameStem(stem: string, following: string): boolean { | ||
| if (!/[-_]/.test(stem)) return false; | ||
| const extension = /^\.([A-Za-z0-9]{1,8})(?![A-Za-z0-9_])/.exec(following)?.[1]; |
There was a problem hiding this comment.
When a dotted token/JWT segment starts with an allow-listed extension followed by -, this lookahead treats it as a filename extension because - is not excluded. Fresh evidence in this revision is that a value like <40+ chars with _>.ts-<more token chars> now leaves the first long segment visible in lastPrompt/title while only redacting the second segment, even though the new tests intend dot-joined JWT-style tokens to stay redacted. Require a real filename boundary here, and mirror the fix in the legacy sanitizer copy.
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 40d2097b61
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
|
||
| function isFileNameStem(stem: string, following: string): boolean { | ||
| if (!/[-_]/.test(stem)) return false; | ||
| const extension = /^\.([A-Za-z0-9]{1,8})(?![A-Za-z0-9+/=_-])/.exec(following)?.[1]; |
There was a problem hiding this comment.
When the long slug uses common compound code suffixes such as .test.ts, .spec.ts, or .d.ts, this helper only evaluates the first dotted component (test, spec, or d) as the extension. Those are not in the allow-list, so prompts like refact-000-08-12-external-hooks-feature-scopes.test.ts still become [redacted].test.ts, leaving session titles unreadable for common test/declaration files; consider recognizing safe compound suffixes or checking the final code extension in both sanitizer copies.
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7205bd2e21
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if (!/[-_]/.test(stem)) return false; | ||
| const suffix = /^((?:\.[A-Za-z0-9]{1,8})+)(?![.A-Za-z0-9+/=_-])/.exec(following)?.[1]; | ||
| if (suffix === undefined) return false; | ||
| const extension = suffix.slice(suffix.lastIndexOf('.') + 1); | ||
| return SAFE_FILENAME_EXTENSIONS.has(extension.toLowerCase()); |
There was a problem hiding this comment.
When an unlabeled long base64/base64url token contains - or _ and is followed by an allow-listed code suffix (for example a signed URL path segment like <40+ chars>_.ts?download=1), this helper classifies it as a filename and returns the raw match, so the secret still lands in lastPrompt/title; the same logic is copied in packages/agent-core/src/session/prompt-metadata.ts. Fresh evidence after the earlier comments is that the current allow-list fixes .json/.env cases but still exempts .ts/.js/other code suffixes, so ambiguous token-shaped stems should fail closed or require stronger human-filename context.
Useful? React with 👍 / 👎.
|
@codex review |
|
Codex Review: Didn't find any major issues. Keep it up! Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b3ba50f7e1
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| function isAbsolutePath(match: string, offset: number, source: string): boolean { | ||
| if (offset === 0 || source[offset - 1] !== '/') return false; | ||
| if (!match.includes('/')) return false; | ||
| return match.split('/').every((segment) => segment.length < 40); |
There was a problem hiding this comment.
When the pasted secret is a base64/base64url-looking value that begins with / and contains another / before any segment reaches 40 chars, this absolute-path exception returns true and the replacement keeps the full token, e.g. /Ab1c…/Z9x8… remains visible in lastPrompt/titles. The catch-all regex previously redacted these slash-containing tokens, so require stronger local-path context or fail closed for slash-delimited token-shaped values in both sanitizer copies.
Useful? React with 👍 / 👎.
| }); | ||
|
|
||
| it('keeps absolute paths readable but redacts token-looking segments', () => { | ||
| expect(sanitize('cd /Users/moonshot/Projects/kimi-code-workspace/')).toBe( |
There was a problem hiding this comment.
These new absolute-path fixtures add moonshot as a username in public test data, while the repository guide asks public text and test data to use neutral placeholders instead. Please replace the user/path with neutral values in both prompt-metadata test copies so the tests do not introduce context-specific internal identifiers.
AGENTS.md reference: AGENTS.md:L80-L80
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 317e0a838b
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| ]); | ||
|
|
||
| function isFileNameStem(stem: string, following: string): boolean { | ||
| if (!/^(?=.*[-_])[a-z0-9_/-]+$/.test(stem)) return false; |
There was a problem hiding this comment.
For prompts that mention a long slug file under an absolute path with capitalized directories, such as /Users/alice/Projects/.../refact-000-08-12-external-hooks-feature-scopes.ts, the catch-all match includes the directory prefix, so this all-lowercase stem check fails before looking at the safe .ts suffix; isAbsolutePath then also rejects the match because the basename segment is at least 40 chars, and the title becomes /[redacted].ts. Since this is the main filename shape the patch is trying to keep readable, evaluate the basename separately, and mirror the fix in the legacy sanitizer.
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 87c3e84b80
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| ]); | ||
|
|
||
| function isFileNameStem(stem: string, following: string): boolean { | ||
| if (!/^(?=.*[-_])[a-z0-9_/-]+$/.test(stem)) return false; |
There was a problem hiding this comment.
When a long lowercase token appears as a path segment before a safe hyphenated filename, this check treats the whole token/refact-... prefix as a filename stem because / is allowed and the - can come from the basename. For example, open ${'a'.repeat(44)}/refact-000-08-12-external-hooks-feature-scopes.ts is kept verbatim in lastPrompt/titles even though the same 40+ token would be redacted when bare; the copied v1 helper has the same behavior. Validate only the basename as the readable filename (and fail closed on token-length path segments), or remove path prefixes from this filename-stem exemption.
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 12f7bfd906
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| const directories = segments.slice(0, -1); | ||
| const base = segments[segments.length - 1]; | ||
| if (!directories.every(isWordShapedSegment)) return false; | ||
| if (isWordShapedSegment(base)) return true; |
There was a problem hiding this comment.
Fresh evidence beyond the earlier mixed-case slash-token comment is that all-lowercase or hex slash-delimited tokens still take this branch: a prompt like token ${'a'.repeat(32)}/${'b'.repeat(32)} is preserved verbatim in lastPrompt/title because both segments are under 40 chars and match isWordShapedSegment, even though the same token material without / is redacted. For token IDs split across URL/path segments and pasted without :=, require stronger local-path context or fail closed for extensionless multi-segment matches; the legacy packages/agent-core/src/session/prompt-metadata.ts copy has the same issue.
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 735b853452
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| function isWordShapedSegment(segment: string): boolean { | ||
| return segment.length < 40 && /^([A-Z]?[a-z0-9_-]*|[A-Z0-9_-]+)$/.test(segment); |
There was a problem hiding this comment.
When the prompt mentions a long path whose basename is camelCase or PascalCase, such as packages/agent-core-v2/src/agent/prompt/promptMetadataText.ts, isPathLike() rejects it here because the segment regex only accepts lowercase, one initial capital, or all-caps words. The catch-all token regex then still collapses the whole path to [redacted].ts, so titles remain unreadable for common TypeScript files in this repo; allow normal code filename casing or decide path basenames from their safe extension before this word-shape filter. The legacy copy in packages/agent-core/src/session/prompt-metadata.ts has the same behavior.
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ea4623ae9d
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| return true; | ||
| } | ||
| if (!isWordShapedSegment(base)) return false; | ||
| return segments.length >= 3 && segments.every((segment) => segment.length <= 24); |
There was a problem hiding this comment.
Fresh evidence in this revision is that the new extensionless path exemption preserves any 3+ word-shaped segments of at most 24 characters, so a pasted token split like aaaaaaaaaaaaaaaaaaaa/bbbbbbbbbbbbbbbbbbbb/cccccccccccccccccccc is kept verbatim in lastPrompt/session titles instead of being redacted by the catch-all pattern. This still affects the v1 copy as well, and because these metadata fields are displayed in session lists, slash-delimited opaque IDs should fail closed unless there is stronger local-path context.
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9dcd7aacbe
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if (!isWordShapedSegment(base)) return false; | ||
| if (segments.length < 3 || !segments.every((segment) => segment.length <= 24)) return false; | ||
| if (offset === 0 || source[offset - 1] !== '/') return false; | ||
| return PATH_ROOT_SEGMENTS.has(segments[0].toLowerCase()) || source[offset - 2] === '~'; |
There was a problem hiding this comment.
Fresh evidence in this revision is that the new root allow-list still preserves extensionless matches whose first segment is also a common API route, so a pasted value like /users/${'a'.repeat(20)}/${'b'.repeat(20)}/${'c'.repeat(20)} or /data/... is kept verbatim in lastPrompt/session titles instead of being redacted. Since these metadata fields are the privacy boundary and slash-delimited opaque IDs often appear under /users or /data, require stronger filesystem context than just the first segment name; the copied v1 helper has the same behavior.
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e023c6a9ce
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if (!isWordShapedSegment(base)) return false; | ||
| if (segments.length < 3 || !segments.every((segment) => segment.length <= 24)) return false; | ||
| if (offset === 0 || source[offset - 1] !== '/') return false; | ||
| return PATH_ROOT_SEGMENTS.has(segments[0]) || source[offset - 2] === '~'; |
There was a problem hiding this comment.
Fresh evidence in this revision is that the narrowed root allow-list still preserves lowercase/hex token chunks when they sit under an allowed filesystem root: secret /tmp/${'a'.repeat(20)}/${'b'.repeat(20)} reaches this return with tmp allow-listed and both opaque segments under the 24-char cap, so the catch-all leaves the full value in lastPrompt/session titles instead of redacting it. Require stronger local filesystem context than root + short word-shaped segments, or fail closed for repeated opaque segments here and in the copied v1 helper.
Useful? React with 👍 / 👎.
1 participant
Related Issue
No linked issue — the problem is explained below.
Problem
Session titles and
lastPromptmetadata are sanitized to avoid leaking secrets pasted into prompts. The catch-all "long token" patterns (thesk-…key pattern and the 40+ character base64/base64url pattern) also match ordinary file names, because their character classes include-and_. A prompt like "帮我看看 refact-000-08-12-external-hooks-feature-scopes.ts" ends up titled "帮我看看 [redacted].ts", and even names likesk-project-notes-2024.mdare redacted — which makes session list titles useless for exactly the kind of prompts developers write most.What changed
agent-coreand the parallelagent-core-v2port): a match followed by.+ a short (1–8 char) alphanumeric extension is treated as a file name and kept readable. The extension length cap plus a trailing alphanumeric check ensures dot-joined secrets such as JWT segments (payload.signature) are still redacted.sk-…keys, and 40-char hex strings such as git SHAs (indistinguishable from tokens without context).Checklist
gen-changesetsskill, or this PR needs no changeset.gen-docsskill, or this PR needs no doc update.