github / github/gh-aw

require-invalid-date-check-before-compare misses arithmetic-derived duration variables (live gap in check_rate_limit.cjs)

Open
#60,577 0 comments 0 reactions 0 assignees View on GitHub
cookie eslint eslint-factory
Dominant language
Go
Stars
5.1k
Forks
541
Avg merge
5h 48m
Merged PRs (30d)
773

Description

### Summary

`require-invalid-date-check-before-compare` tracks `new Date(x)`/`Date.parse(x)` variables (and their direct `.getTime()` receivers) for use in relational comparisons, but it does **not** propagate that tracking through arithmetic. When two tracked `Date` variables are subtracted via `.getTime()` and the numeric result is assigned to a new variable, that variable becomes invisible to the rule — even though it inherits the exact same "may be NaN" hazard the rule exists to catch.

### Live example (ungrounded gap, still present)

`actions/setup/js/check_rate_limit.cjs:193-201`:

```js
if (run.created_at && run.updated_at) {
const runStart = new Date(run.created_at);
const runEnd = new Date(run.updated_at);
const durationSeconds = (runEnd.getTime() - runStart.getTime()) / 1000;

if (durationSeconds < 15) {
core.info(` Skipping run ${run.id} - ran for less than 15s (${durationSeconds.toFixed(1)}s)`);
continue;
}
}
```

`runStart`/`runEnd` are correctly tracked as `dateVars` (kind `"construct"`, since `new Date(run.created_at)` has a non-trivial argument per `isPotentiallyInvalidDateConstruction`). But `durationSeconds < 15` is never inspected: `BinaryExpression`'s operand-side resolution (`require-invalid-date-check-before-compare.ts:340-362`) only recognizes a side as reportable when it is (a) an inline `new Date(...)`/`Date.parse(...)`, (b) a bare `.getTime()` call (`extractGetTimeReceiver`), or (c) an `Identifier` that resolves directly to a `dateVars` entry. `durationSeconds`'s initializer is a `BinaryExpression` (a division of a subtraction of two `.getTime()` calls) — none of those three shapes — so it's never added to `dateVars`, and the comparison at `Program:exit` collects zero `sides` for this `BinaryExpression` and is silently skipped.

### Why it matters

`run.created_at`/`run.updated_at` are truthiness-checked (`if (run.created_at && run.updated_at)`) but never validated for actually being parseable date strings. If either is malformed-but-truthy (e.g. an empty-ish placeholder, a non-ISO string from an unexpected API response shape, etc.), `new Date(...)` produces an Invalid Date, `.getTime()` returns `NaN`, and `durationSeconds` becomes `NaN`. `NaN < 15` evaluates to `false`, so the "skip runs that completed in under 15s" short-circuit is silently defeated instead of raising a visible error — exactly the failure mode this rule is designed to catch, just one arithmetic step removed from what it currently tracks.

### Test coverage gap

`require-invalid-date-check-before-compare.test.ts` has exactly one arithmetic-related case (line 53, `"invalid: Date.now() arithmetic with a non-literal operand is not guaranteed finite"`), which covers `Date.now() - x` (a single numeric `Date.now()` call arithmetically combined with an unrelated operand). There is no test for the shape above: **two separately-tracked `new Date(x)`/`Date.parse(x)` variables, subtracted via `.getTime()`, assigned to a third variable, and only that third variable used in the relational comparison.**

### Suggested fix / acceptance criteria

- When a `VariableDeclarator`'s initializer is a `BinaryExpression` whose operands (after unwrapping `.getTime()` receivers, mirroring `extractGetTimeReceiver`) are each either an inline `new Date(...)`/`Date.parse(...)` or an `Identifier` resolving to an existing `dateVars` entry, register the declared variable in `dateVars` too (e.g. a new kind such as `"derived"`), so it flows into the existing `BinaryExpression` comparison-side and guard-dominance logic unchanged.
- A guard on the derived variable itself (e.g. `if (Number.isNaN(durationSeconds)) return/continue;`) should satisfy validation, in addition to (not instead of) guards on the original source variables before the arithmetic.
- Add a test case matching the `check_rate_limit.cjs` shape: two `new Date(x)` vars, a `.getTime()` subtraction assigned to a third variable, and a relational comparison on that third variable with no guard anywhere — expect it to report.
- Add a valid-case test where the derived variable (or both source variables) is properly guarded before the comparison — expect no report.

> [!WARNING]
>
> Firewall blocked 1 domain
>
> The following domain was blocked by the firewall during workflow execution:
>
> - `api.anthropic.com`
>
> To allow these domains, add them to the `network.allowed` list in your workflow frontmatter:
>
> ```yaml
> network:
> allowed:
> - defaults
> - "api.anthropic.com"
> ```
>
> See [Network Configuration](https://github.github.com/gh-aw/reference/network/) for more information.
>
>

> Generated by [🤖 ESLint Refiner](https://github.com/github/gh-aw/actions/runs/34740049086) · claude · agent · 469.4 AIC · ⌖ 7.38 AIC · ⊞ 5.8K · [◷](https://github.com/search?q=repo%3Agithub%2Fgh-aw+is%3Aissue+%22gh-aw-workflow-call-id%3A+github%2Fgh-aw%2Feslint-refiner%22&type=issues)
> - [x] expires on Sep 19, 2026, 9:38 PM UTC-08:00

Contributor guide

Open the contributing guide

Research direction

Start with require-invalid-date-check-before-compare.ts, especially operand-side resolution around lines 340-362, and read require-invalid-date-check-before-compare.test.ts. Reproduce the actions/setup/js/check_rate_limit.cjs shape, then add coverage for the unguarded derived comparison and guarded variants; done means the missing case reports and valid guards do not.

Written by the indexing model from the issue text.

Assessment

Tech stack
javascript, typescript
Domain
tooling
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
76/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.