Skip to content

fix: attach form rules and stop dummy Arcjet keys - #218

Closed
davidmytton wants to merge 1 commit into
mainfrom
david/cursor/examples-sdk-review-2c15
Closed

davidmytton wants to merge 1 commit into
mainfrom
david/cursor/examples-sdk-review-2c15

Conversation

@davidmytton

Copy link
Copy Markdown
Collaborator

nextjs-form called protect() with no rules while advertising bot, rate-limit, and Shield protection. Several examples also started with a dummy or empty ARCJET_KEY, which looks copy-pasteable and silently disables enforcement.

nextjs-form called protect() with no rules while advertising bot,
rate-limit, and Shield protection. Several examples also started with
a dummy or empty ARCJET_KEY, which looks copy-pasteable and silently
disables enforcement.

Co-authored-by: David Mytton <davidmytton@users.noreply.github.com>

@arcjet-review arcjet-review Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Arcjet Review — 🟢 Low Risk

Decision: Approved

Rationale: Small, focused fixes to example code that improve security posture: (1) nextjs-form's arcjet.protect() now actually attaches the shield, bot detection, and rate-limit rules it advertises, (2) several examples that previously fell back to a dummy/empty ARCJET_KEY (silently disabling enforcement) now either throw at startup or rely on the env var directly, and (3) the Nuxt Dockerfile switches the dummy key from a persisted ENV to a build-time ARG so it does not ship in the image. No production code paths, no auth/infra beyond example scaffolding, no dependency changes.

Summary of Changes

Fixes example code: attaches missing rules to nextjs-form's Arcjet client, removes silent dummy-key fallbacks in eve-agent, firebase-functions, nextjs-bot-categories, nextjs-bot-protection, and nextjs-server-action, converts the Nuxt Dockerfile dummy key from ENV to ARG so it does not persist at runtime, and documents that eve-agent's /webhook is unauthenticated in the demo.

Escalation Triggers

  • CI/CD Pipeline: examples/nuxt/Dockerfile is modified — changing how ARCJET_KEY is supplied at build vs runtime. Scoped to the example, not production CI.

Notes

All changes are confined to example projects under examples/. No production code paths are affected.

Review: 4b07f9ca | Model: anthropic/claude-opus-4-7 | Powered by Arcjet Review

// Get your site key from https://console.arcjet.com
// and set it as an environment variable rather than hard coding.
// See: https://nextjs.org/docs/app/building-your-application/configuring/environment-variables
key: process.env.ARCJET_KEY!,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The stated goal of this PR is to stop silently disabling enforcement when ARCJET_KEY is unset, but process.env.ARCJET_KEY! only silences the TypeScript check — at runtime undefined is still passed to arcjetNextjs, so behavior depends on how the SDK handles it. Consider matching the explicit-throw pattern used in examples/eve-agent/agent/arcjet.ts and examples/firebase-functions/src/index.ts so misconfiguration fails loudly here too.

// Get your site key from https://console.arcjet.com
// and set it as an environment variable rather than hard coding.
// See: https://nextjs.org/docs/app/building-your-application/configuring/environment-variables
key: process.env.ARCJET_KEY!,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as nextjs-bot-categories: process.env.ARCJET_KEY! is a type-level assertion only. If the env var is unset, undefined reaches the SDK. Consider the explicit-throw pattern used in the eve-agent and firebase-functions examples for consistency.

const aj = arcjet({
// Get your site key from https://console.arcjet.com
key,
// and set it as an environment variable rather than hard coding.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same note as the other Next.js examples — process.env.ARCJET_KEY! doesn't guarantee the value is set at runtime. Consider a runtime check with a clear error message so misconfigured examples surface the problem immediately.

Comment thread examples/nuxt/Dockerfile
# Nuxt requires `ARCJET_KEY` at build time. Keep the placeholder on this RUN
# only — do not persist it as a runtime ENV or the image ships a dummy key.
ARG ARCJET_KEY=ajkey_yourkey
RUN ARCJET_KEY=$ARCJET_KEY npm run postinstall && npm run build

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: file is missing a trailing newline (the diff shows \ No newline at end of file). Worth adding while touching this file.

Comment thread examples/nuxt/Dockerfile
RUN npm run postinstall && npm run build
# Nuxt requires `ARCJET_KEY` at build time. Keep the placeholder on this RUN
# only — do not persist it as a runtime ENV or the image ships a dummy key.
ARG ARCJET_KEY=ajkey_yourkey

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The default ARG ARCJET_KEY=ajkey_yourkey still lets a build succeed with an obviously-fake key when the user forgets --build-arg ARCJET_KEY=.... That's the same failure mode this PR removes elsewhere. Would it be preferable to drop the default and let the build fail fast, or at minimum document that the placeholder means bot/rate-limit rules will be inert until the image is rebuilt with a real key?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants