fix(security): close 6 OAuth backend + nx-claude script scan findings

[wkoutre]

Summary

Closes 6 of the 7 open security-scan findings (dated 2026-03-28) against next. Finding #7 (static heredoc delimiter) was already fixed by #509, so this PR covers the remaining six.

Implemented and adversarially verified via a parallel agent workflow (3 domain implementers + 6 per-finding verifiers), then a code-reviewer pass on the diff.

Slack OAuth backend (apps/slack-oauth-backend)

#	Sev	Fix
1	HIGH	OAuth CSRF. Replaced the length-only state check with a stateless HMAC-SHA256 signed state token (new src/oauth/state.ts). generateState() signs a CSPRNG nonce + timestamp with config.sessionSecret and base64url-encodes it; validateState() recomputes the HMAC with timingSafeEqual and a 10-min freshness window. Forged/unsigned/tampered/expired states are rejected.
2	MED	Weak randomness. Nonce now from crypto.randomBytes(16) instead of Math.random()/Date.now().
3	MED	Tokens in cacheable HTML. Cache-Control: no-store (+ no-cache/Pragma/Expires) on all token-bearing /callback responses and the /refresh JSON; the refresh token is no longer rendered in the HTML fallback.
6	MED	Unauthenticated /slack/refresh. Proportionate hardening: rate limit 10 → 5/min, no-store header, documented threat model.

Why HMAC-signed state, not a server-side store (as the scan suggested): the backend runs on Vercel serverless, where in-process memory is not shared across invocations/instances. A MemoryCache would mean /authorize and /callback hit different instances and every legitimate flow would miss the cache. HMAC signing with the existing sessionSecret blocks forgery with no store. base64url output satisfies the existing validateOAuthCallback regex (/^[a-zA-Z0-9_-]+$/, length 16..500).

Why #6 keeps the token relay unauthenticated: the refresh_token is the bearer credential in this stateless flow; per-caller auth can't be added without new session infra, and the real leak vector (a refresh token in cacheable HTML) is closed by #3. The hardening reduces the abuse surface without pretending the security boundary changed.

nx-claude scripts (packages/ai-toolkit-nx-claude)

#	Sev	Fix
4	MED	Bash arithmetic command injection in reset-prerelease-version.sh. Sources the existing-but-unused .github/scripts/validate-numeric.sh and validates registry-derived values (^[0-9]+$) before $((...)). Payload 1.1.0-next.a[$(...)] is now rejected.
5	MED	Shell injection in the generated slack-env.sh. New shSingleQuote() POSIX single-quote escaping in createSlackEnvFile; updateRefreshToken also switches String.replace to a replacement function to neutralize $-substitution corruption on tokens containing $.

Test plan

• nx test slack-oauth-backend → 83/83 pass. New state.spec.ts covers fresh/tampered/expired/empty/malformed tokens; the integration suite was updated to mint real signed states via generateState() (the old hardcoded unsigned states correctly 400 now).
• nx run-many -t typecheck lint -p slack-oauth-backend ai-toolkit-nx-claude → clean (0 errors; pre-existing no-explicit-any warnings only).
• fix(nx-claude): improve init generator file existence indicators and dry-run behavior #4 functionally verified: injection payload rejected, no command execution; clean value passes.
• docs: expand README with comprehensive getting started guide and inst… #5 functionally verified: $(...), backticks, $HOME, and a single-quote breakout all inert when sourced (bash); benign xoxe-1-… token round-trips verbatim.
• Code-reviewer pass on the diff: READY TO MERGE.

Notes

• No CLAUDE.md/version changes: the nx-claude edits are internal hardening (no API change) and the package is not under packages/plugins/, so the mandatory plugin-version-bump rule does not apply.
• Out of scope (noted by verifiers): enforcing a minimum SESSION_SECRET entropy at boot, and the loadSlackConfig parser's single-quote truncation (benign for real xoxe- tokens).

🤖 Generated with Claude Code

AI-Generated Description

Summary

Closes 6 of the 7 open security-scan findings (dated 2026-03-28) against next. Finding #7 (static heredoc delimiter) was already fixed by #509, so this PR covers the remaining six.
Implemented and adversarially verified via a parallel agent workflow (3 domain implementers + 6 per-finding verifiers), then a code-reviewer pass on the diff.

Slack OAuth backend (apps/slack-oauth-backend)

