Skip to main content
Back to blog

Critiq team

  • rules

The most common security findings in TypeScript PRs (and the fixes)

Eight rule-backed security patterns Critiq flags in TypeScript pull requests, with bad code, good code, and the catalog rule ID for each fix.

The most common security findings in TypeScript PRs (and the fixes) content

TypeScript pull requests repeat a small set of security mistakes. The diff looks fine in review: a fetch helper, a login redirect, a React panel that renders HTML from the API. Each change is localized, but the pattern is familiar, request input reaches a sensitive sink without a guard you can name.

Critiq flags these with stable rule IDs from the @critiq/rules catalog. Every finding ties to a rule file you can read, test, and tune. Below are eight patterns we see often in TypeScript and JavaScript PRs, with the exact rule ID, a bad example, a fixed version, and what to change.

These are not exotic CVEs buried in dependencies. They are application-layer mistakes in code your team wrote: Express handlers, React components, Nest services, and shared utilities. They surface in greenfield features and in one-line "quick fixes" during review crunch. Because TypeScript types do not enforce security boundaries, the compiler stays quiet while request data flows toward dangerous APIs.

Why the same eight patterns keep appearing

Pull request review optimizes for feature correctness and readability. Security sinks, eval, fetch, redirect, raw HTML, SQL strings, look like ordinary library calls until you trace where the argument came from. Assistants and copy-paste tutorials often emit working code with permissive defaults: inline secrets for local dev, open redirects for OAuth return paths, dangerouslySetInnerHTML because the design called for rich text.

  • Small diffs hide data flow, one new query parameter can reach fetch without a second reviewer noticing
  • Framework escape hatches (dangerouslySetInnerHTML, res.redirect) are documented as features, not warnings
  • Auth code is high stakes but low frequency, teams ship a login flow once and rarely revisit cookie flags
  • Rule-backed scanners catch the pattern even when the human reviewer is focused on business logic

1. Hardcoded auth secrets, ts.security.hardcoded-auth-secret

Session signing keys, JWT secrets, and strategy passwords belong in configuration, not string literals in source. Inline secrets leak through git history, build artifacts, and forked repos. Critiq matches authentication flows that embed secret material directly in code.

This is distinct from general API keys (caught by security.no-hardcoded-credentials in the shared catalog). ts.security.hardcoded-auth-secret focuses on material used to sign or validate sessions and tokens, the secrets that let an attacker forge identity if exposed.

Bad

app.use(
  session({
    secret: 'inline-session-secret',
    cookie: { secure: false, httpOnly: false },
  }),
);

Good

app.use(
  session({
    secret: process.env.SESSION_SECRET,
    cookie: {
      secure: true,
      httpOnly: true,
      sameSite: 'lax',
    },
  }),
);

Fix: load secrets from environment-backed config or a secret manager. Rotate any value that was ever committed. Pair with secure cookie defaults so the session token is not readable from browser JavaScript.

2. SQL string interpolation, security.no-sql-interpolation

Building SQL with template literals or concatenation lets user input alter the statement shape. Parameterized queries keep data and query structure separate. This rule lives in the shared catalog and applies to TypeScript database helpers alongside other languages.

ORMs reduce raw SQL, but escape hatches remain: pool.query with a template string, knex.raw with interpolation, or dynamic ORDER BY clauses assembled in a helper. The attack is classic SQL injection, an email field that closes the string and appends DROP TABLE. TypeScript string types do not prevent that.

Bad

export async function findUsersByEmail(email: string) {
  return pool.query(
    `SELECT id, email FROM users WHERE email = '${email}'`,
  );
}

Good

export async function findUsersByEmail(email: string) {
  return pool.query(
    'SELECT id, email FROM users WHERE email = $1',
    [email],
  );
}

Fix: pass user-controlled values as bound parameters. Use your driver placeholder syntax ($1, ?, :email). For ORMs, prefer query builders that parameterize by default instead of raw string assembly.

3. Server-side request forgery, ts.security.ssrf

Outbound fetch and HTTP clients that accept a URL from the request can reach internal services, cloud metadata endpoints, or attacker-controlled hosts. SSRF is common in webhook validators, preview fetchers, and import-by-URL features added in small PRs.

