Skip to content

test(fuzz): add fuzz targets for secret transforms and data hashing - #340

Merged
BryanFRD merged 4 commits into
mainfrom
test/fuzzing
Oct 31, 2026
Merged

BryanFRD merged 4 commits into
mainfrom
test/fuzzing

Conversation

@BryanFRD

Copy link
Copy Markdown
Contributor

Go native fuzz targets plus a nightly workflow (300s per target, manual dispatch too, not on PRs).

  • FuzzApplyTransforms: all transform kinds on arbitrary keys/values. Asserts the input map is never mutated, errors are *TransformError, results are deterministic, prefix/suffix/base64Decode keep their values.
  • FuzzHashSecretData: the content hash must not depend on map construction order.

Also fixes a panic found while reading the CLI: parse_pin sliced a 64-byte non-ASCII pin mid-character in hex_decode. Separate commit with a regression test.

Ran locally: FuzzApplyTransforms 66s (394k execs), FuzzHashSecretData 62s (1.0M execs), no failures. go test ./internal/..., cargo test, cargo clippy -D warnings pass.

CONTRIBUTING requires an issue reference; none exists for this change.

Signed-off-by: BryanFRD <bryanferrando59@gmail.com>
Signed-off-by: BryanFRD <bryanferrando59@gmail.com>
@BryanFRD
BryanFRD enabled auto-merge (squash) October 31, 2026 10:16
Comment thread .github/workflows/fuzz.yml Fixed

@ferrfleet-interne ferrfleet-interne Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

One blocking issue: FuzzApplyTransforms can fail spuriously when key == "OTHER".

Signed-off-by: BryanFRD <bryanferrando59@gmail.com>
@ferrfleet-interne

Copy link
Copy Markdown

Changes requested: FuzzApplyTransforms has a false-positive path that would make the nightly fuzz job fail on a non-bug.

Blocking

  • internal/controller/transforms_fuzz_test.go:31: the input map is {key: val, "OTHER": "o"}. When the fuzzed key is "OTHER", the literal keeps only "o", so val is never in the input. The prefix, suffix and base64Decode assertions then compare against val and fail. Fix: add if key == "OTHER" { t.Skip() } after computing typ.

Suggestions

  • internal/controller/transforms_fuzz_test.go:43: use errors.As instead of err.(*TransformError), so the check keeps working if ApplyTransforms ever wraps its error. For example if te := (*TransformError)(nil); !errors.As(err, &te) { ... }, with errors imported.

Rest of the change

The review found the Rust hex_decode fix in cli/src/tls.rs (non-ASCII input now returns an error instead of panicking, with a test) and the fuzz.yml workflow correct.

Formal verification

The formal verification step failed and returned no result, so nothing was verified formally.

@ferrfleet-interne ferrfleet-interne Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Still blocking: c0b956a only touches the workflow pin; the key == "OTHER" false positive in FuzzApplyTransforms (line 31) is not addressed yet.

@ferrfleet-interne

Copy link
Copy Markdown

Changes requested: FuzzApplyTransforms can fail on inputs that are not real bugs, which would turn the nightly fuzz job red and upload a bogus corpus entry.

This is a follow-up review of c0b956a. Only the upload-artifact pin comment changed since the last pass, so the earlier blocking issue is still open.

Blocking

  • Spurious failure when key == "OTHER" (internal/controller/transforms_fuzz_test.go:31). The input literal {key: val, "OTHER": "o"} collapses to {"OTHER": "o"} when the fuzzer picks "OTHER" as the key. The prefix, suffix and base64Decode assertions then compare against a val that was never in the input. Fix: add if key == "OTHER" { t.Skip() } right after picking typ.

Suggestions

  • Use errors.As for the TransformError check (transforms_fuzz_test.go:43). The plain type assertion breaks if ApplyTransforms ever wraps its error. Use if te := (*TransformError)(nil); !errors.As(err, &te) { ... } and import errors.

Formal verification

The formal verification step failed and returned no result, so nothing is verified there. It is not a finding against the PR.

Signed-off-by: BryanFRD <bryanferrando59@gmail.com>

@ferrfleet-interne ferrfleet-interne Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Approve: 29cbe9f fixes the key == "OTHER" false positive; nothing blocking left.

@ferrfleet-interne

Copy link
Copy Markdown

Ready to merge. The review approved at 29cbe9f with no open findings. Formal verification did not run: that step failed and returned no result.

Review

  • 29cbe9f adds t.Skip() for key == "OTHER" in FuzzApplyTransforms. That was the only blocking finding: the fuzz target put OTHER in the input map alongside the fuzzed key, so the two entries collided and the test reported a false positive.
  • The non-ASCII fix in cli/src/tls.rs (hex_decode now rejects non-ASCII input instead of panicking on a byte slice) and its regression test check out.
  • The remaining fuzz assertions were checked against transforms.go and hold: input is not mutated, errors are *TransformError, results are deterministic, and prefix, suffix and base64Decode keep the value.

Formal verification

Property Result
All Not run (verification step failed, no result)

The fuzz workflow (.github/workflows/fuzz.yml) runs nightly and on manual dispatch, 300s per target, and uploads failing inputs as artifacts. It has not been exercised on this PR.

@BryanFRD
BryanFRD merged commit 02799fc into main Oct 31, 2026
22 checks passed
@BryanFRD
BryanFRD deleted the test/fuzzing branch October 31, 2026 10:28
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.

2 participants