Skip to content

Fix the currently-failing CodeQL workflow and run it on pull requests#4980

Open
matthargett wants to merge 3 commits into
bytecodealliance:mainfrom
rebeckerspecialties:ci/fix-codeql-and-run-on-prs
Open

Fix the currently-failing CodeQL workflow and run it on pull requests#4980
matthargett wants to merge 3 commits into
bytecodealliance:mainfrom
rebeckerspecialties:ci/fix-codeql-and-run-on-prs

Conversation

@matthargett

Copy link
Copy Markdown

Summary

The CodeQL workflow is currently failing on main — the nightly Analyze (cpp) job cannot finish — for two independent reasons. This PR fixes both. Secondarily, it starts running CodeQL on pull requests so the analysis gates a change before it merges instead of only after.

The two fixes (the point of this PR)

1. codeql_buildscript.sh — stale libtinfo5 URL breaks the wamrc link.
The script hard-codes libtinfo5_6.3-2ubuntu0.1_amd64.deb, which now 404s because the ubuntu-22.04 runner's ncurses moved to 6.3-2ubuntu0.2. With libtinfo5 missing, linking wamrc against the prebuilt LLVM 18.1.8 (libomptarget.rtl.amdgpu, which references the NCURSES_TINFO_5 symbols) fails:

/usr/bin/ld: .../libomptarget.rtl.amdgpu.so.18.1: undefined reference to `setupterm@NCURSES_TINFO_5.0.19991023'
collect2: error: ld returned 1 exit status
Failed to build wamrc

Fixed by resolving the libtinfo5 compat package at the runner's actual ncurses version (matching the installed libtinfo6), with a pool-scrape fallback so it survives future point-release bumps.

2. codeql_fail_on_error.pyKeyError: 'rules' on current CodeQL SARIF.
The gate reads run["tool"]["driver"]["rules"] and, when that is empty (e.g. a clean scan), falls back to run["tool"]["extensions"][0]["rules"] — but current CodeQL emits a SARIF whose first extension has no rules key, so the script raises KeyError: 'rules' and the step fails. Fixed to read the driver rules defensively and otherwise gather rules from the extensions (which also lets it see error-level rules a query pack contributes).

Either fix alone leaves the job red; together they get Analyze (cpp) green again.

Secondary: run CodeQL on pull requests

codeql.yml runs only on push: dev/**, the nightly cron, and workflow_dispatch, so a change isn't scanned until after it merges. This adds the pull_request event.

Uploading results and reading code-scanning alerts needs security-events: write, and a pull request from a fork gets a read-only GITHUB_TOKEN, so the job would fail on fork PRs. To avoid a spurious red check, it runs on pull_request only when the head branch is in this repository:

if: >-
  (github.event_name == 'pull_request' &&
   github.event.pull_request.head.repo.full_name == github.repository)
  || (github.event_name != 'pull_request' &&
      github.repository == 'bytecodealliance/wasm-micro-runtime')

Non-pull_request events keep the original "upstream repo only" behavior. A concurrency group is added; only a pull_request cancels its own superseded run, while dev/** pushes and the nightly are left to finish so branch and scheduled scans are never dropped.

Consequence: external contributors' fork-head PRs will show the CodeQL job as skipped (not failed) — a read-only token can't upload code-scanning results — so those PRs stay covered by the nightly, while maintainer / in-repo branch PRs get the full pre-merge scan.

Verification

Verified end-to-end on a same-repo pull request in a fork (a fork PR is needed because CodeQL upload requires a writable token, which fork-head PRs lack): with both fixes, Analyze (cpp) completes the build, performs the analysis, uploads SARIF, and the Fail if an error is found gate passes. codeql_buildscript.sh and codeql_fail_on_error.py here are byte-identical to main, so these fixes apply unchanged; the codeql.yml change is layered on top of the current file (action pins untouched).

The hard-coded libtinfo5_6.3-2ubuntu0.1_amd64.deb URL 404s now that the
GitHub runner's ncurses has moved to 6.3-2ubuntu0.2, so libtinfo5 is not
installed and the wamrc link against the prebuilt LLVM 18.1.8
(libomptarget.rtl.amdgpu, which needs the NCURSES_TINFO_5 symbols) fails
with undefined references. Resolve the libtinfo5 compat package at the
same ncurses version as the runner's installed libtinfo6, with a
pool-scrape fallback, so it keeps working across point-release bumps.
codeql_fail_on_error.py crashed with `KeyError: 'rules'` whenever the
CodeQL driver's rule list was empty (e.g. a clean scan) and the first
tool extension carried no "rules" key - the shape current CodeQL
versions emit. Read the driver rules defensively and, when absent,
gather rules from all extensions, so the gate no longer aborts and can
still see error-level rules contributed by a query pack.
codeql.yml only runs on push to dev/**, a nightly cron, and manual
dispatch, so contributor changes are not scanned until after they merge.

Add the pull_request event and a concurrency group. The analysis uploads
results and reads code-scanning alerts, which requires
security-events:write; a pull request from a fork runs with a read-only
GITHUB_TOKEN and would fail those steps, so the job now runs on
pull_request only when the head branch lives in this same repository.
Other events keep the original behavior of running only on the upstream
repository. Only pull_request runs cancel their own superseded run;
branch and scheduled scans are left to finish.
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