-
Notifications
You must be signed in to change notification settings - Fork 169
feat(worker,web): Support google secrets as a token type #594
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughAdded support for Google Cloud Secret Manager as an alternative token source across the application. Updated schema definitions and TypeScript types to accept either environment variables ( Changes
Sequence DiagramsequenceDiagram
participant Caller
participant getTokenFromConfig
participant SecretManagerServiceClient
participant GCP
Caller->>getTokenFromConfig: token: { env: string } | { gcpSecretPath: string }
alt env branch
getTokenFromConfig->>getTokenFromConfig: read process.env[env]
getTokenFromConfig-->>Caller: return token string
else gcpSecretPath branch (new)
getTokenFromConfig->>SecretManagerServiceClient: create client
getTokenFromConfig->>GCP: accessSecretVersion(gcpSecretPath)
GCP-->>SecretManagerServiceClient: SecretVersion payload
getTokenFromConfig->>getTokenFromConfig: decode payload.data
getTokenFromConfig-->>Caller: return token string
else error
getTokenFromConfig-->>Caller: throw error (invalid config)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
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 |
|
@brendan-kellam your pull request is missing a changelog! |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🧹 Nitpick comments (11)
schemas/v3/shared.json (1)
20-32: Consider adding pattern validation for gcpSecretPath format.The description specifies the required format
projects/<project-id>/secrets/<secret-name>/versions/<version-id>, but there's no JSON schemapatternproperty to enforce this. This could allow invalid paths to pass schema validation.Consider adding a pattern to validate the format:
{ "type": "object", "properties": { "gcpSecretPath": { "type": "string", - "description": "The path to the GCP secret that contains the token. Must be in the format `projects/<project-id>/secrets/<secret-name>/versions/<version-id>`." + "description": "The path to the GCP secret that contains the token. Must be in the format `projects/<project-id>/secrets/<secret-name>/versions/<version-id>`.", + "pattern": "^projects/[^/]+/secrets/[^/]+/versions/[^/]+$" } }, "required": [ "gcpSecretPath" ], "additionalProperties": false }packages/schemas/src/v3/bitbucket.schema.ts (1)
31-43: Consider adding pattern validation for gcpSecretPath format.Similar to the shared schema, this gcpSecretPath definition lacks format validation. Consider adding a
patternproperty to enforce the required GCP secret path format.packages/crypto/src/tokenUtils.ts (1)
13-13: Consider reusing SecretManagerServiceClient instance for better performance.Creating a new
SecretManagerServiceClientinstance for each token retrieval may impact performance, especially if tokens are retrieved frequently. Consider creating a singleton instance or accepting it as a parameter.Example approach:
let secretManagerClient: SecretManagerServiceClient | null = null; const getSecretManagerClient = () => { if (!secretManagerClient) { secretManagerClient = new SecretManagerServiceClient(); } return secretManagerClient; }; export const getTokenFromConfig = async (token: Token): Promise<string> => { // ... existing env handling ... } else if ('gcpSecretPath' in token) { const client = getSecretManagerClient(); // ... rest of the code ... } };packages/schemas/src/v3/app.schema.ts (1)
42-55: Consider adding pattern validation for gcpSecretPath format.The
privateKey.gcpSecretPathfield lacks format validation. Consider adding apatternproperty to validate the GCP secret path format, consistent with the recommendation for other schema files.docs/snippets/schemas/v3/github.schema.mdx (1)
28-40: Consider adding pattern validation for gcpSecretPath format.Similar to the schema files, this documentation should reflect pattern validation for the GCP secret path format if added to the source schemas.
docs/snippets/schemas/v3/gitlab.schema.mdx (1)
28-40: Consider adding pattern validation for gcpSecretPath format.Consistent with other schema documentation, consider reflecting pattern validation for the GCP secret path format.
docs/snippets/schemas/v3/bitbucket.schema.mdx (1)
32-44: Consider adding pattern validation for gcpSecretPath format.Consistent with recommendations for other schema files, consider adding pattern validation for the GCP secret path format.
packages/schemas/src/v3/index.schema.ts (2)
299-328: Consider oneOf instead of anyOf for secret refs.Shapes are mutually exclusive (additionalProperties: false), but oneOf yields clearer validation errors (“exactly one schema must match”). Change in the generator to avoid touching every site manually.
Also applies to: 506-536
315-325: Add validation (pattern, minLength, examples) to gcpSecretPath in source schema.Fix
schemas/v3/shared.jsonline 23-29 (Token definition). This single source definition propagates to 80+ occurrences in the generatedpackages/schemas/src/v3/index.schema.tsvia the generator.Apply diff to
schemas/v3/shared.json:"gcpSecretPath": { "type": "string", + "minLength": 1, + "pattern": "^projects\\/(?:[a-z][a-z0-9\\-]{4,28}[a-z0-9]|\\d+)\\/secrets\\/[A-Za-z0-9-_]{1,255}\\/versions\\/(?:latest|\\d+)$", - "description": "The path to the GCP secret that contains the token. Must be in the format `projects/<project-id>/secrets/<secret-name>/versions/<version-id>`." + "description": "GCP Secret Manager version resource path: projects/<project-id|number>/secrets/<secret-id>/versions/<version-id|latest>.", + "examples": ["projects/acme-prod/secrets/GITHUB_TOKEN/versions/latest"] }Run
yarn generateinpackages/schemas/to regenerate and propagate to all downstream occurrences.docs/snippets/schemas/v3/app.schema.mdx (1)
45-56: Doc schema: add gcpSecretPath regex, minLength, example (keep generator in sync).Prevents misconfig in docs-driven validation and keeps UX consistent with code schema.
- "gcpSecretPath": { - "type": "string", - "description": "The path to the GCP secret that contains the token. Must be in the format `projects/<project-id>/secrets/<secret-name>/versions/<version-id>`." - } + "gcpSecretPath": { + "type": "string", + "minLength": 1, + "pattern": "^projects\\/(?:[a-z][a-z0-9\\-]{4,28}[a-z0-9]|\\d+)\\/secrets\\/[A-Za-z0-9-_]{1,255}\\/versions\\/(?:latest|\\d+)$", + "description": "GCP Secret Manager version resource path.", + "examples": ["projects/acme-prod/secrets/GITHUB_APP_PRIVATE_KEY/versions/latest"] + }docs/snippets/schemas/v3/languageModel.schema.mdx (1)
39-50: Add regex, minLength, and examples for gcpSecretPath to ensure consistent validation.The canonical GCP Secret Manager format is projects/{PROJECT_ID}/secrets/{SECRET_ID}/versions/{VERSION_ID}, with "latest" as a valid alias for VERSION_ID. Your suggested pattern aligns with this specification.
- "gcpSecretPath": { - "type": "string", - "description": "The path to the GCP secret that contains the token. Must be in the format `projects/<project-id>/secrets/<secret-name>/versions/<version-id>`." - } + "gcpSecretPath": { + "type": "string", + "minLength": 1, + "pattern": "^projects\\/(?:[a-z][a-z0-9\\-]{4,28}[a-z0-9]|\\d+)\\/secrets\\/[A-Za-z0-9-_]{1,255}\\/versions\\/(?:latest|\\d+)$", + "description": "GCP Secret Manager version resource path.", + "examples": ["projects/acme-prod/secrets/ANTHROPIC_API_KEY/versions/latest"] + }Apply the same update to lines 516-541 and 856-883 for consistency.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (34)
docs/snippets/schemas/v3/app.schema.mdx(2 hunks)docs/snippets/schemas/v3/azuredevops.schema.mdx(1 hunks)docs/snippets/schemas/v3/bitbucket.schema.mdx(1 hunks)docs/snippets/schemas/v3/connection.schema.mdx(5 hunks)docs/snippets/schemas/v3/gitea.schema.mdx(1 hunks)docs/snippets/schemas/v3/github.schema.mdx(1 hunks)docs/snippets/schemas/v3/gitlab.schema.mdx(1 hunks)docs/snippets/schemas/v3/index.schema.mdx(61 hunks)docs/snippets/schemas/v3/languageModel.schema.mdx(54 hunks)docs/snippets/schemas/v3/shared.schema.mdx(3 hunks)packages/crypto/package.json(1 hunks)packages/crypto/src/tokenUtils.ts(1 hunks)packages/schemas/src/v3/app.schema.ts(2 hunks)packages/schemas/src/v3/app.type.ts(1 hunks)packages/schemas/src/v3/azuredevops.schema.ts(1 hunks)packages/schemas/src/v3/azuredevops.type.ts(1 hunks)packages/schemas/src/v3/bitbucket.schema.ts(1 hunks)packages/schemas/src/v3/bitbucket.type.ts(1 hunks)packages/schemas/src/v3/connection.schema.ts(5 hunks)packages/schemas/src/v3/connection.type.ts(5 hunks)packages/schemas/src/v3/gitea.schema.ts(1 hunks)packages/schemas/src/v3/gitea.type.ts(1 hunks)packages/schemas/src/v3/github.schema.ts(1 hunks)packages/schemas/src/v3/github.type.ts(1 hunks)packages/schemas/src/v3/gitlab.schema.ts(1 hunks)packages/schemas/src/v3/gitlab.type.ts(1 hunks)packages/schemas/src/v3/index.schema.ts(61 hunks)packages/schemas/src/v3/index.type.ts(20 hunks)packages/schemas/src/v3/languageModel.schema.ts(54 hunks)packages/schemas/src/v3/languageModel.type.ts(14 hunks)packages/schemas/src/v3/shared.schema.ts(3 hunks)packages/schemas/src/v3/shared.type.ts(1 hunks)packages/web/src/actions.ts(1 hunks)schemas/v3/shared.json(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*
📄 CodeRabbit inference engine (.cursor/rules/style.mdc)
Filenames should always be camelCase. Exception: if there are filenames in the same directory with a format other than camelCase, use that format to keep things consistent.
Files:
packages/crypto/package.jsonpackages/schemas/src/v3/bitbucket.type.tsschemas/v3/shared.jsonpackages/schemas/src/v3/azuredevops.type.tspackages/schemas/src/v3/gitlab.schema.tspackages/schemas/src/v3/shared.type.tsdocs/snippets/schemas/v3/github.schema.mdxpackages/schemas/src/v3/azuredevops.schema.tsdocs/snippets/schemas/v3/azuredevops.schema.mdxpackages/schemas/src/v3/github.schema.tspackages/schemas/src/v3/connection.type.tsdocs/snippets/schemas/v3/gitea.schema.mdxpackages/crypto/src/tokenUtils.tsdocs/snippets/schemas/v3/languageModel.schema.mdxpackages/schemas/src/v3/languageModel.type.tspackages/web/src/actions.tspackages/schemas/src/v3/gitea.type.tsdocs/snippets/schemas/v3/connection.schema.mdxdocs/snippets/schemas/v3/shared.schema.mdxpackages/schemas/src/v3/shared.schema.tspackages/schemas/src/v3/bitbucket.schema.tsdocs/snippets/schemas/v3/index.schema.mdxpackages/schemas/src/v3/github.type.tsdocs/snippets/schemas/v3/bitbucket.schema.mdxpackages/schemas/src/v3/app.type.tspackages/schemas/src/v3/index.schema.tspackages/schemas/src/v3/gitlab.type.tspackages/schemas/src/v3/gitea.schema.tspackages/schemas/src/v3/languageModel.schema.tspackages/schemas/src/v3/app.schema.tsdocs/snippets/schemas/v3/gitlab.schema.mdxpackages/schemas/src/v3/index.type.tsdocs/snippets/schemas/v3/app.schema.mdxpackages/schemas/src/v3/connection.schema.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (25)
packages/web/src/actions.ts (1)
7-12: LGTM! Clean removal of unused imports.The removal of
secretAlreadyExistsandencryptfrom the imports is appropriate since neither symbol is referenced anywhere in this file. This cleanup aligns with the PR's refactoring to support GCP Secret Manager for token storage.packages/schemas/src/v3/app.type.ts (1)
21-33: Private key config union looks correctEnv-based private keys keep working, and the new gcpSecretPath branch matches the broader secret-manager flow without altering the surrounding schema.
packages/schemas/src/v3/languageModel.type.ts (1)
33-549: Token unions consistently expandedEvery credential/token field now accepts either env or gcpSecretPath, mirroring the shared Token shape and preserving the optional semantics across providers.
packages/crypto/package.json (1)
11-15: Secret Manager dependency is appropriatePulling in @google-cloud/secret-manager is necessary for the new gcpSecretPath retrieval logic in crypto; version looks good.
docs/snippets/schemas/v3/gitea.schema.mdx (1)
16-40: Snippet aligns with new token sourceThe JSON schema doc now advertises both env and gcpSecretPath options with matching descriptions, keeping docs in sync with the schema.
packages/schemas/src/v3/shared.type.ts (1)
7-19: Token type covers both sourcesCentral Token union now matches all per-provider usages, so consumers can rely on a single type for env or gcpSecretPath-backed credentials.
packages/schemas/src/v3/github.schema.ts (1)
19-39: GitHub token schema updated cleanlyAdding the gcpSecretPath alternative mirrors the shared Token definition and keeps the env description consistent with other providers.
packages/schemas/src/v3/shared.schema.ts (1)
11-157: Shared token schema stays in syncToken, LanguageModelHeaders, and LanguageModelQueryParams now all allow env or gcpSecretPath objects, keeping the shared schema aligned with the type definitions and provider-specific schemas.
packages/schemas/src/v3/languageModel.schema.ts (1)
1-2864: LGTM! Consistent schema extension across all language model providers.The auto-generated schema correctly extends token configuration to support GCP Secret Manager paths alongside environment variables. The pattern is consistently applied across all providers (AmazonBedrock, Anthropic, Azure, DeepSeek, Google, Mistral, OpenAI, OpenRouter, Xai) for token, credentials, headers, and queryParams fields.
Key observations:
- The
gcpSecretPathformat specification is correct for GCP Secret Manager- Schema structure properly uses
anyOffor the union typeadditionalProperties: falseensures strict validationpackages/schemas/src/v3/gitlab.schema.ts (1)
11-40: LGTM! Token schema correctly extended for GitLab.The token field now accepts either environment variable or GCP Secret Manager path. The schema structure is correct with proper
anyOf,required, andadditionalPropertiesconstraints.packages/schemas/src/v3/gitea.schema.ts (1)
11-40: LGTM! Token schema correctly extended for Gitea.The auto-generated schema properly adds GCP Secret Manager support while maintaining backward compatibility with environment variables.
packages/schemas/src/v3/azuredevops.schema.ts (1)
11-40: LGTM! Token schema correctly extended for Azure DevOps.The schema properly maintains the required token field while adding GCP Secret Manager support.
packages/schemas/src/v3/github.type.ts (1)
11-23: LGTM! TypeScript type correctly reflects schema union.The type definition properly translates the JSON Schema
anyOfto a TypeScript union type, maintaining full type safety for both token sources.packages/schemas/src/v3/azuredevops.type.ts (1)
11-23: LGTM! TypeScript type correctly reflects required token field.The union type properly enforces that token is required (not optional with
?) and supports both environment variable and GCP Secret Manager sources.packages/schemas/src/v3/gitea.type.ts (1)
11-23: LGTM! TypeScript type correctly models optional token union.The type definition properly reflects the schema with an optional token field supporting both token sources.
packages/schemas/src/v3/bitbucket.type.ts (1)
15-27: ✓ All verification requirements satisfied—code is production-ready.The schema generator produces consistent output, the runtime implementation in
packages/crypto/src/tokenUtils.tscorrectly handlesgcpSecretPathwith proper error handling, and@google-cloud/secret-manager@^6.1.1is already configured as a dependency.packages/schemas/src/v3/gitlab.type.ts (1)
11-23: LGTM! Token union type correctly defined.The union type for
tokenproperly supports both environment variable and GCP Secret Manager sources. The description clearly specifies the required format forgcpSecretPath.packages/crypto/src/tokenUtils.ts (1)
4-26: Verify GCP authentication configuration in deployment environments.The implementation relies on Google Cloud's Application Default Credentials (ADC) for authentication—the
SecretManagerServiceClient()is instantiated without explicit credential configuration. Ensure deployment environments have proper GCP credentials configured, either through:
- Service account keys
- Workload Identity (for GKE)
- Compute Engine default service account
- Local gcloud authentication (for development)
Additionally, verify that the service account has the necessary IAM permissions:
roles/secretmanager.secretAccessororsecretmanager.versions.accesspermissiondocs/snippets/schemas/v3/azuredevops.schema.mdx (1)
14-40: Auto-generated schema file—changes look consistent and correct.The token field updates in this provider schema properly add
gcpSecretPathas an alternative toenv. The format specification for GCP secret paths is clear and consistent with other provider schemas in the PR.docs/snippets/schemas/v3/index.schema.mdx (3)
307-307: GitHub provider token field correctly updated with gcpSecretPath option.The env and gcpSecretPath options are properly structured with consistent descriptions across all provider configurations in this comprehensive schema file.
Also applies to: 315-327
1436-1456: Language model credentials and headers consistently updated with dual token sources.Amazon Bedrock's
accessKeyId,accessKeySecret, andsessionTokenfields, as well as header definitions across all language model providers, properly include bothenvandgcpSecretPathoptions with clear format specifications.Also applies to: 1467-1487, 1498-1518, 1552-1572
1-1: Note: Auto-generated file requires no manual edits.This schema documentation file is marked as auto-generated and should be kept in sync with the upstream schema generation source. Verify that the source TypeScript schema files (e.g.,
packages/schemas/src/v3/*.schema.ts) have been updated to produce these changes, and consider whether those source files should be the primary focus of review.docs/snippets/schemas/v3/shared.schema.mdx (3)
7-36: Base Token definition correctly split into env and gcpSecretPath options.The shared Token schema properly defines both
envandgcpSecretPathas mutually exclusive options with appropriate descriptions and format guidance. This serves as the foundation for token handling across all providers and models.
78-121: LanguageModelHeaders properly support dual token sources.The nested anyOf structure for header values correctly allows both string literals and token references (env or gcpSecretPath), maintaining consistency with the core Token definition.
122-165: LanguageModelQueryParams schema correctly updated with token variant support.Query parameters follow the same dual token source pattern as headers, ensuring consistent secret management across all configuration contexts.
Adds support for google secrets manager for storing code host PAT tokens and language model keys.
Summary by CodeRabbit