Skip to content

fix(native): the native image registers the API keys file's YAML model for reflection, so it starts with a named keys file - #371

Merged
bmscomp merged 1 commit into
mainfrom
fix/audit-02-native-keys
Oct 17, 2026
Merged

bmscomp merged 1 commit into
mainfrom
fix/audit-02-native-keys

Conversation

@bmscomp

@bmscomp bmscomp commented Oct 17, 2026

Copy link
Copy Markdown
Owner

Why

The native image can't start once a named API keys file is set. Checked in the code:

  • ApiKeyFile reads kates.api.keys-file with Jackson into ApiKeyFile.FileModel and EntryModel, and Jackson builds both by reflection. Nothing registers either class for the native image: not NativeReflectionConfig, not NativePayloadReflectionConfig, not META-INF/native-image/reflect-config.json. DisruptionPlaybookCatalog reads YAML the same way, and its classes are registered in NativePayloadReflectionConfig.
  • Case. Deploy the published 1.25.1-native image with values-native.yaml and set KATES_API_KEYS_FILE, which is how MCP agent keys are issued. readValue finds no constructor or fields, so the parse throws "the keys file is not YAML of the expected shape". ApiKeys.onStart turns that into an IllegalStateException, the Kates API does not start, and the pod crash-loops. The JVM image is unaffected, so no JVM test can show it.
  • Why CI never saw it. scripts/native-smoke-test.sh boots with KATES_API_SECURITY_ENABLED=false, and ApiKeys.onStart returns early when security is off, so the keys file is never read. No CI job has booted the native image with security on.
  • Why the reflection guard missed it. NativeReflectionRegistryTest leaves com/bmscomp/kates/security out of PAYLOAD_PACKAGES, but adding the package would not catch this either: isSerializedPayload skips every class that is not public, and ApiKeyFile, FileModel and EntryModel are all package-private.

The audit offered a second fix, listing both classes in NativePayloadReflectionConfig's targets next to the playbook classes. That doesn't compile: ApiKeyFile is package-private and the holder is in com.bmscomp.kates.config, so ApiKeyFile.FileModel.class can't be named there.

Changes

ApiKeyFile

  • @RegisterForReflection on FileModel and on EntryModel. Quarkus registers their package-private constructor and fields, and Jackson needs both.
  • The comment above them says why: "Read by Jackson, so the native image registers them."

scripts/native-smoke-test.sh

  • After the existing checks it boots the image a second time, with API security on and a keys file of one named key, the way a deployment issues MCP agent keys:
    • the key is kates_smoke001_native-smoke-only-<pid>, which matches ApiKeyFile.KEY;
    • the file holds one entry: id smoke001, name native-smoke, type human, scopes [read], and the key's SHA-256;
    • it is written with mktemp, then made 0644, because mktemp makes it 0600 and the image runs as a non-root user;
    • it is mounted read-only at /etc/kates/keys.yaml, with KATES_API_KEYS_FILE pointing at it.
  • The second boot checks that GET /api/tests answers 200 with Authorization: Bearer <key> and 401 without one.
  • The key is hashed by sha256sum inside the Postgres container, so the host still needs only docker and curl (macOS has no sha256sum).
  • The docker run flags became start_app, and the liveness wait became wait_live. Both boots call them, each passing its own flags through "$@", so the two boots can't drift apart. Only the first boot passes KATES_API_SECURITY_ENABLED=false.
  • The second boot reuses the ${APP} name, so cleanup() and fail() still cover it. cleanup() also removes the keys file, with ${KEYS_FILE:-} so set -u doesn't trip when the script exits before the file exists.
  • The header comment lists the second boot and its check.

kates/docs/native-image.md

  • The smoke-test paragraph describes the second boot.
  • "Keeping reflection registration honest" says that NativeReflectionRegistryTest scans only public classes in the payload packages, so a package-private model that Jackson reads, such as ApiKeyFile.FileModel, carries @RegisterForReflection itself.

