require-invalid-date-check-before-compare misses arithmetic-derived duration variables (live gap in check_rate_limit.cjs)
- 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
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