Repository navigation
Harden Cloud VM ops workflows - #3196
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds manual GitHub Actions for Cloud VM DB migrations and smoke tests; new Cloud-VM CLI scripts (env audit, RDS IAM migration, project/env helpers, smoke test, preflight), package scripts, a helper to ensure private lease directories used by VM drivers, and updates to docs and Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant GHA as GitHub Actions
participant Vercel
participant AWS
participant RDS as Aurora RDS
User->>GHA: Trigger cloud-vm:migrate (staging/production)
GHA->>GHA: Checkout repo + run preflight (schema-only or full)
alt preflight passes
GHA->>Vercel: Pull target env
Vercel-->>GHA: Env vars
GHA->>AWS: Exchange OIDC token for temporary creds
AWS-->>GHA: Temporary AWS creds
GHA->>AWS: Request RDS IAM auth token
AWS-->>GHA: IAM token
GHA->>RDS: Connect with IAM token and run migrations
RDS-->>GHA: Migration result
GHA-->>User: Report success/failure
else preflight fails
GHA-->>User: Abort with error
end
sequenceDiagram
participant User
participant GHA as GitHub Actions
participant Vercel
participant API as Web API
participant Provider as VM Provider
User->>GHA: Trigger cloud-vm:smoke (target, optional create)
GHA->>Vercel: Pull target env
Vercel-->>GHA: Env vars
GHA->>GHA: Validate env keys
alt create requested
GHA->>API: POST /api/vm (create)
API->>Provider: Provision VM
Provider-->>API: VM details
API-->>GHA: VM created
GHA->>API: POST /api/vm/:id/attach-endpoint (expect websocket)
API-->>GHA: Attach result
GHA->>API: DELETE /api/vm/:id (cleanup)
API-->>GHA: Deleted
end
GHA->>API: GET /api/vm (auth/no-auth checks)
API-->>GHA: 401/200 responses
GHA-->>User: Output JSON summary
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 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)
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 |
Greptile SummaryThis PR moves Cloud VM migration, smoke, and env-audit operations into repo-owned scripts and backs them with three protected manual GitHub Actions workflows. The Confidence Score: 4/5Safe to merge; only P2 findings present, both are low-risk style/hardening notes. All findings are P2. The TOCTOU race in ensurePrivateDirectoryCommand is acknowledged in the PR description as a temporary workaround, and the risk is minimal in a single-user sandbox VM. The loadEnv inline-comment issue is theoretical given Vercel's controlled output format. web/services/vms/drivers/wsLease.ts — TOCTOU gap in directory permission setup for lease token files. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[workflow_dispatch] --> B{inputs.target}
B -->|staging| MS[migrate-staging\nenv: cloud-vm-staging]
B -->|production| PF[preflight]
PF --> MS
MS --> MP[migrate-production\nenv: cloud-vm-production\nrequires approval]
subgraph Smoke
S1[workflow_dispatch] --> S2{create_vm?}
S2 -->|false| S3[auth + list only]
S2 -->|true + confirm_production_create| S4[create → attach → destroy]
end
subgraph EnvAudit
E1[workflow_dispatch] --> E2[vercel env pull]
E2 --> E3{strict?}
E3 -->|missing required / forbidden present| E4[exit 1]
E3 -->|ok| E5[print JSON summary]
end
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
web/scripts/test-cloud-vm-ws-auth.ts (1)
557-560: Prefer reusing the shared lease-directory helper to avoid drift.This script redefines
ensurePrivateDirectoryCommandeven thoughweb/services/vms/drivers/wsLease.tsnow exports the same utility.♻️ Suggested deduplication
import { createHash, randomBytes } from "node:crypto"; import WebSocket from "ws"; import { Sandbox } from "e2b"; import { Freestyle } from "freestyle"; +import { ensurePrivateDirectoryCommand } from "../services/vms/drivers/wsLease"; @@ -function parentDirectory(path: string): string { - const index = path.lastIndexOf("/"); - return index > 0 ? path.slice(0, index) : "."; -} - -function ensurePrivateDirectoryCommand(filePath: string): string { - const directory = shellQuote(parentDirectory(filePath)); - return `mkdir -p ${directory} && chmod 700 ${directory}`; -}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/scripts/test-cloud-vm-ws-auth.ts` around lines 557 - 560, The function ensurePrivateDirectoryCommand is duplicated here; import and reuse the shared helper exported from web/services/vms/drivers/wsLease.ts instead of redefining it. Replace the local ensurePrivateDirectoryCommand usage with the imported helper (retain the same call sites), add the appropriate import for the helper from wsLease.ts, and remove the local function to avoid drift between implementations.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@web/scripts/cloud-vm/migrate-vercel-aurora-iam.mjs`:
- Around line 44-50: Validate env.PGPORT before calling new Pool: parse it to a
number, ensure it's a finite integer within TCP port range (1–65535) using
Number(...) and Number.isInteger/Number.isFinite (or similar), and throw or
return a clear error if invalid; then pass the validated numeric port into the
Pool config instead of Number(env.PGPORT). Reference the Pool creation and
env.PGPORT in your change and perform this check immediately before the const
pool = new Pool({ ... }) line.
In `@web/scripts/cloud-vm/projects.mjs`:
- Around line 83-104: resolveWebDir currently prefers a package.json in the
provided input and only falls back to input/web; change it to prefer the web/
package when present so the monorepo root doesn't shadow the app. In
resolveWebDir(input) first resolve input to an absolute path, then check
existsPackageJson(path.join(webDir, "web")) and, if true, set webDir =
path.join(webDir, "web"); otherwise fall back to checking
existsPackageJson(webDir) and error/exit as before; update callers like
parseWebDirAndTarget only if they assume the old ordering (they can keep using
resolveWebDir), and keep references to existsPackageJson, resolveWebDir, and
parseWebDirAndTarget to locate the changes.
---
Nitpick comments:
In `@web/scripts/test-cloud-vm-ws-auth.ts`:
- Around line 557-560: The function ensurePrivateDirectoryCommand is duplicated
here; import and reuse the shared helper exported from
web/services/vms/drivers/wsLease.ts instead of redefining it. Replace the local
ensurePrivateDirectoryCommand usage with the imported helper (retain the same
call sites), add the appropriate import for the helper from wsLease.ts, and
remove the local function to avoid drift between implementations.
🪄 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
Run ID: f5a34bf6-8f5e-4688-980f-82da8935fe03
📒 Files selected for processing (17)
.github/workflows/cloud-vm-env-audit.yml.github/workflows/cloud-vm-migrate.yml.github/workflows/cloud-vm-smoke.ymldocs/cloud-vm-backend-rollout-todo.mdweb/.env.exampleweb/package.jsonweb/scripts/build-cloud-vm-images.tsweb/scripts/cloud-vm/audit-vercel-env.mjsweb/scripts/cloud-vm/migrate-vercel-aurora-iam.mjsweb/scripts/cloud-vm/projects.mjsweb/scripts/cloud-vm/smoke-vm-api.mjsweb/scripts/cloud-vm/verify-migration-preflight.shweb/scripts/test-cloud-vm-ws-auth.tsweb/services/vms/README.mdweb/services/vms/drivers/e2b.tsweb/services/vms/drivers/freestyle.tsweb/services/vms/drivers/wsLease.ts
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
web/scripts/cloud-vm/projects.mjs (1)
95-99:⚠️ Potential issue | 🟠 MajorPrefer the
web/package when both locations containpackage.json.At Line 97,
resolveWebDir()only switches toweb/if the current directory does not havepackage.json. In this monorepo, invoking from repo root will incorrectly resolve to root instead ofweb/.♻️ Proposed fix
export function resolveWebDir(input) { let webDir = path.resolve(input); - if (!existsPackageJson(webDir) && existsPackageJson(path.join(webDir, "web"))) { - webDir = path.join(webDir, "web"); - } - if (!existsPackageJson(webDir)) { + const nestedWebDir = path.join(webDir, "web"); + if (existsPackageJson(nestedWebDir)) { + webDir = nestedWebDir; + } else if (!existsPackageJson(webDir)) { console.error("Could not find web/package.json. Pass the web directory as the first argument."); process.exit(2); } return webDir; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/scripts/cloud-vm/projects.mjs` around lines 95 - 99, The resolveWebDir function currently only switches into "web/" when the candidate directory lacks a package.json; change the logic to prefer the "web/" subdirectory whenever it contains a package.json (even if the parent also has one). Update the branch in resolveWebDir to check existsPackageJson(path.join(webDir, "web")) first and set webDir = path.join(webDir, "web") when true; keep using existsPackageJson(webDir) elsewhere for validation and ensure you only prefer "web/" when that check passes. This uses the existing existsPackageJson helper so no new helpers are required.
🧹 Nitpick comments (4)
web/scripts/cloud-vm/projects.mjs (1)
196-201: Trim process-sourced env values before returning them.At Line 199, raw
process.envvalues are forwarded unchanged. Whitespace/newline-padded secrets are a known source of auth/config failures; normalize here for parity with file-loaded env behavior.♻️ Proposed fix
function processEnvObject() { const env = {}; for (const [key, value] of Object.entries(process.env)) { - if (value !== undefined) env[key] = value; + if (value !== undefined) env[key] = typeof value === "string" ? value.trim() : value; } return env; }Based on learnings: Repo
manaflow-ai/cmuxapplies env sanitization at the source layer (trimEnv) to prevent trailing-whitespace/newline issues when validation paths are skipped.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/scripts/cloud-vm/projects.mjs` around lines 196 - 201, The processEnvObject function forwards raw process.env values; update it to normalize by trimming whitespace/newlines from each string value before returning to avoid padded secrets causing auth/config failures. In the processEnvObject function, when iterating over process.env entries (key, value) use value.trim() (guarding for undefined/null if needed) and assign the trimmed string into env[key]; preserve the existing check that skips undefined values so behavior remains equivalent except for trimming.web/scripts/cloud-vm/smoke-vm-api.mjs (2)
39-41: Defensively trim Stack credential env values before use.Line 39-41 uses raw env values. Trimming here avoids auth failures caused by trailing whitespace/newlines in managed env stores.
🧹 Proposed defensive trim
- const projectId = env.NEXT_PUBLIC_STACK_PROJECT_ID; - const publishableClientKey = env.NEXT_PUBLIC_STACK_PUBLISHABLE_CLIENT_KEY; - const secretServerKey = env.STACK_SECRET_SERVER_KEY; + const projectId = env.NEXT_PUBLIC_STACK_PROJECT_ID.trim(); + const publishableClientKey = env.NEXT_PUBLIC_STACK_PUBLISHABLE_CLIENT_KEY.trim(); + const secretServerKey = env.STACK_SECRET_SERVER_KEY.trim();Based on learnings: in this repo, source-layer env trimming was introduced to prevent newline-related auth token parsing failures from dashboard-managed env vars.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/scripts/cloud-vm/smoke-vm-api.mjs` around lines 39 - 41, The three Stack credential constants (projectId, publishableClientKey, secretServerKey) are assigned raw env values; defensively trim them before use to avoid trailing whitespace/newline auth failures by changing their initializers to call .trim() safely (e.g. use env.NEXT_PUBLIC_STACK_PROJECT_ID?.trim() etc.), ensuring you preserve undefined/null when the env var is missing and do not throw if the value is undefined.
139-139: Don’t silently swallow temporary user cleanup failures.Line 139 hides cleanup failures, making leaked smoke users hard to detect. Please at least log the error.
🧾 Proposed visibility improvement
} finally { - if (user) await user.delete().catch(() => undefined); + if (user) { + await user.delete().catch((err) => { + console.error( + `cleanup_delete_user_failed error=${err instanceof Error ? err.message : String(err)}`, + ); + }); + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/scripts/cloud-vm/smoke-vm-api.mjs` at line 139, The cleanup currently swallows errors from user.delete() which hides failures; replace the silent catch with explicit error handling for the user deletion (the expression user.delete())—either wrap await user.delete() in a try/catch or change the .catch(() => undefined) to .catch(err => { /* log error */ }); and log the error (e.g., console.error or the existing logger) with a clear message like "Failed to delete smoke user" plus the error object so cleanup failures are visible..github/workflows/cloud-vm-migrate.yml (1)
77-100: Consider deduplicating the AWS OIDC prep block.The staging and production credential-prep scripts are copy-pasted. Extracting to one script/composite action will reduce drift and future hardening misses.
Also applies to: 133-156
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/cloud-vm-migrate.yml around lines 77 - 100, Duplicate AWS OIDC preparation logic exists in the "Prepare AWS web identity credentials" step (and again at lines 133-156); extract this into a single reusable unit (either a shell script checked into the repo or a composite GitHub Action) that accepts inputs for AWS_ROLE_ARN, ACTIONS_ID_TOKEN_REQUEST_URL, ACTIONS_ID_TOKEN_REQUEST_TOKEN, AWS_REGION and the required PG* vars, performs the same token fetch (writing AWS_WEB_IDENTITY_TOKEN_FILE and AWS_ROLE_ARN/AWS_REGION to GITHUB_ENV), validates inputs, and runs aws sts get-caller-identity; replace both in-line blocks with calls to that script/action to avoid drift and ensure identical behavior across staging/production.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/cloud-vm-migrate.yml:
- Around line 37-38: The workflow uses inputs.ref as the checkout ref for jobs
that receive AWS/DB credentials (see use of inputs.ref vs github.ref); to fix,
stop using inputs.ref for any privileged jobs and instead hard-code or use
github.ref (or a validated/whitelisted ref) when calling actions/checkout in
those jobs (replace occurrences of ref: ${{ inputs.ref || github.ref }} with
ref: ${{ github.ref }} or add an explicit conditional that only allows
inputs.ref for non-privileged jobs); update every occurrence (including the
other instances noted) so credentials-only run steps never execute code from an
untrusted inputs.ref.
- Around line 56-65: Add CMUX_DB_DRIVER=aws-rds-iam to the env block for both
migration jobs so the runtime explicitly selects the AWS RDS IAM driver instead
of falling back; update the existing env entries that include AWS_ROLE_ARN,
AWS_REGION, PGHOST, PGPORT, PGUSER, PGDATABASE to include
CMUX_DB_DRIVER=aws-rds-iam in the same sections (the two migration job env
blocks referenced in the diff).
In `@web/scripts/cloud-vm/smoke-vm-api.mjs`:
- Around line 61-64: The outbound fetch calls (e.g., the unauth and authed
requests assigned to unauth and authed) lack timeouts and can hang; add a
reusable helper (e.g., fetchWithTimeout(url, options, timeoutMs)) that uses
AbortController to abort after a configurable timeout and replace all raw
fetch(...) usages in this module (including the calls that create unauth, authed
and the later fetches at the other locations mentioned) with
fetchWithTimeout(...) passing a sane default timeout value and preserving
headers/options (authHeaders) so each request is bounded.
---
Duplicate comments:
In `@web/scripts/cloud-vm/projects.mjs`:
- Around line 95-99: The resolveWebDir function currently only switches into
"web/" when the candidate directory lacks a package.json; change the logic to
prefer the "web/" subdirectory whenever it contains a package.json (even if the
parent also has one). Update the branch in resolveWebDir to check
existsPackageJson(path.join(webDir, "web")) first and set webDir =
path.join(webDir, "web") when true; keep using existsPackageJson(webDir)
elsewhere for validation and ensure you only prefer "web/" when that check
passes. This uses the existing existsPackageJson helper so no new helpers are
required.
---
Nitpick comments:
In @.github/workflows/cloud-vm-migrate.yml:
- Around line 77-100: Duplicate AWS OIDC preparation logic exists in the
"Prepare AWS web identity credentials" step (and again at lines 133-156);
extract this into a single reusable unit (either a shell script checked into the
repo or a composite GitHub Action) that accepts inputs for AWS_ROLE_ARN,
ACTIONS_ID_TOKEN_REQUEST_URL, ACTIONS_ID_TOKEN_REQUEST_TOKEN, AWS_REGION and the
required PG* vars, performs the same token fetch (writing
AWS_WEB_IDENTITY_TOKEN_FILE and AWS_ROLE_ARN/AWS_REGION to GITHUB_ENV),
validates inputs, and runs aws sts get-caller-identity; replace both in-line
blocks with calls to that script/action to avoid drift and ensure identical
behavior across staging/production.
In `@web/scripts/cloud-vm/projects.mjs`:
- Around line 196-201: The processEnvObject function forwards raw process.env
values; update it to normalize by trimming whitespace/newlines from each string
value before returning to avoid padded secrets causing auth/config failures. In
the processEnvObject function, when iterating over process.env entries (key,
value) use value.trim() (guarding for undefined/null if needed) and assign the
trimmed string into env[key]; preserve the existing check that skips undefined
values so behavior remains equivalent except for trimming.
In `@web/scripts/cloud-vm/smoke-vm-api.mjs`:
- Around line 39-41: The three Stack credential constants (projectId,
publishableClientKey, secretServerKey) are assigned raw env values; defensively
trim them before use to avoid trailing whitespace/newline auth failures by
changing their initializers to call .trim() safely (e.g. use
env.NEXT_PUBLIC_STACK_PROJECT_ID?.trim() etc.), ensuring you preserve
undefined/null when the env var is missing and do not throw if the value is
undefined.
- Line 139: The cleanup currently swallows errors from user.delete() which hides
failures; replace the silent catch with explicit error handling for the user
deletion (the expression user.delete())—either wrap await user.delete() in a
try/catch or change the .catch(() => undefined) to .catch(err => { /* log error
*/ }); and log the error (e.g., console.error or the existing logger) with a
clear message like "Failed to delete smoke user" plus the error object so
cleanup failures are visible.
🪄 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
Run ID: 34e9b8bb-e897-4010-a093-898fc03f44b1
📒 Files selected for processing (7)
.github/workflows/cloud-vm-migrate.yml.github/workflows/cloud-vm-smoke.ymldocs/cloud-vm-backend-rollout-todo.mdweb/scripts/cloud-vm/migrate-vercel-aurora-iam.mjsweb/scripts/cloud-vm/projects.mjsweb/scripts/cloud-vm/smoke-vm-api.mjsweb/services/vms/README.md
✅ Files skipped from review due to trivial changes (1)
- web/services/vms/README.md
🚧 Files skipped from review as they are similar to previous changes (3)
- .github/workflows/cloud-vm-smoke.yml
- web/scripts/cloud-vm/migrate-vercel-aurora-iam.mjs
- docs/cloud-vm-backend-rollout-todo.md
|
Warning Review the following alerts detected in dependencies. According to your organization's Security Policy, it is recommended to resolve "Warn" alerts. Learn more about Socket for GitHub.
|
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
web/scripts/cloud-vm/verify-migration-preflight.sh (1)
102-103: Separate declaration and assignment to avoid masking return values.Per shellcheck SC2155, combining
exportwith command substitution masks the exit code. Whilechoose_cmux_portexplicitly exits on failure (soset -ecatches it), splitting improves defensive coding.♻️ Suggested fix
-export CMUX_PORT="$(choose_cmux_port)" +CMUX_PORT="$(choose_cmux_port)" +export CMUX_PORT echo "using isolated CMUX_PORT=$CMUX_PORT for migration preflight"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/scripts/cloud-vm/verify-migration-preflight.sh` around lines 102 - 103, Split the combined export/command-substitution into separate assignment and export to avoid masking return values: call choose_cmux_port and assign its output to CMUX_PORT (CMUX_PORT="$(choose_cmux_port)"), then export CMUX_PORT (export CMUX_PORT) and keep the echo using the variable; locate the code in verify-migration-preflight.sh around where CMUX_PORT is set and referenced.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@web/scripts/cloud-vm/smoke-vm-api.mjs`:
- Around line 97-100: The code currently parses the create response into created
and sets vmId but never validates the provider; add a strict assertion that
created.provider === provider (or the CLI variable used to pass the requested
provider, e.g., opts.provider) and throw a descriptive Error if they differ
(e.g., `throw new Error("provider mismatch: requested X, got Y")`); apply the
same validation where the response is parsed/reported later (the
created/provider check around the 119-123 section) so the smoke flow fails if
the API rewrites/ignores the requested provider.
- Around line 148-151: Replace the synchronous hard exit call process.exit(1)
with setting process.exitCode = 1 so the event loop can drain and the awaited
cleanup in the finally block (including user.delete()) runs; locate the exit
invocation (process.exit(1)) and change it to assign process.exitCode = 1,
ensuring the finally block that awaits user.delete() is allowed to complete
before the process terminates.
---
Nitpick comments:
In `@web/scripts/cloud-vm/verify-migration-preflight.sh`:
- Around line 102-103: Split the combined export/command-substitution into
separate assignment and export to avoid masking return values: call
choose_cmux_port and assign its output to CMUX_PORT
(CMUX_PORT="$(choose_cmux_port)"), then export CMUX_PORT (export CMUX_PORT) and
keep the echo using the variable; locate the code in
verify-migration-preflight.sh around where CMUX_PORT is set and referenced.
🪄 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
Run ID: 14abb79c-b204-4b9a-8003-4cdd6b7ba535
📒 Files selected for processing (5)
.github/workflows/cloud-vm-migrate.ymlweb/scripts/cloud-vm/migrate-vercel-aurora-iam.mjsweb/scripts/cloud-vm/projects.mjsweb/scripts/cloud-vm/smoke-vm-api.mjsweb/scripts/cloud-vm/verify-migration-preflight.sh
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
web/scripts/cloud-vm/smoke-vm-api.mjs (1)
50-52: Normalize env values before constructing Stack auth clientLine 50-Line 52 pass raw env strings directly. Trim these at read time in this script too, so trailing whitespace/newlines in CI/env files don’t cause brittle auth failures.
♻️ Suggested hardening
+ const trimEnv = (value) => (typeof value === "string" ? value.trim() : value); - const projectId = env.NEXT_PUBLIC_STACK_PROJECT_ID; - const publishableClientKey = env.NEXT_PUBLIC_STACK_PUBLISHABLE_CLIENT_KEY; - const secretServerKey = env.STACK_SECRET_SERVER_KEY; + const projectId = trimEnv(env.NEXT_PUBLIC_STACK_PROJECT_ID); + const publishableClientKey = trimEnv(env.NEXT_PUBLIC_STACK_PUBLISHABLE_CLIENT_KEY); + const secretServerKey = trimEnv(env.STACK_SECRET_SERVER_KEY);Based on learnings: In
web/app/env.ts, env values are trimmed at the runtime source layer to prevent newline/whitespace-related auth parsing failures.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/scripts/cloud-vm/smoke-vm-api.mjs` around lines 50 - 52, Trim the raw environment variables when reading them in smoke-vm-api.mjs to avoid whitespace/newline issues: replace direct uses of env.NEXT_PUBLIC_STACK_PROJECT_ID, env.NEXT_PUBLIC_STACK_PUBLISHABLE_CLIENT_KEY, and env.STACK_SECRET_SERVER_KEY with their trimmed equivalents (e.g., String(...).trim()) before constructing the Stack auth client so projectId, publishableClientKey, and secretServerKey contain normalized values.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@web/scripts/cloud-vm/smoke-vm-api.mjs`:
- Line 153: The temp-user cleanup currently swallows errors by using
user.delete().catch(() => undefined); change this to catch and surface failures:
log the error (using the existing logger/processLogger) with context about
failing to delete the temp user, then rethrow the error (or otherwise cause the
run to be marked failed) so the failure is not reported as success; locate the
call to user.delete() and replace the silent catch with a handler that logs the
error and propagates it.
---
Nitpick comments:
In `@web/scripts/cloud-vm/smoke-vm-api.mjs`:
- Around line 50-52: Trim the raw environment variables when reading them in
smoke-vm-api.mjs to avoid whitespace/newline issues: replace direct uses of
env.NEXT_PUBLIC_STACK_PROJECT_ID, env.NEXT_PUBLIC_STACK_PUBLISHABLE_CLIENT_KEY,
and env.STACK_SECRET_SERVER_KEY with their trimmed equivalents (e.g.,
String(...).trim()) before constructing the Stack auth client so projectId,
publishableClientKey, and secretServerKey contain normalized values.
🪄 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
Run ID: d6ce0d07-55aa-4ce2-9c37-5f1e2f7fa6f5
📒 Files selected for processing (1)
web/scripts/cloud-vm/smoke-vm-api.mjs
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
🧹 Nitpick comments (1)
web/scripts/cloud-vm/smoke-vm-api.mjs (1)
44-52: Trim loaded env values beforeStackServerAppinitialization.At Line 50–52, values from
loadTargetEnv(project)are consumed raw. Trimming these three keys locally (or in the shared env loader) avoids newline/whitespace drift causing auth/parser failures.♻️ Proposed hardening patch
- const projectId = env.NEXT_PUBLIC_STACK_PROJECT_ID; - const publishableClientKey = env.NEXT_PUBLIC_STACK_PUBLISHABLE_CLIENT_KEY; - const secretServerKey = env.STACK_SECRET_SERVER_KEY; + const projectId = env.NEXT_PUBLIC_STACK_PROJECT_ID?.trim(); + const publishableClientKey = env.NEXT_PUBLIC_STACK_PUBLISHABLE_CLIENT_KEY?.trim(); + const secretServerKey = env.STACK_SECRET_SERVER_KEY?.trim(); + if (!projectId || !publishableClientKey || !secretServerKey) { + throw new Error(`${project.projectName} smoke has empty Stack env values after trim`); + }Based on learnings: In
web/app/env.ts, env sanitization is intentionally applied at the source layer via trimming to prevent newline/whitespace-caused failures.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/scripts/cloud-vm/smoke-vm-api.mjs` around lines 44 - 52, The loaded env values from loadTargetEnv(project) are used raw and may contain trailing/newline whitespace; before initializing StackServerApp trim the three keys (env.NEXT_PUBLIC_STACK_PROJECT_ID, env.NEXT_PUBLIC_STACK_PUBLISHABLE_CLIENT_KEY, env.STACK_SECRET_SERVER_KEY) or add trimming in the shared loader; specifically update the code that assigns projectId, publishableClientKey, and secretServerKey to use trimmed values (e.g., replace direct env reads with env.<KEY>?.trim()) or implement trimming inside loadTargetEnv so requireEnvKeys and subsequent code receive sanitized strings.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@web/scripts/cloud-vm/smoke-vm-api.mjs`:
- Around line 44-52: The loaded env values from loadTargetEnv(project) are used
raw and may contain trailing/newline whitespace; before initializing
StackServerApp trim the three keys (env.NEXT_PUBLIC_STACK_PROJECT_ID,
env.NEXT_PUBLIC_STACK_PUBLISHABLE_CLIENT_KEY, env.STACK_SECRET_SERVER_KEY) or
add trimming in the shared loader; specifically update the code that assigns
projectId, publishableClientKey, and secretServerKey to use trimmed values
(e.g., replace direct env reads with env.<KEY>?.trim()) or implement trimming
inside loadTargetEnv so requireEnvKeys and subsequent code receive sanitized
strings.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5c189518-92af-4577-b1b4-303fc5edab32
📒 Files selected for processing (1)
web/scripts/cloud-vm/smoke-vm-api.mjs
This branch was successfully deployed
1 participant
Summary
installin cloud VM lease setup because the active Freestyle snapshot has a brokeninstallbinaryVerification
node --check scripts/cloud-vm/projects.mjs && node --check scripts/cloud-vm/migrate-vercel-aurora-iam.mjs && node --check scripts/cloud-vm/smoke-vm-api.mjs && node --check scripts/cloud-vm/audit-vercel-env.mjsbun run cloud-vm:preflight -- --schema-only .bunx tsc --noEmitbun run cloud-vm:env:audit -- staging --strictbun run cloud-vm:env:audit -- production --strictBlockers found
VERCEL_TOKENandAWS_MIGRATION_ROLE_ARN--rpc-auth-lease-file, and new Freestyle snapshot creation currently returns providerINTERNAL_ERRORSummary by cubic
Hardened Cloud VM ops with repo-owned scripts and protected manual Actions. E2B is default and Freestyle is disabled; Actions use GitHub OIDC (no
VERCEL_TOKEN), lease writes avoidinstall, docs cover AWS IAM/env/runbooks, and the smoke script now reports user cleanup failures and cleans up test VMs more reliably.New Features
cloud-vm:env:audit,cloud-vm:migrate,cloud-vm:preflight,cloud-vm:smoke; env audit runs locally and flags required/recommended/forbidden/legacy keys; migration script generates RDS IAM auth tokens and applies Drizzle.Migration
cloud-vm-stagingandcloud-vm-production, set varsAWS_REGION,PGHOST,PGPORT,PGUSER,PGDATABASE,CMUX_DB_SSL_REJECT_UNAUTHORIZED,NEXT_PUBLIC_STACK_PROJECT_ID,NEXT_PUBLIC_STACK_PUBLISHABLE_CLIENT_KEY; add secretsSTACK_SECRET_SERVER_KEY(smoke) andAWS_MIGRATION_ROLE_ARN(migrate).OTEL_EXPORTER_OTLP_ENDPOINT,OTEL_EXPORTER_OTLP_HEADERS,OTEL_SERVICE_NAME; do not setCMUX_DB_SSL_CA_PEM(_BASE64)(use Node’s default trust store).bun run cloud-vm:preflight -- --schema-only ., then use the Actions to smoke and migrate (production waits for staging on the same ref). Keep Freestyle disabled until a new snapshot supports RPC lease and passes smoke.Written for commit 7d8a5f8. Summary will update on new commits. Review in cubic
Summary by CodeRabbit
New Features
Chores
Documentation