sequelize / sequelize/sequelize

Audit the v7 Oracle dialect port (#18050) for validations dropped from v6

Open
#18,283 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

dialect: oracle pending-approval security type: bug
Dominant language
TypeScript
Stars
30.4k
Forks
4.3k
Avg merge
1d 6h
Merged PRs (30d)
68

Description

Why

Two separate defects in the v7 Oracle dialect have now been traced to the same cause: the v7 port
(3a92f263a, #18050) reproduced the shape of v6 code while dropping behaviour that v6 relied on.

  1. A date-literal guard lost its value check. v6 validated the parsed date via a moment round-trip
    before accepting a TO_DATE/TO_TIMESTAMP_TZ literal. The v7 port kept the structural checks and
    dropped the value check. git log --all -S "Invalid date value for TO_DATE" returns only the v6
    commit.
  2. DATEONLY inline escaping became infinitely recursive. v6 called options.escape; v7 calls
    this.escape, which re-enters toBindableValue with no terminating branch. The practical effect is
    that any inline (non-bind) DATEONLY comparison on Oracle throws
    RangeError: Maximum call stack size exceeded — i.e. the feature has never worked on v7.

Both were found independently, by different routes, within the same week. Neither was found by tests.
Two confirmed regressions from one port is a pattern rather than a coincidence, and the sensible
assumption is that there are more.

What to do

A systematic behavioural diff of the v6 and v7 Oracle dialects — not a code re-read, which is what
missed these in the first place. Suggested approach:

  • For each method the port touched, compare v6 and v7 output for the same inputs, including the
    edge/invalid inputs v6's guards were written to reject. The interesting cases are the ones where v6
    threw and v7 returns something.
  • Pay particular attention to anywhere v6 called a validation helper (moment, options.escape, a
    format assertion) and v7 inlined or replaced it.
  • git log -S on the distinctive strings in v6's guards is a cheap way to find checks with no v7
    counterpart — it is how both known cases were confirmed.

Test-coverage gap to close alongside

Both regressions survived because of the shape of the Oracle test suite rather than its size:

  • No test anywhere puts a DATEONLY value in a where clause, on any dialect. The only Oracle
    DATEONLY coverage exercises the bind path (create, findOne({ where: { id } })), which never
    reaches inline escaping.
  • The date-literal guard is exercised only through an Oracle integration job, so it is invisible to
    the always-on unit run.

Adding inline/where-path coverage for the Oracle data types would have caught both, and is worth doing
regardless of what the audit turns up.

Related

  • #18270 — fixes both known regressions. Open at time of writing.
  • #18216 — superseded by #18270.
  • #18050 — the port under audit.

Created by Opus 5 with Claude Code, supervised by @WikiRik.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with the v6 and v7 Oracle dialect methods touched by port #18050, comparing their outputs for matching valid and invalid inputs. Review the existing Oracle integration coverage and DATEONLY bind-path tests, then add coverage for inline DATEONLY values in where clauses. Done means the audit's behavioral differences are identified and the relevant edge cases are covered by tests.

Written by the indexing model from the issue text.

Assessment

Tech stack
sql, typescript
Domain
database
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.