#	Sev	Fix
1	HIGH	OAuth CSRF. Replaced the length-only state check with a stateless HMAC-SHA256 signed state token (new src/oauth/state.ts). generateState() signs a CSPRNG nonce + timestamp with config.sessionSecret and base64url-encodes it; validateState() recomputes the HMAC with timingSafeEqual and a 10-min freshness window. Forged/unsigned/tampered/expired states are rejected.
2	MED	Weak randomness. Nonce now from crypto.randomBytes(16) instead of Math.random()/Date.now().
3	MED	Tokens in cacheable HTML. Cache-Control: no-store (+ no-cache/Pragma/Expires) on all token-bearing /callback responses and the /refresh JSON; the refresh token is no longer rendered in the HTML fallback.
6	MED	Unauthenticated /slack/refresh. Proportionate hardening: rate limit 10 → 5/min, no-store header, documented threat model.
Why HMAC-signed state, not a server-side store (as the scan suggested): the backend runs on Vercel serverless, where in-process memory is not shared across invocations/instances. A MemoryCache would mean /authorize and /callback hit different instances and every legitimate flow would miss the cache. HMAC signing with the existing sessionSecret blocks forgery with no store. base64url output satisfies the existing validateOAuthCallback regex (/^[a-zA-Z0-9_-]+$/, length 16..500).
Why #6 keeps the token relay unauthenticated: the refresh_token is the bearer credential in this stateless flow; per-caller auth can't be added without new session infra, and the real leak vector (a refresh token in cacheable HTML) is closed by #3. The hardening reduces the abuse surface without pretending the security boundary changed.

nx-claude scripts (packages/ai-toolkit-nx-claude)

#	Sev	Fix
4	MED	Bash arithmetic command injection in reset-prerelease-version.sh. Sources the existing-but-unused .github/scripts/validate-numeric.sh and validates registry-derived values (^[0-9]+$) before $((...)). Payload 1.1.0-next.a[$(...)] is now rejected.
5	MED	Shell injection in the generated slack-env.sh. New shSingleQuote() POSIX single-quote escaping in createSlackEnvFile; updateRefreshToken also switches String.replace to a replacement function to neutralize $-substitution corruption on tokens containing $.

Test plan

• nx test slack-oauth-backend → 83/83 pass. New state.spec.ts covers fresh/tampered/expired/empty/malformed tokens; the integration suite was updated to mint real signed states via generateState() (the old hardcoded unsigned states correctly 400 now).
• nx run-many -t typecheck lint -p slack-oauth-backend ai-toolkit-nx-claude → clean (0 errors; pre-existing no-explicit-any warnings only).
• fix(nx-claude): improve init generator file existence indicators and dry-run behavior #4 functionally verified: injection payload rejected, no command execution; clean value passes.
• docs: expand README with comprehensive getting started guide and inst… #5 functionally verified: $(...), backticks, $HOME, and a single-quote breakout all inert when sourced (bash); benign xoxe-1-… token round-trips verbatim.
• Code-reviewer pass on the diff: READY TO MERGE.

Notes

• No CLAUDE.md/version changes: the nx-claude edits are internal hardening (no API change) and the package is not under packages/plugins/, so the mandatory plugin-version-bump rule does not apply.
• Out of scope (noted by verifiers): enforcing a minimum SESSION_SECRET entropy at boot, and the loadSlackConfig parser's single-quote truncation (benign for real xoxe- tokens).

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
ai-toolkit-slack-oauth-backend	Ready	Preview, Comment	Jun 25, 2026 4:58pm

[github-actions]

🤖 Claude Code Review

✅ Review complete

Summary

This PR closes the CSRF gap flagged on the prior review by binding the OAuth state token to the initiating browser via a double-submit cookie pattern (HMAC-signed state + HttpOnly per-browser nonce cookie), and bundles several adjacent hardening fixes (no-store headers on token-bearing responses, refresh token no longer rendered in HTML, shell-quoting fixes in the setup wizard, and numeric validation in the prerelease reset script). The implementation is stateless, which is the right choice for the Vercel serverless target.

The crypto, validation order, and one-time-use semantics all look correct. No blocking issues found.

Detailed analysis

apps/slack-oauth-backend/src/oauth/state.ts

• HMAC-SHA256 over nonce.timestamp.browserNonce with config.sessionSecret, output base64url; structurally sound and satisfies the existing ^[a-zA-Z0-9_-]+$ callback validator.
• validateState order is: type/length guards → base64 decode → 4-part split → signature check (constant-time) → nonce binding check (constant-time) → timestamp parse + freshness. Crucially, signature verification happens before the Number.isInteger check on the timestamp, so an attacker cannot influence issuedAt without already holding the secret.
• The early-length check in constantTimeEqual leaks the length of the expected HMAC (always 43 chars) and browser nonce (always 22 chars), but both lengths are public, so this is fine.
• The try/catch wrap means decode failures (e.g., on a non-base64url state) return false rather than throwing — appropriate for a security-boundary validator.