In cloud deployments, SSRF often targets 169.254.169.254 or internal service names on the container network. The vulnerable line is usually a single fetch(url) where url came from req.query, req.body, or a JSON field. Blocking SSRF requires validation before the HTTP client runs, not after.

Bad

void fetch(req.query.url, { signal: controller.signal });

Good

const target = normalizeAllowedUrl(req.query.url);
void fetch(target, { signal: controller.signal });

Fix: resolve URLs against an allowlist of schemes and hosts, reject private IP ranges and link-local addresses, and prefer a server-side proxy that enforces policy. Never pass raw query parameters straight to fetch, axios, or node-fetch.

4. Open redirect, ts.security.open-redirect

Login and OAuth callbacks often accept a returnTo or next query parameter. Redirecting to that value without validation sends authenticated users to a phishing site. The fix is boring and effective: only redirect to internal paths or explicitly trusted origins.

Open redirects are low severity in some scanners and high impact in practice. An attacker emails a link to your legitimate login page with next=https://evil.example; after the user authenticates, the browser lands on a clone that harvests credentials. Critiq flags res.redirect, reply.redirect, and similar sinks when the target is request-derived.

Bad

export function handleLoginCallback(req: Request, res: Response) {
  res.redirect(req.query.next ?? '/dashboard');
}

Good

const ALLOWED = new Set(['/dashboard', '/settings']);

export function handleLoginCallback(req: Request, res: Response) {
  const next = req.query.next ?? '/dashboard';
  res.redirect(ALLOWED.has(next) ? next : '/dashboard');
}

Fix: normalize the target to a relative path starting with /, or validate the full URL against an allowlist of origins. Reject protocol-relative URLs (//evil.example) and encoded bypasses.

5. Unsafe HTML sinks, ts.security.dangerously-set-inner-html

React escapes text by default. dangerouslySetInnerHTML bypasses that model. When the HTML comes from an API, a CMS, or user content, unsanitized markup is a direct XSS vector. Critiq flags non-literal, non-sanitized values passed to this prop.

Teams reach for this prop when markdown renderers or WYSIWYG output need HTML. The mistake is treating server-side encoding as optional. Client-side XSS steals session cookies (especially when httpOnly is missing), performs actions as the victim, and spreads through stored content. Server-side React is not immune, SSR with unsanitized HTML is equally dangerous.

Bad

export function UnsafePanel({ htmlContent }: { htmlContent: string }) {
  return <div dangerouslySetInnerHTML={{ __html: htmlContent }} />;
}

Good

import DOMPurify from 'dompurify';

export function SafePanel({ htmlContent }: { htmlContent: string }) {
  const safe = { __html: DOMPurify.sanitize(htmlContent) };
  return <div dangerouslySetInnerHTML={safe} />;
}

Fix: prefer normal React rendering when possible. When you need raw HTML, sanitize with a maintained library (DOMPurify on the client, an equivalent on the server) or restrict to fixed markup you control. Related catalog rules include ts.security.no-innerhtml-assignment and ts.security.dangerous-insert-html for non-React sinks.

6. Tokens decoded without verification, ts.security.token-or-session-not-validated

Parsing a JWT or session token is not the same as trusting it. Calling decode without verify lets attackers forge claims. This pattern appears when someone swaps in a convenience helper or copies a snippet that skips signature and expiry checks.

The bug is subtle because the code still "works" in development with unsigned test tokens. In production, an attacker crafts a payload with admin: true and the handler accepts it. Always separate verification (cryptographic trust) from decoding (reading claims). Middleware order matters: verify before your route handler reads req.user.

Bad

export function parseIdentity(req: Request) {
  const token = req.headers.authorization;
  return decode(token ?? '');
}

Good

export function parseIdentity(req: Request) {
  const token = req.headers.authorization ?? '';
  verifyToken(token);
  return decode(token);
}

Fix: verify signature, issuer, audience, and expiry before reading claims. Use your framework middleware where available. Treat missing or malformed tokens as unauthenticated, do not fall through to decode-only paths.

7. Insecure session cookie flags, ts.security.insecure-auth-cookie-flags

Session cookies without httpOnly, secure, and a sensible sameSite value are easy to steal via XSS or send over plaintext HTTP. Small PRs that add res.cookie for a new auth flow often copy permissive defaults from a tutorial.

Critiq distinguishes auth cookies from preference cookies. Setting httpOnly: false on a session token makes it readable from any script on the page, including a single XSS finding elsewhere in the app. sameSite: none without secure is invalid in modern browsers and signals misconfiguration. Treat cookie flags as part of the auth design, not styling.

Bad

res.cookie('sessionToken', accessToken, {
  httpOnly: false,
  secure: false,
  sameSite: 'none',
});

Good

res.cookie('sessionToken', accessToken, {
  httpOnly: true,
  secure: true,
  sameSite: 'lax',
  path: '/',
});

Fix: set httpOnly and secure for auth cookies in production. Use sameSite: lax or strict unless you have a documented cross-site requirement. Non-auth preference cookies can stay looser, auth tokens should not.

8. Dynamic code execution, ts.security.no-dynamic-execution

eval, new Function, and similar APIs execute arbitrary strings as code. They show up in admin tooling, template previews, and "run user snippet" features. One malicious payload equals remote code execution in the Node process.

TypeScript does not sandbox eval. vm.runInNewContext without careful hardening is still risky. If your product requirement is user-defined logic, use a constrained DSL, WASM sandbox, or isolated worker with no network and strict timeouts, and still avoid eval in the request path.

Bad

export function executeUserSnippet(source: string) {
  return eval(source);
}

Good

export function normalizeUserSnippet(source: string) {
  return source.trim();
}

Fix: remove eval entirely. Parse structured input (JSON, AST, sandboxed DSL) instead of executing strings. If you truly need isolation, use a dedicated sandbox with strict resource limits, not eval in your main process.

How to prioritize fixes in a busy PR queue

Not every finding needs the same urgency. A practical merge policy for TypeScript services: treat critical auth-secret and injection findings as blockers, SSRF and open redirect as high priority before any feature touches external URLs, XSS sinks before shipping user-generated content, and cookie or token validation gaps before widening beta access.

  • Critical, ts.security.hardcoded-auth-secret, security.no-sql-interpolation
  • High, ts.security.ssrf, ts.security.open-redirect, ts.security.dangerously-set-inner-html
  • High, ts.security.token-or-session-not-validated, ts.security.insecure-auth-cookie-flags
  • High, ts.security.no-dynamic-execution when exposed to non-admin users

Map severities in .critiq/config.yaml and critiq-action fail-on-severity so CI enforces the same bar you use locally. When a rule is noisy on legacy code, scope exceptions to a path or disable a single rule ID, not the entire security preset.

Run the same checks locally and in CI

Each rule above exists in the OSS catalog with specs and fixtures. Run critiq check before you open the PR so findings match what reviewers see in GitHub Actions. JSON output includes ruleId, severity, file, and line for every match.

Filter to TypeScript paths when triaging a large monorepo: critiq check src/apps/api. Compare branches with the same config your workflow uses so you are not surprised by preset differences between local and CI. The goal is one reproducible story, rule ID in the terminal matches the inline comment on the diff.

npx critiq check .
critiq check --format json src/

Wire critiq-action in CI to post inline comments with the same rule IDs. Treat critical and high security findings like any other merge policy you own. When a finding is a false positive, disable or tune the specific rule in .critiq/config.yaml, not the whole category.

For teams onboarding TypeScript services, start with the security preset and fix the eight patterns above before tuning quality rules. The catalog documents each rule under rules/typescript/ with CWE references where applicable, useful when security champions need to justify a blocker to product.

Read the rule, then fix the code

Security review gets faster when feedback is inspectable. A rule ID is a link to the contract: what matched, why it matters, and how to remediate. The eight patterns here cover a large share of TypeScript PR noise, secrets in source, injection, SSRF, redirects, XSS sinks, auth gaps, cookie posture, and dynamic execution.

Browse the full TypeScript security catalog at https://docs.critiq.dev/rules and run the CLI against your repo. If a finding fires, open the rule file and read the remediation block before you merge. That habit turns repetitive PR comments into fixes you only make once.

The eight rule IDs in this guide are a starting set, not an exhaustive security program. They cover the highest-frequency sinks in Node and React codebases we scan. Add dependency and supply-chain checks on their own timeline, but fix these patterns first so application logic is not undermined by a single mistyped redirect or SQL template.