Skip to content

ci: add CodeQL workflow building against PHP headers - #2720

Open
dunglas wants to merge 2 commits into
mainfrom
ci/codeql
Open

dunglas wants to merge 2 commits into
mainfrom
ci/codeql

Conversation

@dunglas

@dunglas dunglas commented Oct 16, 2026 •

Copy link
Copy Markdown
Member

CodeQL has not analyzed the repository since October 2024. Default setup is disabled, and its last C/C++ run failed because autobuild can't find the PHP headers.

This adds an advanced setup workflow that installs ZTS PHP through the existing setup-php action, exports php-config --includes as CGO_CFLAGS, and builds both the root module and caddy/ so the tracers see the C and Go compilations. It covers actions, c-cpp and go on pull requests, pushes to main, and weekly.

Notes:

  • The Go build cache is disabled for this job: a warm cache skips the compiler invocations CodeQL traces.
  • caddy/ is built from its own working-directory because the Go tracer doesn't recognize go -C.
  • CodeQL's Go extractor records files that use cgo (frankenphp.go, worker.go, …) under their generated copies in GOCACHE, so Go alerts in those files are dropped. C sources and pure Go files (including the whole Caddy module) are covered.
  • GitHub default setup must stay disabled, otherwise the two configurations conflict.

Running the default suites locally on top of this branch reports two issues, fixed separately:

@github-advanced-security

Copy link
Copy Markdown

You are seeing this message because GitHub Code Scanning has recently been set up for this repository, or this pull request contains the workflow file for the Code Scanning tool.

What Enabling Code Scanning Means:

  • The 'Security' tab will display more code scanning analysis results (e.g., for the default branch).
  • Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results.
  • You will be able to see the analysis results for the pull request's branch on this overview once the scans have completed and the checks have passed.

For more information about GitHub Code Scanning, check out the documentation.

@dunglas
dunglas marked this pull request as ready for review October 16, 2026 12:15
paths-ignore:
- "docs/**"
schedule:
- cron: "17 4 * * 1"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason for this? 😄

dunglas added a commit that referenced this pull request Oct 16, 2026
`num_threads`, `max_threads`, `max_requests`, the worker `num` and
`max_threads` options, and `php-server --worker file,N` are parsed with
`strconv.ParseUint(s, 10, 32)` and then converted to `int`. On 32-bit
platforms (we ship `linux/386`, `linux/arm/v6` and `linux/arm/v7`
images), values between 2^31 and 2^32-1 wrap to negative numbers:
`max_threads 4294967295` becomes `-1`, which means `auto`.

These values are now parsed with `strconv.Atoi` through a
`parseNonNegativeInt` helper. Nothing gets converted, so nothing can
wrap, and out-of-range input returns a range error on every platform.
Negative values are still rejected explicitly, because the runtime
treats them as sentinels (auto thread count, default worker count).
64-bit builds accept the same values as before.

Reported by CodeQL (`go/incorrect-integer-conversion`) while setting it
up in #2720.

This branch has not been deployed

No deployments
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.

3 participants