apps/slack-oauth-backend/src/routes/oauth.ts

• /callback: reads the cookie and clearCookies before any branching return (oauth.ts:90-91), so a leaked state can't be replayed on a second callback because the cookie is already gone. Good ordering.
• The latest commit (53c0c91) inlines httpOnly/secure/sameSite/path directly in res.cookie(...) at oauth.ts:224-230 for Semgrep's matcher, while keeping browserNonceCookieBaseOptions as the source of truth for clearCookie. Both currently agree; the inline comment warning "These MUST stay in sync" is appropriate. A future change to path in only one place would silently break clearCookie (browsers require matching path to identify the cookie for removal), so this is the only fragility worth keeping in mind during future edits.
• Cache-Control: no-store, no-cache, must-revalidate, private is correctly applied to all three response branches (error, success, exception). Combined with the change to never render the refresh token in HTML at oauth.ts:172, the cacheable-token-leak vector is closed.

apps/slack-oauth-backend/src/utils/cookies.ts

• Small, dependency-free parser. Handles empty header, missing =, surrounding whitespace, decode failures (falls back to raw value). It does not strip RFC 6265 surrounding quotes (name="value"), but Express's res.cookie does not emit quoted base64url values, so this is fine in practice.

apps/slack-oauth-backend/src/routes/refresh.ts

• Rate limit dropped from 10 → 5/min. Given Slack token lifetimes (~12h), 5/min is still well above any legitimate usage; this is a reasonable tightening.
• Cache-Control: no-store on the JSON response keeps intermediary caches from holding refreshed tokens. Good.
• The new docstring's threat-model paragraph is clear about why there is no per-caller auth (the refresh token is the bearer credential).

packages/ai-toolkit-nx-claude/src/scripts/claude-plus/slack-setup.ts + slack-token.ts

• shSingleQuote correctly uses the POSIX '\'' idiom for embedded single quotes, and single-quoting makes $, backticks, ", \ all inert when the file is sourced. The unit boundary is clean: any token containing a $ (which could otherwise be interpreted as a variable when the file is sourced) is now safely literal.
• In slack-token.ts, switching content.replace(/.../, newLine) → content.replace(/.../, () => newLine) defangs String.prototype.replace's special pattern handling of $&/$1/$$ in replacement strings. This is the right fix — important because the refresh token is attacker-influenceable (it comes from a Slack response) and the previous code would have been confused by a $ in the token.

packages/ai-toolkit-nx-claude/scripts/reset-prerelease-version.sh

• Sources validate-numeric.sh (referenced as ../../../.github/scripts/validate-numeric.sh) and validates MINOR and PRERELEASE_NUM before passing them into $((...)). This blocks command-injection via bash arithmetic, which is good defense even though the inputs come from jq/npm view rather than untrusted user input.

Tests

• state.spec.ts, cookies.spec.ts, oauth.spec.ts, and the integration suite all cover the right cases: matching nonce → success + cleared cookie, mismatched nonce → 400, no cookie → 400, tampered signature → reject, expired token → reject, forged signature → reject. The integration tests correctly drive a real /authorize → /callback round-trip rather than hand-rolling a state, so they exercise the binding end-to-end.

Responses to existing threads

• CSRF binding (P1): implementation matches the described approach; leaving the thread open for the human author since discussion is active.
• Semgrep secure / sameSite / HttpOnly: the inline-flags commit (53c0c91) should make the next Semgrep scan see all three flags statically; leaving open for the Semgrep platform to re-triage.

Links

• PR: fix(security): close 6 OAuth backend + nx-claude script scan findings #525
• Compare: 92541aa...HEAD

---

_{💡 Want a fresh review? Add a comment containing @request-claude-review to trigger a new review at any time.}

[github-actions]

📚 Documentation Check ✅

Verdict: Passed

No files in packages/plugins/ were modified, so no plugin version bump is required. Documentation gaps exist but fail_on_missing_docs is false.

---

PR #525 Documentation Analysis

No plugin version bump required — all changes are in apps/slack-oauth-backend/ and packages/ai-toolkit-nx-claude/, neither of which is under packages/plugins/.

Key Changes in This PR

