Repository navigation
Add secret-derived Basic Auth helpers - #452
Conversation
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds OAuth2 client_credentials token exchange and derived HTTP Basic Auth header support: new runtime helpers (oauthClientCredentials, secretHeaders.basic), {{secret-basic:...}} placeholder parsing/building, gateway expansion to resolve derived Basic headers, runtime exports, typecheck updates, tests, and documentation. ChangesOAuth2 client_credentials and Basic Auth feature
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsTimed out fetching pipeline failures after 30000ms Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
|
🔎 Preview deployed: https://kody-pr-452.kentcdodds.workers.dev Worker: Mocks:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/worker/src/mcp/fetch-gateway.ts`:
- Around line 107-117: The code currently always sets the replacement value to
buildBasicAuthHeader(...) (which includes the "Basic " scheme), so if a template
uses a pre-prefixed header like "Authorization: Basic {{secret-basic:...}}"
expansion becomes "Basic Basic ...". Update the loop that iterates
basicAuthPlaceholders (and uses buildBasicAuthSecretPlaceholderFromReference,
buildBasicAuthHeader, readResolvedSecretValue, resolvedValues, and replacements)
to create the authHeader once, then set two replacement keys: the normal
renderedPlaceholder -> authHeader, and the prefixed key ("Basic " +
renderedPlaceholder) -> authHeader with the "Basic " scheme stripped (e.g.,
authHeader.replace(/^Basic\s+/i, '')). This ensures both "{{secret-basic:...}}"
and "Basic {{secret-basic:...}}" expand correctly without duplicating the
scheme.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 49740107-0b39-43ed-a2d6-e58d908be592
📒 Files selected for processing (14)
docs/use/execute.mddocs/use/secrets-and-values.mdpackages/worker/src/mcp/execute-modules/codemode-utils.node.test.tspackages/worker/src/mcp/execute-modules/codemode-utils.tspackages/worker/src/mcp/fetch-gateway.node.test.tspackages/worker/src/mcp/fetch-gateway.tspackages/worker/src/mcp/generated-ui-api.tspackages/worker/src/mcp/run-codemode-registry.tspackages/worker/src/mcp/secrets/placeholders.tspackages/worker/src/mcp/server-instructions.tspackages/worker/src/mcp/tools/execute.tspackages/worker/src/package-runtime/module-graph.node.test.tspackages/worker/src/package-runtime/module-graph.tspackages/worker/src/repo/checks.ts
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 7c8dc00. Configure here.
| return input.scope | ||
| ? `{{secret-basic:username=${input.usernameSecret},password=${input.passwordSecret}|scope=${input.scope}}}` | ||
| : `{{secret-basic:username=${input.usernameSecret},password=${input.passwordSecret}}}` | ||
| } |
There was a problem hiding this comment.
Duplicated placeholder builder risks silent format divergence
Medium Severity
buildBasicAuthSecretPlaceholder is independently implemented with identical logic in both codemode-utils.ts (private, used by secretHeaders.basic) and placeholders.ts (exported, used by the fetch gateway for building and matching). Since the fetch gateway's parseBasicAuthSecretPlaceholders regex must match what secretHeaders.basic produces, any format change in one file without the other would silently break secret resolution at runtime. The codemode-utils.ts copy could import from placeholders.ts instead of duplicating the template string.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 7c8dc00. Configure here.
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/worker/src/mcp/fetch-gateway.ts`:
- Around line 114-120: The current replacement logic sets only the `Basic
${renderedPlaceholder}` key, so incoming headers with a lowercase scheme like
`basic {{...}}` will not match and produce `basic Basic ...`; update the
replacement setup in fetch-gateway.ts to handle scheme case-insensitively by
adding a lowercase variant (e.g., also set `basic ${renderedPlaceholder}`) or by
normalizing the scheme when creating keys (lowercasing the prefix before setting
entries) for the replacements Map; adjust where `prefixedPlaceholder`,
`renderedPlaceholder`, `replacements`, and `authHeader` are used so both `Basic`
and `basic` lookups map to `authHeader`.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 9c658537-fec3-4405-b001-00490b9281eb
📒 Files selected for processing (2)
packages/worker/src/mcp/fetch-gateway.node.test.tspackages/worker/src/mcp/fetch-gateway.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/worker/src/mcp/fetch-gateway.node.test.ts
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
2 participants
Summary
secretHeaders.basic(...)andoauthClientCredentials(...)runtime helpers.Validation
npx vitest run --project node-unit packages/worker/src/mcp/fetch-gateway.node.test.ts packages/worker/src/mcp/execute-modules/codemode-utils.node.test.ts packages/worker/src/package-runtime/module-graph.node.test.tsnpm run format:checknpm run lint(passes with existing warnings)npm run typechecknpm run testnpm run validatenpm run format:check && npx vitest run --project node-unit packages/worker/src/mcp/fetch-gateway.node.test.ts && npm run validatenpm run format && npm run format:check && npx vitest run --project node-unit packages/worker/src/mcp/fetch-gateway.node.test.ts && npm run validateSummary by CodeRabbit
New Features
Documentation