Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion .github/skills/agentic-workflows/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,6 @@ Load these files from `github/gh-aw` (they are not available locally).
- `.github/aw/test-coverage.md`
- `.github/aw/test-expression.md`
- `.github/aw/token-optimization-caching-budgets.md`
- `.github/aw/token-optimization-observability.md`
- `.github/aw/token-optimization.md`
- `.github/aw/triggers.md`
- `.github/aw/update-agentic-workflow.md`
Expand Down
28 changes: 28 additions & 0 deletions eslint-factory/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ This project hosts custom ESLint linters for `/actions/setup/js`.
| [`require-mkdirsync-try-catch`](#require-mkdirsync-try-catch) | Require try/catch around `fs.mkdirSync` calls |
| [`require-new-url-try-catch`](#require-new-url-try-catch) | Require try/catch around `new URL(variable)` calls |
| [`require-parseInt-radix`](#require-parseInt-radix) | Require an explicit radix argument to `parseInt()` |
| [`require-nan-check-after-env-numeric-parse`](#require-nan-check-after-env-numeric-parse) | Require NaN validation after parsing numeric values from `process.env` |
| [`require-return-after-core-setfailed`](#require-return-after-core-setfailed) | Require a control-transfer statement after `core.setFailed()` |
| [`require-execsync-try-catch`](#require-execsync-try-catch) | Require try/catch around `execSync(...)` calls from `child_process` |
| [`require-execfilesync-try-catch`](#require-execfilesync-try-catch) | Require try/catch around `execFileSync(...)` calls from `child_process` |
Expand Down Expand Up @@ -327,6 +328,33 @@ Flagged forms:

Why: omitting the radix allows implicit base detection, which can silently accept prefixes such as `0x`.

### `require-nan-check-after-env-numeric-parse`

Require NaN validation after parsing numeric values from `process.env`.

Why: `parseInt`, `parseFloat`, `Number.parseInt`, `Number.parseFloat`, and `Number()` silently return `NaN` for malformed environment input (empty string, typo, unexpected value). An unvalidated `NaN` can propagate silently into comparisons (e.g. rate-limit thresholds, size limits, timeouts), loop bounds, or GitHub API payloads without any error surfacing.

**Detected parse forms (first argument must trace back to `process.env`):**
- `parseInt(process.env.FOO, 10)` — global `parseInt`
- `parseFloat(process.env.FOO)` — global `parseFloat`
- `Number.parseInt(process.env.FOO, 10)` — `Number.parseInt`
- `Number.parseFloat(process.env.FOO)` — `Number.parseFloat`
- `Number(process.env.FOO)` — `Number` conversion function

**Detected env-access patterns in the first argument:**
- Direct: `process.env.FOO`
- Logical fallbacks: `process.env.FOO || "default"`, `process.env.FOO ?? "default"`
- Optional chaining: `process.env.FOO?.trim()`
- Ternary: `process.env.FOO ? process.env.FOO : "default"`

**Considered validated when** the declared variable is passed as the sole argument to `Number.isNaN(...)` or `isNaN(...)` anywhere in the enclosing file scope.

**Safe pattern:**
```js
const maxRuns = parseInt(process.env.MAX_RUNS, 10);
if (Number.isNaN(maxRuns)) throw new Error("MAX_RUNS must be a valid integer");
```

### `require-mkdirsync-try-catch`

Require `fs.mkdirSync` calls to be wrapped in `try/catch`.
Expand Down
1 change: 1 addition & 0 deletions eslint-factory/eslint.config.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ module.exports = [
"gh-aw-custom/no-duplicate-constant-values": "warn",
"gh-aw-custom/require-escaped-regexp-interpolation": "warn",
"gh-aw-custom/require-fetch-timeout": "warn",
"gh-aw-custom/require-nan-check-after-env-numeric-parse": "warn",
},
},
{
Expand Down
2 changes: 2 additions & 0 deletions eslint-factory/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ import { noCoreErrorThenSetFailedRule } from "./rules/no-core-error-then-setfail
import { noDuplicateConstantValuesRule } from "./rules/no-duplicate-constant-values";
import { requireEscapedRegexpInterpolationRule } from "./rules/require-escaped-regexp-interpolation";
import { requireFetchTimeoutRule } from "./rules/require-fetch-timeout";
import { requireNanCheckAfterEnvNumericParseRule } from "./rules/require-nan-check-after-env-numeric-parse";

const plugin = {
meta: {
Expand Down Expand Up @@ -79,6 +80,7 @@ const plugin = {
"no-duplicate-constant-values": noDuplicateConstantValuesRule,
"require-escaped-regexp-interpolation": requireEscapedRegexpInterpolationRule,
"require-fetch-timeout": requireFetchTimeoutRule,
"require-nan-check-after-env-numeric-parse": requireNanCheckAfterEnvNumericParseRule,
},
};

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,234 @@
import { RuleTester } from "eslint";
import { describe, expect, it } from "vitest";
import { requireNanCheckAfterEnvNumericParseRule } from "./require-nan-check-after-env-numeric-parse";

const cjsRuleTester = new RuleTester({
languageOptions: {
ecmaVersion: 2022,
sourceType: "commonjs",
},
});

const esmRuleTester = new RuleTester({
languageOptions: {
ecmaVersion: 2022,
sourceType: "module",
},
});

describe("require-nan-check-after-env-numeric-parse", () => {
it("uses the correct docs URL", () => {
expect(requireNanCheckAfterEnvNumericParseRule.meta.docs.url).toBe("https://github.com/github/gh-aw/tree/main/eslint-factory#require-nan-check-after-env-numeric-parse");
});

it("valid: parseInt from process.env validated with Number.isNaN", () => {
cjsRuleTester.run("require-nan-check-after-env-numeric-parse", requireNanCheckAfterEnvNumericParseRule, {
valid: [`const maxRuns = parseInt(process.env.MAX_RUNS, 10); if (Number.isNaN(maxRuns)) throw new Error("invalid MAX_RUNS");`, `const port = parseInt(process.env.PORT, 10); if (!Number.isNaN(port)) startServer(port);`],
invalid: [],
});
});

it("valid: parseInt from process.env validated with global isNaN", () => {
cjsRuleTester.run("require-nan-check-after-env-numeric-parse", requireNanCheckAfterEnvNumericParseRule, {
valid: [`const maxRuns = parseInt(process.env.MAX_RUNS, 10); if (isNaN(maxRuns)) throw new Error("invalid MAX_RUNS");`],
invalid: [],
});
});

it("valid: Number.parseInt from process.env validated with Number.isNaN", () => {
cjsRuleTester.run("require-nan-check-after-env-numeric-parse", requireNanCheckAfterEnvNumericParseRule, {
valid: [`const port = Number.parseInt(process.env.PORT, 10); if (Number.isNaN(port)) throw new Error("invalid PORT");`],
invalid: [],
});
});

it("valid: Number.parseFloat from process.env validated with Number.isNaN", () => {
cjsRuleTester.run("require-nan-check-after-env-numeric-parse", requireNanCheckAfterEnvNumericParseRule, {
valid: [`const delay = Number.parseFloat(process.env.DELAY); if (Number.isNaN(delay)) throw new Error("invalid DELAY");`],
invalid: [],
});
});

it("valid: Number() from process.env validated with Number.isNaN", () => {
cjsRuleTester.run("require-nan-check-after-env-numeric-parse", requireNanCheckAfterEnvNumericParseRule, {
valid: [`const runId = Number(process.env.RUN_ID); if (Number.isNaN(runId)) throw new Error("invalid RUN_ID");`],
invalid: [],
});
});

it("valid: parseFloat from process.env validated with Number.isNaN", () => {
cjsRuleTester.run("require-nan-check-after-env-numeric-parse", requireNanCheckAfterEnvNumericParseRule, {
valid: [`const rate = parseFloat(process.env.RATE); if (Number.isNaN(rate)) throw new Error("invalid RATE");`],
invalid: [],
});
});

it("valid: parseInt without process.env is not flagged", () => {
cjsRuleTester.run("require-nan-check-after-env-numeric-parse", requireNanCheckAfterEnvNumericParseRule, {
valid: [`const x = parseInt("42", 10);`, `const y = parseInt(someVariable, 10);`, `const z = Number("42");`],
invalid: [],
});
});

it("valid: env access validated with isNaN using logical fallback pattern", () => {
cjsRuleTester.run("require-nan-check-after-env-numeric-parse", requireNanCheckAfterEnvNumericParseRule, {
valid: [`const count = parseInt(process.env.COUNT || "0", 10); if (Number.isNaN(count)) throw new Error();`, `const count = parseInt(process.env.COUNT ?? "0", 10); if (Number.isNaN(count)) throw new Error();`],
invalid: [],
});
});

it("valid: env access validated with isNaN using optional chaining pattern", () => {
cjsRuleTester.run("require-nan-check-after-env-numeric-parse", requireNanCheckAfterEnvNumericParseRule, {
valid: [`const count = parseInt(process.env.COUNT?.trim(), 10); if (Number.isNaN(count)) throw new Error();`],
invalid: [],
});
});

it("valid: env access validated with isNaN using ternary pattern", () => {
cjsRuleTester.run("require-nan-check-after-env-numeric-parse", requireNanCheckAfterEnvNumericParseRule, {
valid: [`const delay = parseInt(process.env.DELAY ? process.env.DELAY : "1000", 10); if (Number.isNaN(delay)) throw new Error();`],
invalid: [],
});
});

it("invalid: parseInt from process.env without NaN check", () => {
cjsRuleTester.run("require-nan-check-after-env-numeric-parse", requireNanCheckAfterEnvNumericParseRule, {
valid: [],
invalid: [
{
code: `const maxRuns = parseInt(process.env.MAX_RUNS, 10);`,
errors: [{ messageId: "requireNaNCheck", data: { name: "maxRuns" } }],
},
{
code: `const port = parseInt(process.env.PORT, 10); startServer(port);`,
errors: [{ messageId: "requireNaNCheck", data: { name: "port" } }],
},
],
});
});

it("invalid: parseFloat from process.env without NaN check", () => {
cjsRuleTester.run("require-nan-check-after-env-numeric-parse", requireNanCheckAfterEnvNumericParseRule, {
valid: [],
invalid: [
{
code: `const rate = parseFloat(process.env.RATE);`,
errors: [{ messageId: "requireNaNCheck", data: { name: "rate" } }],
},
],
});
});

it("invalid: Number.parseInt from process.env without NaN check", () => {
cjsRuleTester.run("require-nan-check-after-env-numeric-parse", requireNanCheckAfterEnvNumericParseRule, {
valid: [],
invalid: [
{
code: `const port = Number.parseInt(process.env.PORT, 10);`,
errors: [{ messageId: "requireNaNCheck", data: { name: "port" } }],
},
],
});
});

it("invalid: Number.parseFloat from process.env without NaN check", () => {
cjsRuleTester.run("require-nan-check-after-env-numeric-parse", requireNanCheckAfterEnvNumericParseRule, {
valid: [],
invalid: [
{
code: `const delay = Number.parseFloat(process.env.DELAY);`,
errors: [{ messageId: "requireNaNCheck", data: { name: "delay" } }],
},
],
});
});

it("invalid: Number() from process.env without NaN check", () => {
cjsRuleTester.run("require-nan-check-after-env-numeric-parse", requireNanCheckAfterEnvNumericParseRule, {
valid: [],
invalid: [
{
code: `const runId = Number(process.env.RUN_ID);`,
errors: [{ messageId: "requireNaNCheck", data: { name: "runId" } }],
},
],
});
});

it("invalid: logical fallback env access without NaN check", () => {
cjsRuleTester.run("require-nan-check-after-env-numeric-parse", requireNanCheckAfterEnvNumericParseRule, {
valid: [],
invalid: [
{
code: `const maxFileSize = parseInt(process.env.MAX_FILE_SIZE || "1000", 10);`,
errors: [{ messageId: "requireNaNCheck", data: { name: "maxFileSize" } }],
},
{
code: `const timeout = parseInt(process.env.TIMEOUT ?? "5000", 10);`,
errors: [{ messageId: "requireNaNCheck", data: { name: "timeout" } }],
},
],
});
});

it("invalid: optional chaining env access without NaN check", () => {
cjsRuleTester.run("require-nan-check-after-env-numeric-parse", requireNanCheckAfterEnvNumericParseRule, {
valid: [],
invalid: [
{
code: `const port = parseInt(process.env.PORT?.trim(), 10);`,
errors: [{ messageId: "requireNaNCheck", data: { name: "port" } }],
},
],
});
});

it("invalid: ternary-wrapped env access without NaN check", () => {
cjsRuleTester.run("require-nan-check-after-env-numeric-parse", requireNanCheckAfterEnvNumericParseRule, {
valid: [],
invalid: [
{
code: `const configuredDelay = parseInt(process.env.DELAY ? process.env.DELAY : "1000", 10);`,
errors: [{ messageId: "requireNaNCheck", data: { name: "configuredDelay" } }],
},
],
});
});

it("invalid: multiple unvalidated env parse declarations are each reported", () => {
cjsRuleTester.run("require-nan-check-after-env-numeric-parse", requireNanCheckAfterEnvNumericParseRule, {
valid: [],
invalid: [
{
code: `
const maxFileSize = parseInt(process.env.MAX_FILE_SIZE, 10);
const maxFileCount = parseInt(process.env.MAX_FILE_COUNT, 10);
`.trim(),
errors: [
{ messageId: "requireNaNCheck", data: { name: "maxFileSize" } },
{ messageId: "requireNaNCheck", data: { name: "maxFileCount" } },
],
},
],
});
});

it("valid: ESM import style — validated with Number.isNaN", () => {
esmRuleTester.run("require-nan-check-after-env-numeric-parse", requireNanCheckAfterEnvNumericParseRule, {
valid: [`const count = parseInt(process.env.COUNT, 10); if (Number.isNaN(count)) throw new Error();`],
invalid: [],
});
});

it("invalid: ESM import style — missing NaN check", () => {
esmRuleTester.run("require-nan-check-after-env-numeric-parse", requireNanCheckAfterEnvNumericParseRule, {
valid: [],
invalid: [
{
code: `const count = parseInt(process.env.COUNT, 10);`,
errors: [{ messageId: "requireNaNCheck", data: { name: "count" } }],
},
],
});
});
});
Loading