fix(cron): validate custom session ids#59835
Conversation
Greptile SummaryThis PR hardens cron custom session ID handling by introducing Confidence Score: 5/5Safe to merge — the fix is narrowly scoped, fails closed on invalid input, and all prior review concerns are addressed. Both previous thread concerns (adding SAFE_SESSION_ID_RE enforcement and backslash/NUL test cases) are resolved in this diff. No new P0/P1 issues introduced. The two-path run-time logic (parseAgentSessionKey fast-track → normalizeCronCustomSessionId fallback → cron: default) is correct and covered by tests at both the unit and integration level. No files require special attention. Reviews (3): Last reviewed commit: "fix(cron): preserve colon-delimited sess..." | Re-trigger Greptile |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 41557fe2a5
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 41557fe2a5
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c01bdd74fd
ℹ️ 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".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e988192ec6
ℹ️ 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".
|
@codex review |
|
Codex Review: Didn't find any major issues. Can't wait for the next one! ℹ️ 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". |
Summary
session:<id>cron targets that contain path separator characters or NULssessionKeyChanges
src/cron/normalize.tscron:<job-id>Validation
corepack pnpm test -- src/cron/normalize.test.ts src/gateway/server.cron.test.tssession:../../...values no longer survive normalizationsession:targets do not override the cron-scoped fallback session keyclaude -p "/review"; it requested interactive GitHub approval and returned no code findings in this environmentNotes
session:<id>inputs still behave as before