Skip to content

fix: apply code quality findings - #740

Merged
kojiwakayama merged 1 commit into
mainfrom
fix/code-quality-findings
Mar 25, 2026
Merged

kojiwakayama merged 1 commit into
mainfrom
fix/code-quality-findings

Conversation

@kojiwakayama

Copy link
Copy Markdown
Contributor

Summary

Squashes safe fixes from Copilot Autofix PRs #730, #731, #733 into a single commit.

  • Proxy JWKS cache: Fix race condition with idempotent cache initialization
  • Proxy redirect validation: Ensure auth return path cannot be an absolute URL (open redirect prevention)
  • Project discovery: Guard cache deletion against concurrent overwrites
  • Version util: Use shorthand property for projectUpdated

Excluded PRs

Closes #730, closes #731, closes #733

- Fix JWKS cache race condition in proxy handler
- Add open redirect validation for auth return path
- Guard discovery cache deletion against concurrent overwrites
- Use shorthand property for projectUpdated param

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: def06e2b8d

ℹ️ 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".

Comment thread src/proxy/handler.ts
Comment on lines +383 to +384
if (!returnPath.startsWith("/") || returnPath.includes("://")) {
returnPath = "/";

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve valid query strings in auth return path

The new returnPath.includes("://") guard turns many legitimate protected URLs into from=/ (for example, /dashboard?next=https://docs.example.com) because it scans the entire path+query string, not just whether the return target itself is absolute. In those cases users lose their original destination after sign-in and are always sent to /, which is a functional regression for flows that include full URLs in query params.

Useful? React with 👍 / 👎.

if (!cacheCompletedDiscovery) {
discoveredProjects.delete(key);
const current = discoveredProjects.get(key);
if (current === promise) {
@kojiwakayama
kojiwakayama merged commit ddd7895 into main Mar 25, 2026
14 checks passed
@kojiwakayama
kojiwakayama deleted the fix/code-quality-findings branch March 25, 2026 22:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant