Skip to content

Testing & CI hardening: compile-the-output harness and coverage gaps (#48) - #53

Merged
aaylward merged 1 commit into
mainfrom
claude/issue-48-ndwegf
Jul 8, 2026
Merged

aaylward merged 1 commit into
mainfrom
claude/issue-48-ndwegf

Conversation

@aaylward

@aaylward aaylward commented Jul 8, 2026

Copy link
Copy Markdown
Collaborator

What

Works through the tractable core of #48, leading with the item the issue calls the single highest-leverage testing investment:

  • Compile-the-output harness (codegen/compile-tests/): a hostile "gauntlet" Smithy model packs Generator emits uncompilable C++ for legal-but-unusual models (escaping / name-collision class) #43's whole bug class into one fixture — C++ keyword member names (class, int, operator, …), enum values containing quotes/backslashes/newlines, the )__smithy" raw-string delimiter attack, int64-min @default/@range bounds, recursion through members and lists, keyword union variants, sparse collections. Bazel runs the real generator on it for all three protocols in both client and server mode, compiles the six resulting libraries, and a test spot-checks the escaping contract (trailing-underscore members, hostile enum round-trips, int32 extremes, boxed recursion). A generator escaping regression now fails bazel test //... on every CI platform instead of a consumer's build — and SmithyCppGenerate finally executes in a test rather than only being analysis-checked (also a Tracking: testing & CI hardening #48 item).
  • Curated hostile CBOR corpus (runtime/tests/cbor/cbor_hostile_test.cc), the counterpart of the vendored JSONTestSuite bank: systematic header/body truncations, reserved encodings, indefinite-length abuse, depth bombs, boundary integers and half-precision edge cases, plus an every-strict-prefix-rejects property. It immediately found a decoder bug: additional-information 31 on integers and tags (not well-formed per RFC 8949 §3.3) decoded as 0 / -1 / an ignored tag — the decoder now rejects all three.
  • Unit tests for the two untested runtime modules: core/uuid.cc (canonical form, version/variant bits, uniqueness, thread-local streams) and client/observability.cc (attempt observations for success/transport-error/retry, traceparent stamping, existing-header respect, application context, fresh-root fallback).
  • Deterministic regex ReDoS bound: regex_test.cc's 5-second wall clock is now a step-count assertion via a new counting Search(text, &steps) overload — the Pike VM's stamp dedup bounds work at program size × (input + 2), so a loaded runner can't flake it. The nested-quantifier bombs assert the same bound.
  • make verify / make verify-full: one-command local verification mirroring .github/workflows/ci.yml one target per job, replacing the eight scattered commands docs/development.md listed.
  • CI touch-ups: the fuzz smoke job now runs the regex harness (it existed but was never in the loop); clang-format covers codegen/compile-tests.

Remaining #48 items (golden self-ratification, coverage tooling, benchmark gating, union×{cbor,jsonRpc2} conformance cells, HTTP-parser fuzzing, malformed-server evenness, cross-platform sanitizers) stay open on the tracking issue.

Testing

  • bazel test //... passes locally (67/67), excluding only the Boost/Beast targets and //benchmarks/... because the sandbox's egress proxy blocks their archive downloads — the exact exclusion docs/development.md documents for proxied environments; CI runs them.
  • The new gauntlet harness generates + compiles + passes for all six protocol/mode combinations; the CBOR decoder fix is covered by the new corpus and leaves all 251 protocol-conformance vectors, the property tests, and the fuzz smoke tests green.
  • No Java changes, so goldens are untouched (git diff clean after generation was only run into bazel-bin).

Checklist

  • Tests added/updated for the change
  • bazel test //... passes locally (Boost/benchmark targets excluded per docs; CI covers them); codegen untouched, gradle job unaffected
  • Formatting clean (clang-format, buildifier; spotless N/A — no Java changes)
  • Architectural decisions recorded as an ADR (not applicable)

🤖 Generated with Claude Code

https://claude.ai/code/session_012VFUppN1idpmU1UrbHkknZ


Generated by Claude Code

Works through the tractable core of issue #48:

- Compile-the-output harness (codegen/compile-tests/): the generator runs
  inside the Bazel graph on a hostile gauntlet model — C++ keyword member
  names, quote/backslash/newline enum values, a raw-string delimiter attack,
  int64-extreme bounds and defaults, recursion through members and lists,
  keyword union variants, sparse collections — and the result is compiled
  for all three protocols in both client and server mode, then spot-checked
  for the escaping contract. Issue #43's bug class (generator emits
  uncompilable C++ for legal models) now fails CI instead of a consumer
  build, and SmithyCppGenerate finally executes in a test rather than only
  being analysis-checked.