• New CSRF defense: state.ts introduces stateless HMAC-SHA256 signed, browser-bound OAuth state tokens with a double-submit cookie pattern (OAUTH_STATE_COOKIE, generateBrowserNonce, generateState, validateState).
• New utility: utils/cookies.ts adds a minimal cookie reader to avoid cookie-parser middleware.
• Hardened oauth.ts: The /authorize route now mints a browser nonce cookie; /callback validates both the HMAC signature and the cookie binding before exchanging the code.
• Tightened refresh.ts: Rate limit halved (10→5/min), no-store cache headers added.
• shSingleQuote helper in slack-setup.ts / slack-token.ts fixes shell injection risk when writing slack-env.sh.
• reset-prerelease-version.sh uses validate_numeric guard to prevent arithmetic injection.

Documentation Gaps Found

1. apps/slack-oauth-backend/src/oauth/CLAUDE.md (warning) — The entire new state.ts module (the core of this PR's security work) is absent. CLAUDE.md only lists handler.ts, types.ts, and handler.spec.ts.
2. apps/slack-oauth-backend/CLAUDE.md (info) — SESSION_SECRET is missing from the Required Environment Variables list. The utils/ directory description doesn't mention cookies.ts.
3. packages/ai-toolkit-nx-claude/src/scripts/claude-plus/CLAUDE.md (info) — The slack-setup.ts Key Functions section is missing the new shSingleQuote export (also re-exported via slack-token.ts).

apps/slack-oauth-backend/README.md is already correct — it shows SESSION_SECRET with a generation hint.

Missing Updates

Type	File	Severity	Reason
📘 claude_md	apps/slack-oauth-backend/src/oauth/CLAUDE.md	⚠️ warning	New state.ts module (HMAC-signed browser-bound OAuth state tokens) is completely undocumented. CLAUDE.md only lists handler.ts, types.ts, and handler.spec.ts — the core security contribution of this PR has no entry.
📘 claude_md	apps/slack-oauth-backend/CLAUDE.md	ℹ️ info	SESSION_SECRET is now a required environment variable (used by state.ts for HMAC signing) but is absent from the Required variables list.
📘 claude_md	packages/ai-toolkit-nx-claude/src/scripts/claude-plus/CLAUDE.md	ℹ️ info	The new shSingleQuote export added to slack-setup.ts (and consumed by slack-token.ts) is not listed in the Key Functions section.

Suggestions (4)

💡 Inline suggestions have been posted as review comments. Click "Commit suggestion" to apply each fix directly.

• ⚠️ apps/slack-oauth-backend/src/oauth/CLAUDE.md: The new state.ts file is the core security contribution of this PR. Its CLAUDE.md entry is completely missing, leaving future AI assistants with no context about the double-submit cookie CSRF pattern, the token format, or the exported API surface.
• ℹ️ apps/slack-oauth-backend/CLAUDE.md: SESSION_SECRET is now required for the HMAC-signed OAuth state tokens introduced in this PR. Omitting it from the docs will cause confusion during local setup.
• ℹ️ apps/slack-oauth-backend/CLAUDE.md: The oauth/ directory now contains the new state.ts module. Updating the comment clarifies the expanded scope of the directory.
• ℹ️ packages/ai-toolkit-nx-claude/src/scripts/claude-plus/CLAUDE.md: shSingleQuote is a new export from slack-setup.ts (also re-imported by slack-token.ts) that prevents shell injection when writing slack-env.sh. It should be listed in the CLAUDE.md API surface so future contributors know it exists and why.

---

_{🤖 Generated by Claude Documentation Validator | Mode: suggest}

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6ff9be7061

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[github-actions]

📋 Review verdict: APPROVE

👆 The main review comment above is the source of truth for this PR review. It is automatically updated on each review cycle, so always refer to it for the most current feedback.

This formal review submission is for the verdict only.

Superseded by new review after PR update

[github-actions]

📋 Review verdict: APPROVE

👆 The main review comment above is the source of truth for this PR review. It is automatically updated on each review cycle, so always refer to it for the most current feedback.

This formal review submission is for the verdict only.

Superseded by new review after PR update

[github-actions]

📋 Review verdict: APPROVE

👆 The main review comment above is the source of truth for this PR review. It is automatically updated on each review cycle, so always refer to it for the most current feedback.

This formal review submission is for the verdict only.

Superseded by new review after PR update

[github-actions]

📋 Review verdict: APPROVE

👆 The main review comment above is the source of truth for this PR review. It is automatically updated on each review cycle, so always refer to it for the most current feedback.

This formal review submission is for the verdict only.