Tests

  • No JVM test is added. The fix is two annotations, and a JVM test can't show that a class is missing from the native image's reflection registration. ApiKeyFileTest and ApiKeysTest still pass.
  • The new check is the second native smoke boot above. It also runs a security-on native boot for the first time in CI.
  • Run locally, all passing:
    • git diff --name-only origin/main...HEAD: 3 paths, kates/docs/native-image.md, kates/src/main/java/com/bmscomp/kates/security/ApiKeyFile.java and scripts/native-smoke-test.sh. They trigger ci.yml (kates), native.yml (kates/** and scripts/native-smoke-test.sh), the lint.yml scripts job, CodeQL (kates/src), integration.yml and ci-docker.yml. No cli, docs/**, charts, .github, Makefile or yml change.
    • cd kates && ./mvnw -o test -Dquarkus.grpc.codegen.skip=true -Dquarkus.http.test-port=0: 1,466 tests, 0 failures, 0 errors, 0 skipped.
    • spotless:check on JDK 25: clean.
    • cd cli && go test ./...: all packages ok, including the caveat ref anchors in cmd. No CLI ref points at ApiKeyFile.java, so the lines added there move no ref.
    • go vet ./..., golangci-lint run --new-from-rev=origin/main ./... (0 issues), govulncheck ./... (no vulnerabilities).
    • scripts/check-cli-style.sh, scripts/check-cli-compat.sh.
    • shellcheck -x -S warning -e SC2317,SC2034,SC2155 scripts/*.sh kates-ci.sh, the lint.yml scripts job's exact command, with shellcheck 0.11.0. bash -n on native-smoke-test.sh is also clean.
    • scripts/check-install-kates.sh, the scripts job's second step: all 11 checks ok.
  • Skipped locally:
    • the book checks, because nothing under docs/** changed (kates/docs/native-image.md is outside the docs.yml and book.yml filters);
    • the chart checks, because nothing under charts/** changed;
    • make check-help, actionlint and yamllint, because no Makefile, .github/** or yml file changed;
    • scripts/native-smoke-test.sh kates:native-ci, because it needs a Docker native-image build (about 20 minutes) and Docker stays off locally. This is the only check that runs the new code paths. Reviewed by hand instead: the key matches ApiKeyFile.KEY; a keys file with no kates.api.key passes SecurityStartupValidator; KATES_API_KEYS_FILE maps to kates.api.keys-file; postgres:16-alpine has sha256sum; and the docs note matches NativeReflectionRegistryTest's Modifier.isPublic filter.
  • Runs only in CI:
    • CI · Native image (native.yml): builds kates/Dockerfile.native and runs scripts/native-smoke-test.sh. It is the only check that runs the keys-file boot and the reflection fix;
    • Integration tests (integration.yml): the *IT tests and the Kind tests;
    • the Docker image build (ci-docker.yml);
    • CodeQL analysis of kates/src (codeql.yml);
    • the github-advanced-security AI scan, which fails on every PR for a GitHub-side reason.

Decisions

  • The classes are registered with @RegisterForReflection on FileModel and EntryModel, as PagedResponse does, not in NativePayloadReflectionConfig (see Why).
  • The native smoke test gets the second, security-on boot with a one-key keys file: 0644, hashed with sha256sum inside the container. The alternative was the annotations alone, with one native deploy checked by hand.
  • NativeReflectionRegistryTest is not changed. Scanning com/bmscomp/kates/security wouldn't catch this bug, because the scan skips classes that are not public. It would flag only public classes that are never serialized (ApiKeyAuthenticationRequest, KatesIdentities, Scopes, KatesPrincipal), and each would need a NOT_SERIALIZED exemption. Widening the scan to package-private classes in all 9 packages isn't worth it either: ApiKeyFile.FileModel is the only readValue/convertValue target in kates/src/main/java that isn't public.

Deliberately not done

  • No native build and no smoke run here, because Docker is off locally. The Native image job is the first real proof that the annotations work and that a security-on native boot starts. That first boot may fail somewhere else, for example in the auth mechanism, the gRPC interceptor or a startup validator. If it does, that fix goes in a separate PR.
  • The keys file the script writes wasn't rendered and parsed on the JVM. Its shape follows the ApiKeyFile javadoc and its parse rules: an id matching ^[a-z0-9]{4,32}$, a valid name, type human, scope read, and 64 lower-case hex characters.
  • The second boot adds about 10 to 30 s to the Native image job.

Landing

This PR shares no file with the earlier plan PRs, so it can land at any time.

…l for reflection, so it starts with a named keys file

Jackson reads kates.api.keys-file into ApiKeyFile.FileModel and EntryModel by reflection, and nothing registered either class for the native image. Deploying the 1.25.1-native image with KATES_API_KEYS_FILE set, which is how MCP agent keys are issued, made readValue throw, so ApiKeys.onStart refused the file as "not YAML of the expected shape" and the pod crash-looped. The JVM image was unaffected, so no JVM test could show it. Both classes now carry @RegisterForReflection, which registers their package-private constructor and fields.

The native smoke test never reached this path: it boots with KATES_API_SECURITY_ENABLED=false, and with security off the keys file is never read. After its existing checks it now boots the image again with security on and a keys file of one human read key, written 0644 so the image's non-root user can read it, and checks that GET /api/tests answers 200 with that key and 401 without one. The docker run flags and the liveness wait became start_app and wait_live, shared by both boots. The key is hashed inside the Postgres container, so the host still needs only docker and curl.

kates/docs/native-image.md describes the second boot, and says that NativeReflectionRegistryTest scans only public classes in the payload packages, so a package-private model that Jackson reads carries @RegisterForReflection itself.
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