Skip to content

ci: stamp the version in the scan image and record that CVE-2026-33503 does not apply - #16

Merged
dmakarenko merged 3 commits into
goodnotesfrom
courier-pagination-sqli-fix-20261002
Oct 2, 2026
Merged

dmakarenko merged 3 commits into
goodnotesfrom
courier-pagination-sqli-fix-20261002

Conversation

@dmakarenko

Copy link
Copy Markdown

The CI image scan now sees the Kratos module: the docker target of the Makefile passes VERSION from git describe, cve-scan.yaml checks out the full history, and .trivyignore gets CVE-2026-33503 with its reason.

Trivy flags the published v1.3.0-gn-v11 image for CVE-2026-33503, but the fork is not affected: the flaw is in keysetpagination_v2 of ory/x (Kratos v25.4.0 and later), and ory/x v0.0.660 does not have that package.

Tested with a new forged page token case in courier/handler_test.go (it fails under a mutation that puts token content into the statement) and with local Trivy scans of a make docker image: no stamp gives 0 findings, the stamp gives the CVE and exit 42, the stamp plus the ignore entry gives exit 0.

https://claude.ai/code/session_01LbDrkZWVmFoiWnumt6MpWU

…tement

CVE-2026-33503 (GHSA-hgx2-28f8-6g2r) is an SQL injection through a
forged page token of the ListCourierMessages Admin API. Its cause is
the encrypted-token pagination package (ory/x keysetpagination_v2):
that package builds the WHERE and ORDER BY clauses from the column
names inside the token. Upstream moved the courier list to that
package in 9931ea5 (first release v25.4.0) and added the column
check in e006333 (first release v26.2.0).

This fork is v1.3.0 with ory/x v0.0.660, and keysetpagination_v2 does
not exist there. The courier list takes its column names from the
server (WithColumn and the model columns) and binds the token values
as query parameters. The advisory does not apply.

The test sends two forged tokens to the endpoint: one with an unknown
column name and one with an unbalanced quote in a column value. Each
must return 200 with the recipient filter in effect.

The test passed on the first run, because no defect exists. A
temporary mutation of ListMessages proved that it can fail: with the
token column names in the statement, the column name case returned
500 (no such column), and with the token values in the statement, the
column value case returned 500 (syntax error).

Claude-Session: https://claude.ai/code/session_01LbDrkZWVmFoiWnumt6MpWU
The CI scan could not see a CVE in the Kratos module itself. Trivy
takes the version of the main module from the config.Version linker
flag, because .dockerignore excludes .git. publish-image.yml passes
VERSION, but the docker make target did not. So the published image
v1.3.0-gn-v11 shows CVE-2026-33503 (HIGH), and the scan run on the
same tag showed 0 findings for usr/bin/kratos.

Scan gate changes:
- Makefile: the docker target passes VERSION from
  `git describe --tags --always`.
- cve-scan.yaml: the checkout uses fetch-depth 0. With a shallow
  checkout the version is a bare commit hash, and Trivy does not match
  a bare hash.
- .trivyignore: adds CVE-2026-33503. The fork is not affected: the
  flaw is in ory/x keysetpagination_v2 (Kratos v25.4.0 and later), and
  ory/x v0.0.660 does not have that package. The test in
  courier/handler_test.go pins it. Remove the entry when the fork
  moves to an upstream version that has keysetpagination_v2.

Local Trivy scans of a `make docker` image, with the options of
cve-scan.yaml:
- no stamp: 0 findings, exit 0
- stamp, no ignore entry: CVE-2026-33503, exit 42
- stamp and ignore entry: 0 findings, exit 0
- a bare commit hash as the version: 0 findings, exit 0

Claude-Session: https://claude.ai/code/session_01LbDrkZWVmFoiWnumt6MpWU
…e zone

The "column name" case built its created_at bound from the clock in
UTC. SQLite compares created_at as text, and pop stores it with the
local offset of the machine. At UTC+2 or farther east each stored row
sorted after the bound, so the list was empty and the test failed with
0 items in place of 2.

The bound is now the fixed far-future value that
Message.DefaultPageToken also uses, and the value of the unknown
column is a constant, because only its name has an effect.

Before: TZ=Europe/Berlin failed 2 of 4 subtests. After: 4 of 4 pass
with TZ set to Europe/Berlin, Asia/Shanghai, UTC, America/Los_Angeles
and Pacific/Kiritimati.

Claude-Session: https://claude.ai/code/session_01LbDrkZWVmFoiWnumt6MpWU
@dmakarenko
dmakarenko merged commit 52fa689 into goodnotes Oct 2, 2026
12 of 21 checks passed
@dmakarenko
dmakarenko deleted the courier-pagination-sqli-fix-20261002 branch October 2, 2026 09:21
@dmakarenko

Copy link
Copy Markdown
Author

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