Skip to content

ci(codeql): add pre-merge schema-validation pin for .github/codeql/config.yml #236

@cagataycali

Description

@cagataycali

Problem

.github/codeql/config.yml ships a query-filters: [{exclude: {id: py/unsafe-cyclic-import}}] block whose schema correctness is currently validated only out-of-band: the next CodeQL run on main either auto-closes the targeted alerts (#85, #86, #87, #253, #254, #255) or it doesn't. A typo at the schema level -- excludes: for exclude:, id: becoming a list, or the rule id misspelled -- silently no-ops the suppression with no signal at PR time.

The regression contract added in #216 (tests/simulation/test_no_cyclic_imports.py + the existing test_no_import_cycle.py) pins runtime safety, not CodeQL behaviour. A schema-level typo in config.yml would not trip either pin -- it only manifests as alerts re-appearing in the Security tab on the next scheduled scan.

This is the same shape as the dependabot.yml wrong-path bug (#235): a config file that looks right, parses as YAML, but is silently ignored by the consumer. PR #216 R5 review surfaced this concern explicitly.

Proposed fix

Add a small unit test (tests/test_codeql_config_schema.py or similar) that:

  1. Loads .github/codeql/config.yml via yaml.safe_load.
  2. Asserts the query-filters block matches the expected shape:
    import yaml
    from pathlib import Path
    
    def test_codeql_config_query_filter_shape():
        config = yaml.safe_load(Path('.github/codeql/config.yml').read_text())
        filters = config.get('query-filters', [])
        assert len(filters) >= 1, 'config.yml: query-filters block missing'
        exclude = filters[0].get('exclude')
        assert exclude is not None, "config.yml: first query-filter must be an 'exclude' (not 'excludes' or another key)"
        assert exclude.get('id') == 'py/unsafe-cyclic-import', (
            f"config.yml: exclude.id must be 'py/unsafe-cyclic-import', got {exclude.get('id')!r}"
        )
  3. (Optional) Validates the rest of the config's top-level keys against a known allowlist (name, disable-default-queries, queries, paths-ignore, paths, query-filters) so an accidental typo at the top level (querie-filters:) also fails loud.

The test should fail with a diagnostic that points at the config file and names the actual key found, so the failure mode is debuggable without reading the test source.

Acceptance criteria

  • Test exists and pins the expected shape (exclude.id == 'py/unsafe-cyclic-import').
  • Test fails loudly with diagnostic naming the actual key on each schema mutation:
    • excludes instead of exclude
    • id value misspelled (e.g. py/cyclic-unsafe-import)
    • id becomes a list instead of a string
    • query-filters becomes queries-filter (top-level typo)
  • Test runs in the same suite as tests/simulation/test_no_cyclic_imports.py (or wherever the CodeQL pin tests live).
  • AGENTS.md compliance: ASCII clean, no emojis in code/diagnostic strings.
  • Referenced from .github/codeql/README.md so future readers know the schema is pin-tested.

Why this issue (not bundled into #216)

Context


Filed by autonomous agent. Strands Agents.

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type
    No fields configured for issues without a type.

    Projects

    Status
    Backlog

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions