feat(build): add support for marking env vars as secrets in syncEnvVars #2417
Conversation
julienvanbeveren
commented
Aug 19, 2025
julienvanbeveren
commented
- Added isSecret flag to SyncEnvVarsBody type
- Updated CLI's syncEnvVarsWithServer to pass secret flags
- Modified backend API endpoint to handle secret flags
- Added documentation for the new feature
- Added isSecret flag to SyncEnvVarsBody type - Updated CLI's syncEnvVarsWithServer to pass secret flags - Modified backend API endpoint to handle secret flags - Added documentation for the new feature
🦋 Changeset detectedLatest commit: 4a13ef6 The changes in this PR will be included in the next version bump. This PR includes changesets to release 23 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
WalkthroughAdds per-variable secret support across sync/import flows. New changeset added. API schema and client types gain optional Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used📓 Path-based instructions (1)**/*.{ts,tsx}📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
🧬 Code graph analysis (1)packages/cli-v3/src/commands/deploy.ts (1)
🔇 Additional comments (2)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
.changeset/fresh-bats-travel.md (1)
1-7: Fix stray character and finalize the summary sentence.The trailing "7" looks accidental and could break the changeset parsing. Also, end the summary sentence with a period.
Apply this diff:
--- "trigger.dev": minor "@trigger.dev/build": minor --- -Added support for the secret flag on variables in the syncEnvVars extension +Added support for the secret flag on variables in the syncEnvVars extension. -7apps/webapp/app/routes/api.v1.projects.$projectRef.envvars.$slug.import.ts (1)
41-49: Avoid defaulting isSecret to false — this can unintentionally demote existing secrets.If the client omits
secretsfor a key, passingfalsehere will force a non-secret, potentially downgrading existing secret env vars. Prefer passingundefinedwhen the secret status is not explicitly provided, and let the repository preserve current state.Apply this diff:
variables: Object.entries(body.variables).map(([key, value]) => ({ key, value, - isSecret: body.secrets?.[key] ?? false, + isSecret: body.secrets ? body.secrets[key] : undefined, })),Also ensure
EnvironmentVariablesRepository.createtreatsisSecret: undefinedas “don’t change”.packages/build/src/extensions/core/syncEnvVars.ts (1)
117-125: Secrets collected are not propagated to the deploy layer — end-to-end secret marking will be lost.
callSyncEnvVarsFnnow preparessecrets/parentEnvSecrets, but they aren’t included incontext.addLayer. The CLI can’t forward them to the API unless they’re in the layer, so secret flags will silently drop.Apply this diff to include filtered secrets in the layer payload:
const env = stripUnsyncableEnvVars(result.env); const parentEnv = result.parentEnv ? stripUnsyncableEnvVars(result.parentEnv) : undefined; + const secrets = + result.secrets ? filterSecrets(result.secrets, env) : undefined; + const parentSecrets = + result.parentSecrets + ? filterSecrets(result.parentSecrets, parentEnv ?? {}) + : undefined; const numberOfEnvVars = Object.keys(env).length + (parentEnv ? Object.keys(parentEnv).length : 0); if (numberOfEnvVars === 0) { $spinner.stop("No env vars detected"); return; } else if (numberOfEnvVars === 1) { $spinner.stop(`Found 1 env var`); } else { $spinner.stop(`Found ${numberOfEnvVars} env vars to sync`); } context.addLayer({ id: "sync-env-vars", deploy: { env, parentEnv, + secrets, + parentSecrets, override: options?.override ?? true, }, });Add this helper to keep the secrets map aligned with the filtered env keys:
// Place near other helpers in this file function filterSecrets( secrets: Record<string, boolean>, env: Record<string, string> ): Record<string, boolean> | undefined { const filtered: Record<string, boolean> = {}; for (const key of Object.keys(env)) { if (secrets[key]) filtered[key] = true; } return Object.keys(filtered).length > 0 ? filtered : undefined; }Note: if the manifest/layer types don’t yet include
secrets/parentSecrets, update them and the CLI reader accordingly.packages/cli-v3/src/commands/deploy.ts (1)
352-358: Secrets flags are not forwarded to the server — pass them to syncEnvVarsWithServerThe new
secrets/parentEnvSecretsparameters are never passed here, so the API won’t receive the secret flags. This breaks the PR objective.Apply this diff to forward the flags gathered during build:
const uploadResult = await syncEnvVarsWithServer( projectClient.client, resolvedConfig.project, options.env, childVars, - parentVars + parentVars, + buildManifest.deploy.sync?.secrets, + buildManifest.deploy.sync?.parentEnvSecrets );
🧹 Nitpick comments (4)
packages/build/src/extensions/core/syncEnvVars.ts (2)
199-205: Only create the parent secrets map if needed; and rename toparentSecrets.Avoid emitting empty objects and keep naming consistent.
- if (!resolvedEnvVars.parentEnv) { - resolvedEnvVars.parentEnv = {}; - resolvedEnvVars.parentEnvSecrets = {}; - } + if (!resolvedEnvVars.parentEnv) { + resolvedEnvVars.parentEnv = {}; + } resolvedEnvVars.parentEnv[item.name] = item.value; - if (item.isSecret && resolvedEnvVars.parentEnvSecrets) { - resolvedEnvVars.parentEnvSecrets[item.name] = true; - } + if (item.isSecret) { + if (!resolvedEnvVars.parentSecrets) { + resolvedEnvVars.parentSecrets = {}; + } + resolvedEnvVars.parentSecrets[item.name] = true; + }
207-213: Nit: mirror lazy initialization pattern forsecrets.Current code is fine; mentioning parity with the parent branch after the rename above.
packages/cli-v3/src/commands/deploy.ts (2)
587-589: DRY the test URL: reuse rawTestLink to avoid duplicationYou’re recomputing the same test URL. Reuse
rawTestLinkto reduce duplication and prevent mismatches if the format changes.Apply this diff:
TRIGGER_DEPLOYMENT_URL: `${authorization.dashboardUrl}/projects/v3/${resolvedConfig.project}/deployments/${deployment.shortCode}`, - TRIGGER_TEST_URL: `${authorization.dashboardUrl}/projects/v3/${resolvedConfig.project - }/test?environment=${options.env === "prod" ? "prod" : "stg"}`, + TRIGGER_TEST_URL: rawTestLink,
595-596: Also DRY the test URL outputMirror the change for the GitHub Action outputs.
Apply this diff:
deploymentUrl: `${authorization.dashboardUrl}/projects/v3/${resolvedConfig.project}/deployments/${deployment.shortCode}`, - testUrl: `${authorization.dashboardUrl}/projects/v3/${resolvedConfig.project - }/test?environment=${options.env === "prod" ? "prod" : "stg"}`, + testUrl: rawTestLink,
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (6)
.changeset/fresh-bats-travel.md(1 hunks)apps/webapp/app/routes/api.v1.projects.$projectRef.envvars.$slug.import.ts(2 hunks)packages/build/src/extensions/core/syncEnvVars.ts(3 hunks)packages/cli-v3/src/commands/deploy.ts(5 hunks)packages/core/src/v3/apiClient/types.ts(1 hunks)packages/core/src/v3/schemas/api.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Always prefer using isomorphic code like fetch, ReadableStream, etc. instead of Node.js specific code
For TypeScript, we usually use types over interfaces
Avoid enums
No default exports, use function declarations
Files:
packages/core/src/v3/apiClient/types.tspackages/core/src/v3/schemas/api.tsapps/webapp/app/routes/api.v1.projects.$projectRef.envvars.$slug.import.tspackages/cli-v3/src/commands/deploy.tspackages/build/src/extensions/core/syncEnvVars.ts
{packages/core,apps/webapp}/**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
We use zod a lot in packages/core and in the webapp
Files:
packages/core/src/v3/apiClient/types.tspackages/core/src/v3/schemas/api.tsapps/webapp/app/routes/api.v1.projects.$projectRef.envvars.$slug.import.ts
apps/webapp/**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/webapp.mdc)
apps/webapp/**/*.{ts,tsx}: In the webapp, all environment variables must be accessed through theenvexport ofenv.server.ts, instead of directly accessingprocess.env.
When importing from@trigger.dev/corein the webapp, never import from the root@trigger.dev/corepath; always use one of the subpath exports as defined in the package's package.json.
Files:
apps/webapp/app/routes/api.v1.projects.$projectRef.envvars.$slug.import.ts
🧠 Learnings (1)
📚 Learning: 2025-08-18T10:07:17.345Z
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-08-18T10:07:17.345Z
Learning: Applies to trigger.config.ts : Provide a valid Trigger.dev configuration using defineConfig with project ref and dirs (e.g., ["./trigger"]; tests/specs auto-excluded)
Applied to files:
packages/cli-v3/src/commands/deploy.ts
🧬 Code Graph Analysis (1)
packages/cli-v3/src/commands/deploy.ts (1)
packages/cli-v3/src/utilities/cliOutput.ts (2)
isLinksSupported(7-7)chalkError(24-26)
🔇 Additional comments (8)
packages/core/src/v3/schemas/api.ts (1)
802-804: LGTM: schema additions for secret flags are correct.Adding
secretsandparentSecretsasz.record(z.boolean()).optional()aligns with the intended payload shape and keeps the body strict.packages/core/src/v3/apiClient/types.ts (1)
16-27: LGTM: params now support parent variables and per-key secret flags.The additions are well-documented and match the server schema.
packages/build/src/extensions/core/syncEnvVars.ts (1)
5-5: LGTM:isSecret?: booleanon array items is a clean, minimal extension.This keeps the record form backward-compatible and enables per-item secret flags when needed.
packages/cli-v3/src/commands/deploy.ts (5)
384-385: LGTM: test link constructionNo logic change; URL composition remains correct.
564-565: LGTM: consolidated outro with embedded linksThe final message is cleaner while preserving content.
608-610: LGTM: extended signature to accept secret flagsAdding
secretsandparentEnvSecretshere aligns the CLI surface with the backend API.
614-616: LGTM: request fields match API schemaForwarding
secretsandparentSecretsmatches the backend schema additions.
643-644: LGTM: error outro formatting improvementInline message reads better; no behavioral change.
apps/webapp/app/routes/api.v1.projects.$projectRef.envvars.$slug.import.ts
Show resolved
Hide resolved
1 participant
