-
Notifications
You must be signed in to change notification settings - Fork 30
improve secret management and deployment with scripts + gsm #1618
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
- Updated the migration script paths in the configuration to reflect the new directory structure. - Deleted the obsolete `migrate-customers-table.ts` script. - Introduced new migration scripts for project entity management and secret downloading. - Added npm scripts for downloading secrets and managing CI processes in the new `scripts/package.json`. - Enhanced the organization of migration files for better maintainability.
- Added steps to authenticate with Google Cloud and download secrets to a JSON file. - Implemented bulk upload of secrets to Cloudflare, improving the deployment process. - Updated the secret download script to target the production environment variables. These changes streamline the API deployment process by integrating secret management directly into the workflow.
WalkthroughThis PR refactors the CI/CD deployment workflow to centralize secrets management using Google Cloud Secret Manager. Migration scripts are reorganized into a new migrations directory, the root package.json now includes a scripts workspace, and new utility modules handle downloading and transforming secrets from GCP into local JSON files. Changes
Sequence Diagram(s)sequenceDiagram
participant GHA as GitHub Actions
participant GCP as Google Cloud<br/>Secret Manager
participant CF as Cloudflare
participant Deploy as Deployment
Note over GHA: New Deployment Flow
GHA->>GCP: 1. Authenticate via service account
GHA->>GCP: 2. Download secrets (decocms-api-dev-vars-prod)
GCP-->>GHA: Returns secret content
GHA->>GHA: 3. Transform & write to .secrets.json
GHA->>CF: 4. Upload secrets to Cloudflare bulk
CF-->>GHA: Secrets stored
GHA->>Deploy: 5. Publish with environment config
Deploy-->>GHA: Deployment complete
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~25 minutes The changes introduce new Google Cloud Secret Manager integration and restructure the deployment workflow with medium complexity. While most individual file changes are straightforward (path updates, configuration additions), the interconnected nature of the secrets management system, workflow orchestration changes, and script reorganization require careful review to ensure proper integration and security practices. Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
Deploying chat with
|
| Latest commit: |
5b7a10d
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://9c684a19.chat-46r.pages.dev |
| Branch Preview URL: | https://secrets-dx.chat-46r.pages.dev |
- Updated the `downloadSecretToFile` function to enhance readability by adjusting the parameter formatting. - Ensured consistent formatting in the `gsm-download-secrets.ts` script for better maintainability. These changes improve code clarity and maintainability across secret management scripts.
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: 4
🧹 Nitpick comments (5)
scripts/package.json (1)
7-7: Consider using a stable version of drizzle-orm.The beta version
1.0.0-beta.1-ac4ce44may contain bugs or breaking changes. If you don't require specific beta features, consider upgrading to a stable release.scripts/secrets/core.ts (2)
5-5: Consider making PROJECT_ID configurable.The hardcoded
PROJECT_IDworks for this specific project but reduces reusability. If you anticipate needing this for multiple GCP projects, consider reading from an environment variable with a fallback.Example approach:
const PROJECT_ID = process.env.GCP_PROJECT_ID || "decocms";
27-38: Enhance robustness of the env parser.The
envLikeToObjectfunction could benefit from:
- Trimming keys and values to handle whitespace
- Validating that lines contain an
=separatorApply this diff to improve robustness:
export function envLikeToObject(envLike: string) { const obj = {} as Record<string, string>; const lines = envLike.split("\n"); for (const line of lines) { - if (!line || line.startsWith("#")) { + const trimmedLine = line.trim(); + if (!trimmedLine || trimmedLine.startsWith("#")) { continue; } - const [key, ...value] = line.split("="); - obj[key] = value.join("="); + const equalIndex = trimmedLine.indexOf("="); + if (equalIndex === -1) { + console.warn(`Skipping malformed line: ${trimmedLine}`); + continue; + } + const key = trimmedLine.slice(0, equalIndex).trim(); + const value = trimmedLine.slice(equalIndex + 1).trim(); + obj[key] = value; } return obj; }scripts/secrets/gsm-download-secrets.ts (1)
3-4: Consider adding error handling for better debugging.While the underlying
downloadSecretToFilefunction will throw errors, wrapping these calls in a try-catch would provide clearer context about which secret download failed during execution.Apply this diff to add error handling:
-await downloadSecretToFile("decocms-api-dev-vars", "../apps/api/.dev.vars"); -await downloadSecretToFile("decocms-web-env", "../apps/web/.env"); +try { + await downloadSecretToFile("decocms-api-dev-vars", "../apps/api/.dev.vars"); + await downloadSecretToFile("decocms-web-env", "../apps/web/.env"); +} catch (error) { + console.error("Failed to download secrets:", error); + process.exit(1); +}scripts/ci/gsm-download-to-secret-json.ts (1)
3-7: Consider adding error handling for CI reliability.In a CI context, clear error messages are crucial for debugging. Adding a try-catch block would provide better context if the secret download or transformation fails.
Apply this diff:
-await downloadSecretToFile( - "decocms-api-dev-vars-prod", - "../.secrets.json", - (content) => JSON.stringify(envLikeToObject(content)), -); +try { + await downloadSecretToFile( + "decocms-api-dev-vars-prod", + "../.secrets.json", + (content) => JSON.stringify(envLikeToObject(content)), + ); +} catch (error) { + console.error("Failed to download and transform secrets:", error); + process.exit(1); +}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
bun.lockbis excluded by!**/bun.lockb
📒 Files selected for processing (9)
.github/workflows/deploy-api.yaml(1 hunks).oxlintrc.json(1 hunks)package.json(2 hunks)scripts/ci/gsm-download-to-secret-json.ts(1 hunks)scripts/migrate-customers-table.ts(0 hunks)scripts/migrations/migrate-project-entity-table.ts(1 hunks)scripts/package.json(1 hunks)scripts/secrets/core.ts(1 hunks)scripts/secrets/gsm-download-secrets.ts(1 hunks)
💤 Files with no reviewable changes (1)
- scripts/migrate-customers-table.ts
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/data-flow.mdc)
**/*.{ts,tsx}: MCP tools must use Zod schemas for input and, when applicable, output validation
Register tools with server.registerTool providing description, inputSchema.shape, and optional outputSchema.shape
In every MCP tool handler, perform authorization checks first and then call context.resourceAccess.grant() before business logic
Name tools using the {RESOURCE}_{ACTION} pattern (e.g., AGENTS_CREATE, THREADS_LIST)
Group related tools into typed collections (e.g., GLOBAL_TOOLS, WORKSPACE_TOOLS) and export them as const
Always check workspace/team access before executing operations that touch workspace resources
Return proper authorization errors and handle forbidden operations explicitlyPrefer specific TypeScript types over any
**/*.{ts,tsx}: Write concise, maintainable, and technically accurate TypeScript code
Use functional and declarative programming patterns; avoid classes
Favor iteration and modularization to follow DRY and avoid code duplication
Use descriptive variable names with auxiliary verbs (e.g., isLoading, hasError)
Organize files so each contains only related content (components, subcomponents, helpers, static content, types)
Prefer interfaces over types for object shapes
Avoid enums; use maps (objects/records) instead
Use the function keyword for pure functions to benefit from hoisting and clarity
Ensure dependency arrays use stable references; do not inline new objects/arrays/functions
Memoize options/objects used in hooks (e.g., useMemo for options passed to useMemo/useCallback)
Favor named exports for functions
Files:
scripts/secrets/gsm-download-secrets.tsscripts/secrets/core.tsscripts/ci/gsm-download-to-secret-json.tsscripts/migrations/migrate-project-entity-table.ts
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/data-flow.mdc)
Log authorization failures to aid debugging and auditing
Files:
scripts/secrets/gsm-download-secrets.tsscripts/secrets/core.tsscripts/ci/gsm-download-to-secret-json.tsscripts/migrations/migrate-project-entity-table.ts
scripts/**
📄 CodeRabbit inference engine (AGENTS.md)
Keep infrastructure helper scripts under scripts/
Files:
scripts/secrets/gsm-download-secrets.tsscripts/secrets/core.tsscripts/ci/gsm-download-to-secret-json.tsscripts/migrations/migrate-project-entity-table.tsscripts/package.json
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: Biome formatting: use two-space indentation and double quotes
Keep imports sorted
Name hooks and utility functions using camelCase
Files:
scripts/secrets/gsm-download-secrets.tsscripts/secrets/core.tsscripts/ci/gsm-download-to-secret-json.tsscripts/migrations/migrate-project-entity-table.ts
{apps/api,scripts}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use explicit file extensions in imports where Deno requires them
Files:
scripts/secrets/gsm-download-secrets.tsscripts/secrets/core.tsscripts/ci/gsm-download-to-secret-json.tsscripts/migrations/migrate-project-entity-table.ts
🧠 Learnings (2)
📚 Learning: 2025-09-26T00:32:16.621Z
Learnt from: CR
PR: deco-cx/chat#0
File: .cursor/rules/structure.mdc:0-0
Timestamp: 2025-09-26T00:32:16.621Z
Learning: Applies to {apps/web,packages/{sdk,ui}}/package.json : Maintain a package.json in each workspace (apps/web, packages/sdk, packages/ui)
Applied to files:
package.json
📚 Learning: 2025-09-26T00:32:16.621Z
Learnt from: CR
PR: deco-cx/chat#0
File: .cursor/rules/structure.mdc:0-0
Timestamp: 2025-09-26T00:32:16.621Z
Learning: Use Bun for dependency management and scripts (bun install, bun run fmt, bun run check, bun run lint, bun dev)
Applied to files:
package.json
🧬 Code graph analysis (2)
scripts/secrets/gsm-download-secrets.ts (1)
scripts/secrets/core.ts (1)
downloadSecretToFile(17-25)
scripts/ci/gsm-download-to-secret-json.ts (1)
scripts/secrets/core.ts (2)
downloadSecretToFile(17-25)envLikeToObject(27-38)
🔇 Additional comments (5)
.oxlintrc.json (1)
4-4: LGTM!The ignore pattern update correctly reflects the migration script's new location under
scripts/migrations/.scripts/migrations/migrate-project-entity-table.ts (1)
1-14: LGTM! Import updates align with file relocation.The import path adjustments (from
../to../../) correctly reflect the script's move into themigrations/subdirectory. The switch to a type-only import forPgTableWithColumnsfollows TypeScript best practices.package.json (1)
18-20: LGTM! Workspace setup follows best practices.The addition of the
scriptsworkspace and delegation scripts properly centralizes script execution using Bun, consistent with the repository's established patterns.Also applies to: 31-31
.github/workflows/deploy-api.yaml (2)
35-37: Verify the GOOGLE_APPLICATION_CREDENTIALS path.The
google-github-actions/auth@v2action doesn't write credentials to/tmp/gcp-key.jsonby default. The action automatically sets up Application Default Credentials (ADC) that the Google Cloud client libraries will discover.The
GOOGLE_APPLICATION_CREDENTIALSenvironment variable might not be necessary here, as the@google-cloud/secret-managerclient in your script will automatically use the ADC set up by the auth action.If you need to explicitly set this variable, verify the correct path by checking the auth action's output or consider removing these lines:
- name: Download secrets to .secrets.json run: bun run ci:download-secrets - env: - # this env is available for gcloud auth inside your script if needed - GOOGLE_APPLICATION_CREDENTIALS: /tmp/gcp-key.json
24-26: LGTM! Workflow improvements enhance maintainability.The simplified
bun installcommand and the shift from individual secret environment variables to bulk upload via Google Secret Manager significantly improves the workflow's maintainability and security posture.Also applies to: 46-52
| - name: Download secrets to .secrets.json | ||
| run: bun run ci:download-secrets | ||
| env: | ||
| # this env is available for gcloud auth inside your script if needed | ||
| GOOGLE_APPLICATION_CREDENTIALS: /tmp/gcp-key.json | ||
|
|
||
| - name: Upload secrets to Cloudflare in bulk | ||
| working-directory: ./apps/api | ||
| run: npx wrangler secret bulk .secrets.json | ||
| env: | ||
| CLOUDFLARE_API_TOKEN: ${{ secrets.CF_API_TOKEN }} | ||
| CLOUDFLARE_ACCOUNT_ID: ${{ secrets.CF_ACCOUNT_ID }} |
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.
Critical path mismatch: Secret file location inconsistency.
Line 34 runs ci:download-secrets from the repository root, which writes to ../.secrets.json (i.e., .secrets.json in the repo root based on scripts/ci/gsm-download-to-secret-json.ts line 5).
However, line 41 runs npx wrangler secret bulk .secrets.json with working-directory: ./apps/api, which looks for the file at ./apps/api/.secrets.json.
Apply this diff to fix the path:
- name: Upload secrets to Cloudflare in bulk
working-directory: ./apps/api
- run: npx wrangler secret bulk .secrets.json
+ run: npx wrangler secret bulk ../../.secrets.json
env:
CLOUDFLARE_API_TOKEN: ${{ secrets.CF_API_TOKEN }}
CLOUDFLARE_ACCOUNT_ID: ${{ secrets.CF_ACCOUNT_ID }}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - name: Download secrets to .secrets.json | |
| run: bun run ci:download-secrets | |
| env: | |
| # this env is available for gcloud auth inside your script if needed | |
| GOOGLE_APPLICATION_CREDENTIALS: /tmp/gcp-key.json | |
| - name: Upload secrets to Cloudflare in bulk | |
| working-directory: ./apps/api | |
| run: npx wrangler secret bulk .secrets.json | |
| env: | |
| CLOUDFLARE_API_TOKEN: ${{ secrets.CF_API_TOKEN }} | |
| CLOUDFLARE_ACCOUNT_ID: ${{ secrets.CF_ACCOUNT_ID }} | |
| - name: Download secrets to .secrets.json | |
| run: bun run ci:download-secrets | |
| env: | |
| # this env is available for gcloud auth inside your script if needed | |
| GOOGLE_APPLICATION_CREDENTIALS: /tmp/gcp-key.json | |
| - name: Upload secrets to Cloudflare in bulk | |
| working-directory: ./apps/api | |
| run: npx wrangler secret bulk ../../.secrets.json | |
| env: | |
| CLOUDFLARE_API_TOKEN: ${{ secrets.CF_API_TOKEN }} | |
| CLOUDFLARE_ACCOUNT_ID: ${{ secrets.CF_ACCOUNT_ID }} |
🤖 Prompt for AI Agents
In .github/workflows/deploy-api.yaml around lines 33 to 44, the workflow
downloads .secrets.json into the repository root but then runs `npx wrangler
secret bulk .secrets.json` from working-directory ./apps/api so the file is not
found; update the wrangler step to point to the repo-root file (e.g., run `npx
wrangler secret bulk ../.secrets.json` from ./apps/api) or remove the
working-directory so the command runs from the repo root—ensure the path used by
wrangler matches where ci:download-secrets writes the file.
| - name: Publish default apps | ||
| if: github.event_name == 'push' && github.ref == 'refs/heads/main' | ||
| run: bun run ./scripts/publish.ts | ||
| run: bun run ci:publish-default-apps | ||
| env: | ||
| DECO_TOKEN: ${{ secrets.DECO_TOKEN }} | ||
| PROJECT: /deco/default |
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.
🧩 Analysis chain
Verify the publish-default-apps script implementation.
The workflow references ci:publish-default-apps (which runs scripts/ci/publish-default-apps.ts), but this file is not included in the PR for review. Ensure that:
- The script exists and is functional
- It properly uses the
DECO_TOKENandPROJECTenvironment variables - It includes appropriate error handling for CI reliability
Run this script to verify the file exists and examine its implementation:
🏁 Script executed:
#!/bin/bash
# Check if publish-default-apps.ts exists and show its content
if fd -t f "publish-default-apps.ts" scripts/ci/; then
cat scripts/ci/publish-default-apps.ts
else
echo "Error: scripts/ci/publish-default-apps.ts not found"
exit 1
fiLength of output: 4648
Fix incorrect error message in publish-default-apps.ts line 62.
The script properly implements the workflow requirements and uses both DECO_TOKEN and PROJECT environment variables correctly with appropriate error handling. However, there is a critical bug:
Line 62 throws an error message saying "WORKSPACE environment variable is required" when it should say "PROJECT environment variable is required" (the variable being checked is process.env.PROJECT, not WORKSPACE).
🤖 Prompt for AI Agents
In publish-default-apps.ts around line 62, the thrown error incorrectly says
"WORKSPACE environment variable is required" while the code is checking
process.env.PROJECT; update the thrown error/message to "PROJECT environment
variable is required" (or use the exact process.env key in the message) so the
error accurately reflects the missing environment variable being validated.
| "private": true, | ||
| "devDependencies": { | ||
| "@google-cloud/secret-manager": "^6.1.1", | ||
| "@types/bun": "latest", |
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.
Pin the @types/bun version for reproducible builds.
Using "latest" as a version specifier is non-deterministic and can lead to inconsistent builds across environments and over time.
Apply this diff to pin to a specific version:
- "@types/bun": "latest",
+ "@types/bun": "^1.1.0",(Adjust the version number to the current latest stable version you want to use.)
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "@types/bun": "latest", | |
| "@types/bun": "^1.1.0", |
🤖 Prompt for AI Agents
In scripts/package.json around line 6 the dependency "@types/bun" is pinned to
"latest", which is non-deterministic; update that entry to a specific version
string (for example "0.2.3" or whatever current stable you choose) to ensure
reproducible builds, then run your package manager's install/lock command (npm
install/yarn install/pnpm install) to update the lockfile so the exact version
is recorded.
| export async function downloadSecretToFile( | ||
| secretName: string, | ||
| filePath: string, | ||
| transform?: (secret: string) => string, | ||
| ) { | ||
| const secret = await readSecret(secretName); | ||
| writeFileSync(filePath, transform ? transform(secret) : secret); | ||
| console.log(`✅ Wrote ${secretName} → ${filePath}`); | ||
| } |
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.
🛠️ Refactor suggestion | 🟠 Major
Add error handling for file write operations.
The writeFileSync call on line 23 can throw errors (e.g., permission issues, disk full, invalid path) that would crash the script without a clear error message.
Apply this diff to add basic error handling:
export async function downloadSecretToFile(
secretName: string,
filePath: string,
transform?: (secret: string) => string,
) {
const secret = await readSecret(secretName);
- writeFileSync(filePath, transform ? transform(secret) : secret);
- console.log(`✅ Wrote ${secretName} → ${filePath}`);
+ try {
+ writeFileSync(filePath, transform ? transform(secret) : secret);
+ console.log(`✅ Wrote ${secretName} → ${filePath}`);
+ } catch (error) {
+ throw new Error(`Failed to write ${secretName} to ${filePath}: ${error}`);
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export async function downloadSecretToFile( | |
| secretName: string, | |
| filePath: string, | |
| transform?: (secret: string) => string, | |
| ) { | |
| const secret = await readSecret(secretName); | |
| writeFileSync(filePath, transform ? transform(secret) : secret); | |
| console.log(`✅ Wrote ${secretName} → ${filePath}`); | |
| } | |
| export async function downloadSecretToFile( | |
| secretName: string, | |
| filePath: string, | |
| transform?: (secret: string) => string, | |
| ) { | |
| const secret = await readSecret(secretName); | |
| try { | |
| writeFileSync(filePath, transform ? transform(secret) : secret); | |
| console.log(`✅ Wrote ${secretName} → ${filePath}`); | |
| } catch (error) { | |
| throw new Error(`Failed to write ${secretName} to ${filePath}: ${error}`); | |
| } | |
| } |
🤖 Prompt for AI Agents
In scripts/secrets/core.ts around lines 17 to 25, the writeFileSync call can
throw and currently will crash without a helpful message; wrap the writeFileSync
in try/catch, catch and log a clear error including secretName, filePath and the
caught error (use console.error or a logger), and rethrow or exit with a
non-zero code as appropriate for the script so callers see failure; ensure
transform is applied inside the try block and avoid swallowing the error.
Summary by CodeRabbit