- Curated hostile CBOR corpus (runtime/tests/cbor/cbor_hostile_test.cc), the
  counterpart of the vendored JSONTestSuite bank: systematic header/body
  truncations, reserved encodings, indefinite-length abuse, depth bombs,
  boundary integers and half-precision edge cases, and an
  every-strict-prefix-rejects property. It immediately found a decoder bug:
  additional-information 31 on integers and tags (not well-formed per
  RFC 8949 §3.3) decoded as 0 / -1 / an ignored tag — now rejected.

- Direct unit tests for the two untested runtime modules: core/uuid.cc
  (canonical form, version/variant bits, uniqueness, thread-local streams)
  and client/observability.cc (attempt observations for success/transport
  error/retry, traceparent stamping, existing-header respect, application
  context, fresh-root fallback).

- The regex ReDoS guard asserts a deterministic step count instead of
  5 wall-clock seconds: Search(text, &steps) instrumentation counts Pike VM
  work, bounded by program size x (input + 2), so a loaded CI runner cannot
  flake it. The nested-quantifier bombs assert the same bound.

- make verify / make verify-full: one-command local verification mirroring
  .github/workflows/ci.yml one target per job, replacing the eight scattered
  commands docs/development.md used to list.

- CI: the fuzz smoke job now includes the regex harness (it existed but
  never ran under libFuzzer); clang-format covers codegen/compile-tests.

Closes #48's compile-harness, CBOR-corpus, unit-test-gap, regex-flake, and
verify-entry-point items; golden self-ratification, coverage tooling,
benchmark gating, union x protocol conformance cells, HTTP-parser fuzzing,
and malformed-server evenness remain open on the tracking issue.

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012VFUppN1idpmU1UrbHkknZ
@aaylward
aaylward merged commit 3338b59 into main Jul 8, 2026
12 checks passed
aaylward pushed a commit that referenced this pull request Jul 8, 2026
…lformed-server coverage

Continues issue #48 after #53, taking the two hostile-input items:

- The hand-rolled HTTP/1.1 message reader — the one network-facing parser
  with no fuzz coverage — moves out of socket_transport.cc into a pure,
  callback-fed function (smithy/http/http1.h) behind the same transports,
  byte-for-byte behavior-preserving. On top of it:
  * //fuzz:http1_fuzz, a libFuzzer harness that replays the wire bytes in
    varying chunk sizes so the incremental header/body accumulation paths
    are exercised, wired into the CI fuzz smoke loop and `make fuzz-smoke`
    (and the deterministic-driver //fuzz:http1_fuzz_smoke variant that runs
    in every bazel matrix job).
  * http1_hostile_test.cc, a platform-independent hostile bank at the pure
    parser level: request-smuggling framing (duplicate/conflicting CL,
    transfer-encoding, TE+CL), hostile content-lengths, malformed header
    blocks, every-strict-prefix truncation, header floods, plus the
    accept-side (padding, case-insensitivity, body-until-EOF) and the
    start-line helpers. Unlike socket_transport_hostile_test.cc it needs no
    sockets, so it also runs on Windows.
  Hardening found while banking: an empty Content-Length and a "+4"-style
  signed one previously parsed as valid lengths (strtoull laxity); the
  parser now requires digits-only per RFC 9110.

- Malformed-server coverage was jsonrpc2-only (its model carries
  httpMalformedRequestTests; the alloy and official rpcv2Cbor suites carry
  none). New hand-written suites — outside the golden generated/ trees —
  pin the generated servers' reject paths before the handler runs:
  * simpleRestJson (PizzaAdminService): 404/405 routing, unparseable JSON
    body -> 400 + x-error-type SerializationException, wrong content-type
    -> 415, and the suite-exact enum-violation ValidationException message.
  * simpleRestJson @pattern-violation wire message (issue #48's explicit
    gap) via the roundtrip REST fixture's pattern-constrained SinkId,
    plus the exact length-violation message.
  * rpcv2Cbor (RpcV2Protocol): missing/wrong smithy-protocol header -> 400
    SerializationException, wrong content-type -> 415
    UnsupportedMediaTypeException, truncated and non-map CBOR bodies,
    404/405 routing — asserting the CBOR __type and protocol headers on
    every error response.

CI's clang-format check (and make lint/format) now covers protocol-tests
outside generated/.

