Skip to content

fix(config): decode urunc.json before validating mandatory fields - #886

Open
u7k4rs6 wants to merge 2 commits into
urunc-dev:mainfrom
u7k4rs6:fix/config-decode-before-validate
Open

fix(config): decode urunc.json before validating mandatory fields#886
u7k4rs6 wants to merge 2 commits into
urunc-dev:mainfrom
u7k4rs6:fix/config-decode-before-validate

Conversation

@u7k4rs6

@u7k4rs6 u7k4rs6 commented Aug 3, 2026

Copy link
Copy Markdown

Description

GetUnikernelConfig validated the urunc.json config while its values were
still base64 encoded, and only decoded them afterwards. The base64 decoder
skips CR and LF, so a field holding only newlines is non-empty when
validate() inspects it and decodes to the empty string immediately after.

The result is that a config whose three mandatory fields are newlines is
accepted with all three fields empty. Map() skips empty values, so those
keys never reach the state annotations, and every later Get on the container
fails with ErrNotUnikernel. Sixteen references in unikontainers.go read
those annotations back without rechecking them, so validate() is the only
gate in front of all of them.

This PR swaps the two calls so that validation runs on decoded values, and
adds fuzz coverage for the config path, which had none.

Two commits:

  • test(unikontainers) adds the fuzz targets on their own, so they can be
    read independently of the behaviour change.
  • fix(config) makes the change and adds the failing input to the seed
    corpus as a regression test.

Related issues

How was this tested?

  • make unittest passes.
  • golangci-lint in the project's v2.9 container reports no issues in any
    file this PR touches. It does report 4 pre-existing issues elsewhere, in
    cmd/urunc/create.go and pkg/containerd-shim/guest_rootfs.go, which are
    untouched here.
  • Both commits build and pass unit tests individually, so the branch bisects.
  • testdata/fuzz/FuzzConfigMandatoryFields/newline_decodes_to_empty fails on
    main and passes with the fix, under plain go test with no -fuzz flag,
    so the regression is deterministic in normal CI.
  • go test -fuzz FuzzConfigMandatoryFields and
    go test -fuzz FuzzGetUnikernelConfigJSON were each run for 60 to 90
    seconds against the fix with no new failures.

One caveat worth stating plainly. An earlier version of
FuzzConfigMandatoryFields composed validate() and decode() directly with
no filesystem access, and rediscovered this bug from neutral seeds in about
1.5 seconds. Driving the same property through GetUnikernelConfig is the
more meaningful invariant, but it costs roughly two orders of magnitude of
throughput, and in that form the fuzzer did not rediscover the input within 60
seconds. That is why the failing input is checked into the seed corpus rather
than left for the fuzzer to refind. If you would prefer the faster unit-level
target instead of, or alongside, the end-to-end one, I am happy to change it.

Scope note: only CR and LF bypass the check. Space and tab produce
illegal base64 data at input byte 0.

This is the urunc.json path only. I have not touched the spec annotation
path, since 5a654d3 establishes that those annotations arrive already decoded.

Checklist

  • I have read the contribution guide.
  • The linter passes locally (make lint).
  • The e2e tests of at least one tool pass locally (make test_ctr, make test_nerdctl, make test_docker, make test_crictl).
  • If LLMs were used: I have read the llm policy.

u7k4rs6 added 2 commits August 3, 2026 12:30
Add two native fuzz targets over the urunc.json config path. Both assert
that any config GetUnikernelConfig accepts carries a non-empty
unikernelType, hypervisor and binary, since sixteen references in
unikontainers.go read those straight out of the state annotations
without rechecking them.

FuzzConfigMandatoryFields supplies the four field values directly, so the
fuzzer mutates the base64 payloads instead of having to build the
surrounding JSON. FuzzGetUnikernelConfigJSON supplies arbitrary bytes to
the same entry point to cover parsing.

Signed-off-by: u7k4rs6 <utkarshbahuguna10@gmail.com>
GetUnikernelConfig validated the urunc.json config while its values were
still base64 and only decoded afterwards. The base64 decoder ignores CR
and LF, so a field holding only newlines is non-empty when validated and
decodes to the empty string.

A config whose mandatory fields are newlines was therefore accepted with
all three fields empty. Map() drops empty values, so the mandatory keys
never reach the state annotations and every later Get on that container
fails with ErrNotUnikernel.

Swap the order so that validation runs on decoded values. Add the case to
the FuzzConfigMandatoryFields seed corpus as a regression test.

Fixes: urunc-dev#885

Signed-off-by: u7k4rs6 <utkarshbahuguna10@gmail.com>
@netlify

netlify Bot commented Aug 3, 2026

Copy link
Copy Markdown

Deploy Preview for urunc canceled.

Name Link
🔨 Latest commit c990905
🔍 Latest deploy log https://app.netlify.com/projects/urunc/deploys/6a704793e3e00800086ee647

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

do-not-merge invalid This doesn't seem right

Projects

None yet

Development

Successfully merging this pull request may close these issues.

validate() runs before decode(), so base64 fields containing only CR/LF pass validation and decode to empty

2 participants