🔒 [Security Fix] Enforce Strict Cookie Controls for CSRF in Production#1222
🔒 [Security Fix] Enforce Strict Cookie Controls for CSRF in Production#1222Hardonian wants to merge 2 commits into
Conversation
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🔒 Release Safety Check Results✅ Safety Checks Passed {"code":"NOT_FOUND","message":"Requested function was not found"} |
🔍 Classification Check Results
✅ No violations detected. Safe to merge! |
![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: 252ffa3c9b
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| httpOnly: isProduction, // Must be accessible to JavaScript in dev, strict in prod | ||
| secure: isProduction || config.security.secureCookies, // HTTPS only in production |
There was a problem hiding this comment.
When production users already have a csrf-token from the previous release, this branch is skipped because existingToken is present, so the browser keeps the old HttpOnly=false/possibly non-Secure cookie attributes until the 24-hour expiry; the same skip exists in getCsrfToken. In production/preview, re-set the cookie with the existing token (or clear it) so the stricter attributes take effect immediately after deployment.
Useful? React with 👍 / 👎.
- Replaced `cargo-deny-action@v1` with `@v2` and `github/codeql-action/*@v3` with `@v4` to avoid deprecation errors on Node 24. - Fixed `gitleaks-action` which now requires `GITHUB_TOKEN` injected instead of `config-path`. - Updated `packages/logger/tsconfig.json` to emit composite type declarations and added a `references` to it in `packages/agents/tsconfig.json`, resolving the `error TS2307: Cannot find module '@settler/logger'` build failure.
🔒 Release Safety Check Results✅ Safety Checks Passed {"code":"NOT_FOUND","message":"Requested function was not found"} |
🔍 Classification Check Results
✅ No violations detected. Safe to merge! |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2932abcbb5
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| uses: EmbarkStudios/cargo-deny-action@v2 | ||
| with: | ||
| command: check |
There was a problem hiding this comment.
In this Rust Verify workflow, replacing the CLI invocation that ran from crates/ means the job no longer loads the repository's only deny policy (crates/deny.toml; I checked with find . -maxdepth 2 -name deny.toml). The cargo-deny action runs from the repository root and its docs/action.yml say it defaults to --manifest-path ./Cargo.toml and expects a root deny.toml, so PR/scheduled Rust security checks will either run without Settler's license/source/bans configuration or fail to find it. Please pass the existing config path (or move the file to the root) so the gate keeps enforcing the same policy.
Useful? React with 👍 / 👎.
|
Closed during PR cleanup: mergeable=UNKNOWN base=main. Reopen only with a current verified change. |
1 participant
🎯 What: The CSRF token cookie was being generated with
⚠️ Risk: If an application allows JavaScript access to the CSRF cookie (
httpOnly: falsehardcoded andsecure: config.security.secureCookieswithout explicit defaults ensuring strictness in production.httpOnly: false) in production, it increases the risk that an XSS vulnerability could be used to steal the CSRF token and bypass CSRF protection. In addition, not enforcing thesecureflag in production makes it susceptible to Man-in-the-Middle attacks if the site is ever loaded over HTTP.🛡️ Solution: Updated
csrfProtectioncookie configuration to enforcehttpOnly: trueandsecure: truewhen running in production or preview environments. The token remains available to the client via the response body of the/api/csrf-tokenendpoint, allowing clients to securely authenticate state-changing requests without the need for raw JS access to the cookie.PR created automatically by Jules for task 15726436235836208553 started by @Hardonian