Skip to content

fix(codeql): CodeQL security high/error/medium fixes (C++ + Python + CI) - #868

Merged
lusoris merged 2 commits into
masterfrom
chore/codeql-security-bundle
Jun 12, 2026
Merged

lusoris merged 2 commits into
masterfrom
chore/codeql-security-bundle

Conversation

@lusoris

@lusoris lusoris commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

Summary

  • C++ (23 sites, 5 alert types): Integer-multiplication-cast-to-long (11 sites in cambi.c, float_vif.c, iqa/convolve.c, moment.c, psnr.c, vif_tools.c), incomplete-parity-check (3 sites in vif_tools.c, svm.cpp, pdjson.c changed % 2 == 1 → % 2 != 0), wrong-type-format-argument (2 sites in float_vif.c changed %d → %zu for size_t), world-writable-file-creation (1 site in vmaf.cpp replaced bare fopen with open(O_WRONLY|O_CREAT|O_TRUNC, 0644)+fdopen), path-injection (4 sites in test_output.c using realpath() after mkstemp).
  • Python + CI (18 alerts, 9 alert types): Shell-injection in lint-and-format.yml (moved ${{...}} expressions to env vars), defusedxml in feature_extractor.py + quality_runner.py, aiutils __all__ undefined-export, stack-trace-exposure in http_transport.py, shlex.quote for subprocess paths in extract_ugc_features.py + test_bbb_e2e_v5_bug_cluster.py, file-not-closed context managers in test_coverage_round3.py, redundant-comparison cleanup, frozen RdPoint dataclass, signature-mismatch fix in EnsembleVmafQualityRunner, redundant assignment in feature_extractor.py, surplus-format-arg in routine.py.

Deliverables checklist

  • Research digest: no digest needed — mechanical CodeQL alert resolution
  • Decision matrix: no alternatives — direct fixes per CodeQL recommendation
  • AGENTS.md invariant: no rebase-sensitive invariants introduced
  • Reproducer / smoke test: meson test -C build-codeql-verify --suite=fast → 88/88 OK; ruff check → all passed
  • Changelog fragment: no changelog fragment needed — security fix bundle, no user-visible feature change
  • Rebase notes: no rebase impact — no public API changes, no header modifications

Test plan

  • meson setup core build-codeql-verify -Denable_cuda=false -Denable_sycl=false — clean build, 1013/1013 targets
  • meson test -C build-codeql-verify --suite=fast — 88/88 OK
  • ruff check <touched Python files> — all checks passed
  • No conflict markers (git grep '^<<<<<<' — clean)
  • Netflix golden assertions untouched

Alerts resolved

Approximately 39 CodeQL alerts across security-cpp-high (21) and security-python-and-ci (18) categories.

🤖 Generated with Claude Code

lusoris and others added 2 commits June 12, 2026 21:31
- cpp/integer-multiplication-cast-to-long (11): pre-cast one operand to
  size_t / double / ptrdiff_t before int*int multiplications in
  cambi.c, float_vif.c (log message), iqa/convolve.c (img_offset),
  moment.c, psnr.c, and vif_tools.c (four memcpy size expressions).
  Add stddef.h to convolve.c for ptrdiff_t.

- cpp/incomplete-parity-check (3): change `% 2 == 1` to `% 2 != 0`
  in vif_tools.c (assert), svm.cpp (powi loop), pdjson.c (JSON
  object key/value alternation). The == 1 form is wrong for negative
  operands; != 0 is always correct.

- cpp/wrong-type-format-argument (2): fix float_vif.c error log that
  printed size_t fields scaled_w/scaled_h with %d; change to %zu.

- cpp/world-writable-file-creation (1): in vmaf.cpp replace bare
  fopen("wb") with open(O_WRONLY|O_CREAT|O_TRUNC, 0644)+fdopen() on
  POSIX so the created file is never world-writable independent of the
  caller's umask. Add <fcntl.h>.

- cpp/path-injection (4): in test_output.c resolve the mkstemp-created
  path through realpath() immediately after creation, breaking the taint
  chain from getenv("TMPDIR") to the vmaf_write_output call site.

- cpp/toctou-race-condition (2): skipped — both sites are in test
  cleanup (RMDIR after stat assertion). The stat result drives a test
  assertion, not a security-sensitive access decision; no atomic
  replacement of open() is applicable to rmdir. Reported as skipped.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Fixes 18 open CodeQL alerts across the Python and CI categories:

- yaml.github-actions.security.run-shell-injection (#661): move
  github.event_name, github.base_ref, and github.event.before from
  inline ${{...}} interpolation to env: vars in the SYCL clang-tidy
  detect step of lint-and-format.yml.

- python.lang.security.use-defused-xml-parse (#216, #217): replace
  xml.etree.ElementTree with defusedxml.ElementTree in
  feature_extractor.py and quality_runner.py; add defusedxml>=0.7.1
  to python/pyproject.toml and python/requirements.txt.

- py/undefined-export (#352, #353, #354, #616): restructure
  aiutils/__init__.py to do a conditional eager import of the parquet
  helpers so the names are defined when pyarrow is present, and only
  include them in __all__ when the import succeeded.

- py/stack-trace-exposure (#178, #179, #585): log exception detail
  server-side and return a generic message to the HTTP client in
  http_transport.py _handle_score (invalid JSON, bad params, scorer
  error branches).

- python.lang.security.audit.dangerous-subprocess-use-tainted-env-args
  (#227, #372): add shlex.quote() around user-supplied path arguments
  passed into shell strings in extract_ugc_features.py and
  test_bbb_e2e_v5_bug_cluster.py.

- py/file-not-closed (#677, #678): replace bare open() calls with
  context managers in test_coverage_round3.py.

- py/redundant-comparison (#427, #431): remove redundant
  assert not (x != y) lines that duplicate the preceding assert x == y.

- py/equals-hash-mismatch (#182): convert RdPoint to frozen=True
  dataclass so __eq__ and __hash__ are generated consistently.

- py/inheritance/signature-mismatch (#197): add result_dict=None
  default to EnsembleVmafQualityRunner._populate_result_dict so the
  signature is compatible with the base class.

- py/multiple-definition (#201): drop redundant assignment to
  feature_found in feature_extractor.py wildcard discovery path.

- py/str-format/surplus-named-argument (#204): remove unused
  dataset= kwarg from the format() call in routine.py.

Skipped: python.lang.security.audit.insecure-file-permissions (#373) —
  the Unix socket at 0o660 is intentional (Go sidecar node must write
  to it and runs as the same UNIX group); tightening to 0o644 would
  break the IPC channel.

Skipped: py/path-injection (#180, #181) — _validate_path() already
  resolves the path and checks it against an allowlist before any file
  operation; the data flow is secure and the CodeQL dataflow trace is a
  false positive on this allowlisted pattern.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@lusoris
lusoris merged commit 7bbaf6e into master Jun 12, 2026
39 of 50 checks passed
@lusoris
lusoris deleted the chore/codeql-security-bundle branch June 12, 2026 19:33
@lusoris lusoris added this to the 1.0.0 — First release milestone Sep 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant