Skip to content

Truthiness/exception-handling bugs allow silent validation bypass and uncaught crashes in validate.py #408

Description

@jbonofre

Found while reviewing #373. None of these are regressions introduced by that PR — they're pre-existing bugs in code adjacent to what it touches, and outside its diff hunks — but they're the same bug classes #373 fixes for the "Warning:" text-matching case (silent bypass via a shortcut check, and uncaught exceptions crashing the validator).

1. validate_references silently skips empty-string dataset references (validate.py:221)

if from_ds and from_ds not in datasets:

The truthiness check (and the equivalent to_ds check) skips the unknown-dataset
check whenever from/to is "". Since the schema has no minLength constraint,
this is schema-valid and currently passes validation:

{"from": "orders", "to": "", "from_columns": ["customer_id"], "to_columns": ["id"]}

"" is never a real dataset — empty-named datasets are excluded from the datasets
map — yet no error is raised.

Fix: check is not None (or just always run the not in datasets check)
instead of relying on truthiness.

2. validate_unique_names silently skips empty-string names (validate.py:180)

dataset_names = [d.get("name") for d in datasets if d.get("name")]

Same truthiness-filter pattern (repeated for fields/metrics/relationships below)
drops empty-string names before duplicate detection runs. Two datasets both named
"" are schema-valid and currently pass with no duplicate-name error.

Fix: filter on is not None instead of truthiness.

3. validate_sql_expression only catches ParseError/TokenError (validate.py:295)

Other exceptions sqlglot can raise propagate uncaught and crash main() with a raw
traceback instead of a [SQL] diagnostic.

Repro: a SQL expression with ~5000 nested ( raises RecursionError from
sqlglot.parse_one, uncaught.

Fix: broaden the catch (or catch Exception and re-tag as [SQL]) so malformed
input can't take down the whole validator run.

4. main() YAML load only catches yaml.YAMLError (validate.py:384)

A RecursionError from PyYAML's scanner/composer on deeply nested input propagates
uncaught.

Repro: a file with ~3000 nested [ crashes with a raw multi-frame traceback instead
of "Error: Invalid YAML: ...". Exit code stays non-zero either way, but tooling/humans
expecting clean diagnostic output on stdout get a stack trace instead.

Fix: widen the catch here as well.

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions