Skip to content

require-nan-check-after-env-numeric-parse: ternary-wrapped env parse escapes detection, risking silent bypass of file-size guard #50496

Description

@github-actions

Rule

eslint-factory/src/rules/require-nan-check-after-env-numeric-parse.ts

Problem

The VariableDeclarator visitor (lines 114-117) only tracks a name when node.init?.type === "CallExpression". When the numeric-parse call is wrapped in a ConditionalExpression — e.g. const x = process.env.FOO ? parseInt(process.env.FOO, 10) : default; — the declarator's init is a ConditionalExpression, not a CallExpression, so the variable is never added to unvalidated and the rule silently allows it through with zero validation.

This is a distinct gap from the already-covered case of a ternary appearing inside the parse call's argument (parseInt(process.env.DELAY ? process.env.DELAY : "1000", 10), already valid-tested) — here the ternary wraps the entire parse call itself.

Grounded live false negative with real risk

actions/setup/js/safe_outputs_handlers.cjs:480-482:

const maxSizeKB = process.env.GH_AW_ASSETS_MAX_SIZE_KB ? parseInt(process.env.GH_AW_ASSETS_MAX_SIZE_KB, 10) : 10240; // Default 10MB
if (sizeKB > maxSizeKB) {
  throw new Error(`${ERR_VALIDATION}: File size ${sizeKB} KB exceeds maximum allowed size ${maxSizeKB} KB`);
}

Grep confirms no isNaN/Number.isFinite anywhere in this file for maxSizeKB. If GH_AW_ASSETS_MAX_SIZE_KB is set to a non-numeric string, parseInt returns NaN, and sizeKB > NaN always evaluates to false — the asset size limit is silently disabled with no error and no warning. This is exactly the failure mode the rule exists to catch, but the ternary-around-the-call shape hides it from detection entirely.

Acceptance criteria

  • Extend the VariableDeclarator visitor (or isNumericParseCallFromEnv) to also unwrap a ConditionalExpression init and check whether its consequent or alternate branch is a numeric-parse-from-env call, tracking the declarator the same way a direct CallExpression init is tracked today.
  • Add an invalid-case test mirroring const maxSizeKB = process.env.FOO ? parseInt(process.env.FOO, 10) : 10240; with no downstream NaN check.
  • Keep the existing valid-case ternary-inside-argument tests (parseInt(process.env.DELAY ? process.env.DELAY : "1000", 10)) passing unchanged — this fix targets ternary-around-the-call, not ternary-inside-the-argument.
  • Verify the fix would flag the safe_outputs_handlers.cjs:480 pattern.

Scope

eslint-factory/** (in scope). Grounding evidence drawn from actions/setup/js/** (rule target, in scope).

Generated by 🤖 ESLint Refiner · agent · 155 AIC · ⌖ 30.9 AIC · ⊞ 4.9K ·

  • expires on Aug 11, 2026, 10:14 PM UTC-08:00

Activity

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

Metadata

Metadata

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions