local emulator security and features fixes#1247
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
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:
📝 WalkthroughWalkthroughThis PR expands the local emulator infrastructure with publishable client key support, enhances credential generation and validation, refactors the emulator provisioning pipeline with improved logging and health checks, adds mock service support (OAuth, Stripe, Freestyle), updates the CLI to support config-based credential injection, and introduces new QEMU boot testing capabilities. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
Contributor
Greptile SummaryThis PR extends the local emulator to support the full SDK hierarchy (client, server, and admin) by adding Key changes and findings:
Confidence Score: 2/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant App as SDK App (Client/Server/Admin)
participant CI as StackClientInterface
participant SI as StackServerInterface
participant BE as Local Emulator Backend
participant DB as Prisma DB
App->>CI: new StackClientInterface({ prepareRequest })
App->>App: _emulatorInitPromise = fetchEmulatorProjectCredentials()
App->>BE: POST /api/v1/internal/local-emulator/project
BE->>DB: findFirst(apiKeySet where secretServerKey!=null AND superSecretAdminKey!=null)
alt Key set found (may lack publishableClientKey — bug)
DB-->>BE: existing keySet
BE->>BE: assert publishableClientKey != null (throws if null)
else No key set found
DB-->>BE: null
BE->>DB: create apiKeySet (all 3 keys)
DB-->>BE: new keySet
end
BE-->>App: { project_id, publishable_client_key, secret_server_key, super_secret_admin_key }
App->>CI: _updateEmulatorCredentials(credentials)
App->>SI: _updateEmulatorCredentials(credentials)
Note over App,SI: Client requests: prepareRequest awaits _emulatorInitPromise ✓
Note over App,SI: Server/Admin requests: NO prepareRequest set — may race ✗
App->>CI: sendClientRequest (awaits prepareRequest → init promise)
App->>SI: sendServerRequest (NO prepareRequest → may use placeholder keys)
|
Contributor
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 (1)
apps/backend/src/app/api/latest/internal/local-emulator/project/route.tsx (1)
113-130:⚠️ Potential issue | 🟡 MinorPotential issue: Prisma query doesn't filter for
publishableClientKeyexistence.The query at lines 113-130 filters for
secretServerKey: { not: null }andsuperSecretAdminKey: { not: null }but notpublishableClientKey: { not: null }. If there are existing key sets in the database without apublishableClientKey, this could return them, and the assertion at line 144 would throw.Consider adding
publishableClientKey: { not: null }to the query filter for consistency:Proposed fix
const existingKeySet = await globalPrismaClient.apiKeySet.findFirst({ where: { projectId, manuallyRevokedAt: null, expiresAt: { gt: new Date(), }, + publishableClientKey: { + not: null, + }, secretServerKey: { not: null, }, superSecretAdminKey: { not: null, }, },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/backend/src/app/api/latest/internal/local-emulator/project/route.tsx` around lines 113 - 130, The Prisma query in globalPrismaClient.apiKeySet.findFirst (assigned to existingKeySet) filters for secretServerKey and superSecretAdminKey but not publishableClientKey, so it may return rows missing publishableClientKey and later cause the assertion at line 144 to fail; update the findFirst where clause to include publishableClientKey: { not: null } so only key sets that contain all three keys (secretServerKey, superSecretAdminKey, publishableClientKey) are returned by existingKeySet.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/stack-shared/src/interface/client-interface.ts`:
- Around line 53-70: The emulator override only updates
_projectIdOverride/_publishableClientKeyOverride via _updateEmulatorCredentials
but fetchNewAccessToken, getOAuthUrl, and callOAuthCallback still read
this.options.publishableClientKey (and possibly this.options.projectId), causing
inconsistent behavior; update those methods to read the effective accessors
(this.publishableClientKey and this.projectId) instead of directly using
this.options.* so the OAuth/token refresh flow honors the emulator overrides
from _updateEmulatorCredentials.
In `@packages/template/src/lib/stack-app/apps/implementations/admin-app-impl.ts`:
- Around line 154-164: AdminAppImpl (and similarly ServerAppImpl) sets
this._emulatorInitPromise but never wires a prepareRequest callback on the
created interface, allowing requests to race before emulator credentials load;
add a prepareRequest async callback when constructing/assigning this._interface
that awaits this._emulatorInitPromise if present (i.e., implement
prepareRequest: async () => { if (this._emulatorInitPromise) await
this._emulatorInitPromise; }) so any request via the interface waits for
_emulatorInitPromise to resolve before proceeding.
In `@packages/template/src/lib/stack-app/apps/implementations/client-app-impl.ts`:
- Around line 503-538: The emulator credentials are applied asynchronously after
the StackClientInterface is created, so synchronous methods that derive cookie
names (notably _getSession and _getOrCreateTokenStore) can use the pre-override
projectId and miss emulator sessions; fix by ensuring emulator credentials are
resolved before any code can synchronously read projectId or cookie names:
either await fetchEmulatorProjectCredentials(emulatorConfigFilePath) and apply
the returned project_id/publishable_client_key to projectId/publishableClientKey
before constructing StackClientInterface, or modify _getSession and
_getOrCreateTokenStore to await this._emulatorInitPromise (if present) before
deriving cookie names (and ensure any hook/path that calls them will suspend
while _emulatorInitPromise resolves); update references to prepareRequest only
for network protection and keep a single source of truth by applying the
emulator override prior to exposing the app interface.
In `@packages/template/src/lib/stack-app/apps/implementations/server-app-impl.ts`:
- Around line 420-438: The server interface is created immediately so initial
requests can run with pre-override keys; to fix, mirror the client-side behavior
by ensuring emulator credential fetch completes before the server request path
uses the interface: when isEmulator and no extraOptions.interface, set
this._emulatorInitPromise = fetchEmulatorProjectCredentials(...), then either
await that promise before constructing/assigning the StackServerInterface or
ensure this._interface’s request-building methods internally await
this._emulatorInitPromise (i.e., update server-app-impl.ts so the code around
StackServerInterface, this._interface and _emulatorInitPromise ensures the
credentials are applied before any inherited request code runs).
---
Outside diff comments:
In `@apps/backend/src/app/api/latest/internal/local-emulator/project/route.tsx`:
- Around line 113-130: The Prisma query in
globalPrismaClient.apiKeySet.findFirst (assigned to existingKeySet) filters for
secretServerKey and superSecretAdminKey but not publishableClientKey, so it may
return rows missing publishableClientKey and later cause the assertion at line
144 to fail; update the findFirst where clause to include publishableClientKey:
{ not: null } so only key sets that contain all three keys (secretServerKey,
superSecretAdminKey, publishableClientKey) are returned by existingKeySet.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f1b977d5-c56f-409f-a728-6a208f15751d
📒 Files selected for processing (11)
apps/backend/src/app/api/latest/internal/local-emulator/project/route.tsxapps/backend/src/lib/local-emulator.tsdocker/server/entrypoint.shpackages/stack-shared/src/interface/admin-interface.tspackages/stack-shared/src/interface/client-interface.tspackages/stack-shared/src/interface/server-interface.tspackages/template/src/lib/stack-app/apps/implementations/admin-app-impl.tspackages/template/src/lib/stack-app/apps/implementations/client-app-impl.tspackages/template/src/lib/stack-app/apps/implementations/common.tspackages/template/src/lib/stack-app/apps/implementations/server-app-impl.tspackages/template/src/lib/stack-app/apps/interfaces/client-app.ts
Add prepareRequest callbacks to StackServerInterface and StackAdminInterface that await _emulatorInitPromise, matching the existing pattern in the client app. Prevents race conditions where requests use placeholder credentials before the emulator credentials are fetched.
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/template/src/lib/stack-app/apps/implementations/server-app-impl.ts (1)
435-435: Consider replacing non-null assertion with explicit error.While the non-null assertion is logically safe here (the
isEmulatorguard on line 433 ensuresemulatorConfigFilePathis truthy), the coding guidelines prefer explicit error messages that state the assumption.💡 Suggested improvement
- this._emulatorInitPromise = fetchEmulatorProjectCredentials(emulatorConfigFilePath!).then((data) => { + this._emulatorInitPromise = fetchEmulatorProjectCredentials(emulatorConfigFilePath ?? throwErr("emulatorConfigFilePath must be defined when isEmulator is true")).then((data) => {As per coding guidelines: "Prefer
?? throwErr(...)over non-null assertions with good error messages explicitly stating the assumption."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/template/src/lib/stack-app/apps/implementations/server-app-impl.ts` at line 435, Replace the non-null assertion on emulatorConfigFilePath when initializing this._emulatorInitPromise with an explicit null-check that throws a clear error; specifically, change the call to fetchEmulatorProjectCredentials(emulatorConfigFilePath!) in the _emulatorInitPromise assignment to use emulatorConfigFilePath ?? throwErr("expected emulatorConfigFilePath to be set when isEmulator is true") (or your project's throw helper), keeping the surrounding isEmulator guard and the fetchEmulatorProjectCredentials call intact so the code fails with a descriptive message instead of relying on a non-null assertion.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/template/src/lib/stack-app/apps/implementations/server-app-impl.ts`:
- Line 435: Replace the non-null assertion on emulatorConfigFilePath when
initializing this._emulatorInitPromise with an explicit null-check that throws a
clear error; specifically, change the call to
fetchEmulatorProjectCredentials(emulatorConfigFilePath!) in the
_emulatorInitPromise assignment to use emulatorConfigFilePath ??
throwErr("expected emulatorConfigFilePath to be set when isEmulator is true")
(or your project's throw helper), keeping the surrounding isEmulator guard and
the fetchEmulatorProjectCredentials call intact so the code fails with a
descriptive message instead of relying on a non-null assertion.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b1420a7b-0c62-4608-9dff-9f228ac2008c
📒 Files selected for processing (2)
packages/template/src/lib/stack-app/apps/implementations/admin-app-impl.tspackages/template/src/lib/stack-app/apps/implementations/server-app-impl.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/template/src/lib/stack-app/apps/implementations/admin-app-impl.ts
Merge upstream envVars abstraction (replacing raw process.env reads) with local emulator support additions. Add new NEXT_PUBLIC_STACK_LOCAL_EMULATOR_CONFIG_FILE_PATH to envVars.
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/backend/src/app/api/latest/internal/local-emulator/project/route.tsx (1)
114-146:⚠️ Potential issue | 🟠 MajorAvoid selecting legacy key sets that cannot satisfy the new contract
Line 114 can still load active key sets without
publishableClientKey, and Line 145 then hard-fails. That breaks older emulator projects instead of rotating to a new valid key set.Suggested fix
const existingKeySet = await globalPrismaClient.apiKeySet.findFirst({ where: { projectId, manuallyRevokedAt: null, expiresAt: { gt: new Date(), }, + publishableClientKey: { + not: null, + }, secretServerKey: { not: null, }, superSecretAdminKey: { not: null, }, },Based on learnings: Adopt the convention
// TODO next-releasein this codebase for temporary backward-compat shims that only need to survive one release.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/backend/src/app/api/latest/internal/local-emulator/project/route.tsx` around lines 114 - 146, The find/create logic can return legacy key sets missing publishableClientKey/secretServerKey/superSecretAdminKey and later throws in the StackAssertionError; update the selection to prefer only key sets that have all three keys (modify the globalPrismaClient.apiKeySet.findFirst where clause to require publishableClientKey/secretServerKey/superSecretAdminKey not null) and if the found existingKeySet still lacks any required key, ignore it and force creation via globalPrismaClient.apiKeySet.create (or explicitly create a new key set when existingKeySet is falsy or incomplete); add a short inline comment marked "// TODO next-release" by this shim so it can be removed in the next release.
🧹 Nitpick comments (1)
apps/backend/src/app/api/latest/internal/local-emulator/project/route.tsx (1)
145-145: Use explicit null checks in the required-key assertionLine 145 uses truthiness checks. Please switch to explicit
== nullchecks to match repo standards and avoid conflating empty-string and nullish cases.Suggested fix
- if (!keySet.publishableClientKey || !keySet.secretServerKey || !keySet.superSecretAdminKey) { + if ( + keySet.publishableClientKey == null || + keySet.secretServerKey == null || + keySet.superSecretAdminKey == null + ) {As per coding guidelines: Prefer explicit null/undefinedness checks over boolean checks (e.g.,
foo == nullinstead of!foo).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/backend/src/app/api/latest/internal/local-emulator/project/route.tsx` at line 145, The truthiness check on keySet fields conflates empty strings with null/undefined; update the assertion in route.tsx to use explicit nullish checks for the three properties: replace the condition that references publishableClientKey, secretServerKey, and superSecretAdminKey with checks that each is == null (which catches null or undefined only) so the branch only triggers for missing keys, not falsy-but-present values.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@apps/backend/src/app/api/latest/internal/local-emulator/project/route.tsx`:
- Around line 114-146: The find/create logic can return legacy key sets missing
publishableClientKey/secretServerKey/superSecretAdminKey and later throws in the
StackAssertionError; update the selection to prefer only key sets that have all
three keys (modify the globalPrismaClient.apiKeySet.findFirst where clause to
require publishableClientKey/secretServerKey/superSecretAdminKey not null) and
if the found existingKeySet still lacks any required key, ignore it and force
creation via globalPrismaClient.apiKeySet.create (or explicitly create a new key
set when existingKeySet is falsy or incomplete); add a short inline comment
marked "// TODO next-release" by this shim so it can be removed in the next
release.
---
Nitpick comments:
In `@apps/backend/src/app/api/latest/internal/local-emulator/project/route.tsx`:
- Line 145: The truthiness check on keySet fields conflates empty strings with
null/undefined; update the assertion in route.tsx to use explicit nullish checks
for the three properties: replace the condition that references
publishableClientKey, secretServerKey, and superSecretAdminKey with checks that
each is == null (which catches null or undefined only) so the branch only
triggers for missing keys, not falsy-but-present values.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4823839f-2f09-4674-9563-f63f39d1530b
📒 Files selected for processing (2)
apps/backend/src/app/api/latest/internal/local-emulator/project/route.tsxapps/backend/src/lib/local-emulator.ts
✅ Files skipped from review due to trivial changes (1)
- apps/backend/src/lib/local-emulator.ts
Provisioning used to silently wait out the full 6000s timeout on any guest-side failure because the cleanup trap only logged the error. Now it writes STACK_CLOUD_INIT_FAILED and shuts the VM down, and the host waiter breaks on that marker and reports it distinctly. Also bump smoke test timeout 120s->300s, dump docker ps / container logs / free -m / verbose curl when it fails, log the qemu accel path, and enable /dev/kvm on the CI runner so the VM isn't stuck in TCG.
The arm64 matrix entry cross-compiles on the amd64 CI runner, so the guest runs under QEMU TCG. Under -cpu max, V8 emits armv8.5+ JIT code that TCG mistranslates and node crashes with SIGTRAP (exit 133) during migrations. Three changes together get it working: - Drop to -cpu cortex-a72 for TCG arm64 guests. Limits V8 to armv8.0-a which TCG handles cleanly. Native paths (HVF/KVM) keep -cpu max for full performance. - Run migrations with NODE_OPTIONS=--jitless as belt-and-suspenders. Migrations are I/O-bound so the perf hit is negligible. - Skip the in-guest smoke test on arm64. A full Next.js backend under cross-arch TCG either SIGTRAPs or times out; the amd64 build still runs the smoke test, which covers every non-arch-specific code path. Arch is propagated into the guest via a new build-arch.env marker in the stack-bundle ISO.
The previous commit set NODE_OPTIONS=--jitless on the migration docker exec. That was wrong for two reasons: - --jitless disables eval and new Function, which some code in the migration path uses, so it broke amd64 builds that had been passing. - --jitless is a V8 feature gate, not a TCG workaround. If it breaks one arch it breaks both — it could never have helped arm64 either. Revert the --jitless flag and rely on -cpu cortex-a72 (added in the parent commit) as the root-cause fix for the arm64 TCG SIGTRAP. Keep the stdout/stderr capture for the migration exec so the next failure dumps the actual node error through log-provision instead of being swallowed by the serial-only stream.
Cross-arch TCG on ubicloud-standard-8 either SIGTRAPs during migrations (old QEMU) or hangs in wait-for-deps with no progress. GitHub's ubuntu-24.04-arm runner is an Azure arm64 VM — same-arch TCG, no KVM (no nested virt exposed) — but empirically completes migrations, the dep setup, and image packaging end-to-end (verified on the diagnostics branch run). Only failure there was the backend smoke test hitting its 300s timeout, which the parent commit on this branch already skips on arm64. Keep amd64 on ubicloud-standard-8 for its KVM acceleration.
wait-for-deps used to loop forever on each service, so any single dep that failed to start (e.g. a service crash-looping under TCG) hung the build until the outer 6000s provision timeout. Rewrite as a wait_for helper with: - Hard 1500s budget across the full dep wait (overridable via STACK_DEPS_TIMEOUT). On timeout, dump docker ps -a, last 300 lines of the deps container, and per-service reachability, then exit 1 so provision-build's cleanup trap fires and the VM shuts down fast. - "<service> ready (Ns)" log lines on each service so successful runs show which service was the bottleneck. - 30s heartbeat per service so long-running waits don't look frozen. amd64 is unaffected — services come up in ~1s each under KVM, which is well inside any threshold here.
Same-arch TCG (e.g. arm64 guest on the arm64 ubuntu-24.04-arm runner that has no nested virt) was falling through to -cpu cortex-a72 too. Empirically that hangs wait-for-deps indefinitely — services never reach a ready state — probably because QEMU's TCG emulation of named CPU models is less well-tested than -cpu max, especially for the LSE atomic fallback paths the dep services exercise. The cortex-a72 workaround is only needed for cross-arch TCG, where V8 emits JIT instructions the amd64 host's TCG mistranslates. Restrict it to that case; same-arch TCG now gets -cpu max, matching the known working config from the diagnostics branch run on ubuntu-24.04-arm.
N2D4
approved these changes
Apr 12, 2026
…tor-support # Conflicts: # packages/stack-shared/src/interface/client-interface.ts # packages/template/src/lib/stack-app/apps/implementations/admin-app-impl.ts # packages/template/src/lib/stack-app/apps/implementations/client-app-impl.ts # packages/template/src/lib/stack-app/apps/implementations/server-app-impl.ts
Keeps stdout clean for callers parsing CLI output; the notice is informational rather than a primary result.
build.env sets NEXT_PUBLIC_STACK_IS_LOCAL_EMULATOR=true, which makes docker/server/entrypoint.sh require the three internal SEED keys. At real-VM boot those come from render-stack-env via local-emulator.env, but the build-time smoke test doesn't run that path, so the backend was crash-looping and the amd64 QEMU build failed with a /health timeout. Mint throwaway hex keys for the smoke-test container only.
…m/stack-auth/stack-auth into client-sdk-local-emulator-support
Contributor
|
Promptless prepared a documentation update related to this change. Triggered by PR #1247 Added documentation for the new Review: Document Local Emulator Mode |
12 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
Release Notes
New Features
emulator runCLI command to execute applications with emulator credentials automatically injectedImprovements