Verified locally: bazel test //... green (72 tests; Boost/benchmark targets
excluded per docs/development.md's proxy note, CI covers them), plus a 15s
real-libFuzzer ASan run of the new harness (160k execs, no findings).

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012VFUppN1idpmU1UrbHkknZ
aaylward pushed a commit that referenced this pull request Jul 8, 2026
…overage tooling

Continues issue #48 after #53 and #54:

- Union x {rpcv2Cbor, jsonRpc2} conformance (protocol-tests/unions/): union
  round-tripping was only pinned for simpleRestJson; the other two cells
  relied on the seeded random integration tests, which flip a coin on
  whether the union appears at all and can only prove serde
  self-consistency — a symmetric encode/decode bug passes them. The new
  suites pin the wire subdocument for every SinkChoice variant
  deterministically, in all four directions per protocol (client encode,
  client decode, server decode, server echo), plus the reject cells
  (empty, multi-member, unknown-member, null-member, non-map unions), the
  exactly-one-member wire invariant, and the __type discriminator
  tolerance. Both suites were mutation-checked: one expectation flipped
  per suite fails the suite, so a first-run pass is not vacuous.

- Code-coverage tooling (issue #48's "module gaps only findable by manual
  srcs<->tests correspondence"): a coverage CI job runs bazel coverage
  with a combined lcov report over the runtime, prints the per-module
  summary in the log, and uploads the rendered HTML as a build artifact;
  `make coverage` runs the same locally. Measurement only, no threshold
  gate yet — same posture as the benchmarks job.

Verified locally: bazel test //... green (74 tests; Boost/benchmark targets
excluded per docs/development.md's proxy note, CI covers them), and
bazel coverage --combined_report=lcov produces a 21-file runtime report.

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012VFUppN1idpmU1UrbHkknZ
aaylward pushed a commit that referenced this pull request Jul 8, 2026
… pinning, macOS sanitizers

Continues issue #48 (after #53, #54, #55) and works through issue #56:

- Union member-type gauntlet (protocol-tests/unions/): SinkChoice only
  covers string/int/struct members, so a generated-in-graph UnionGauntlet
  service (same Bazel path as codegen/compile-tests) extends the union x
  {rpcv2Cbor, jsonRpc2} cells to blob, timestamp, list, map, enum, intEnum,
  and recursive-struct members. Each member kind is pinned with a
  hand-built typed value on one side and a hand-built wire document on the
  other — never both ends produced by the code under test — through client
  encode, the full client<->server loopback loop, and the wire subdocument.
  Protocol-specific representations are pinned where they differ (blobs as
  base64 text and timestamps as epoch-seconds numbers in JSON; blob and
  tag-1 timestamp nodes in CBOR).
- Byte-exact request vectors: the CBOR body pinned as hex derived by hand
  from RFC 8949's deterministic encoding rules, and the full JSON-RPC
  envelope pinned as its exact compact sorted-key text — closing the seam
  Document-level equality leaves between "codec is deterministic" and
  "serde feeds the codec what we think" (issue #56).
- Error-shape union cell: a modeled error whose union member rides next to
  its __type discriminator, inspected on the wire (the payload keeps both)
  and round-tripped into the typed error detail on the client — the case
  the serde's exactly-one-member arithmetic exists to tolerate. That
  contract is now documented in docs/generated-types.md, giving the
  tolerance test an authority beyond the implementation (issue #56).
- Reject-cell diagnosis pinning (issue #56): the phase-3 union reject cells
  now assert which rule was violated ("expected exactly one union member",
  "unknown or missing union member", "expected a map on the wire") on both
  client and server, not merely that a rejection happened.
- Sanitizers x platforms (issue #48): the asan+ubsan CI job is now a
  linux/clang + macos/apple-clang matrix, putting the transport layer's
  Apple-specific paths (SO_NOSIGPIPE, libc++) under a sanitizer for the
  first time. MSVC ASan under Bazel remains future work.

Also confirmed while scoping: the Gradle toolchain works in this
environment (gradle build + the Java unit suite pass), so the
generator-class unit-test item is unblocked for a next phase.

Model note: recursion *through* a union (Node carrying a BigUnion) is
rejected by the generator with a clear diagnostic (std::variant needs
complete alternatives); the gauntlet documents that boundary and covers a
self-recursive struct as a union member instead.

Verified locally: bazel test //... green (76 tests; Boost/benchmark targets
excluded per docs/development.md's proxy note, CI covers them).

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012VFUppN1idpmU1UrbHkknZ
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