Conversation
GenAI-Security-Project#62's review ran sixteen deliberately-broken copies by hand to confirm validate.py's checks actually fire, one error and a non-zero exit each, no false positives on the real files. None of that was committed, so a future change to validate.py could silently stop catching any of them and nothing would say so - the thing CI now runs on every PR would keep passing regardless. tests/fixtures/valid_minimal.json is the smallest trace that satisfies schema.json. Every other fixture in tests/fixtures/ is that same trace with exactly one field changed, named for the one thing it breaks: a span naming itself as its own parent, a dangling parent_span_id, a backwards t_offset_ms, a bearer token or an internal hostname or a credential-shaped key=value assignment or two routable-looking public IPv4 addresses in one string, a non-ISO date_added or span timestamp, a non-hypothetical tier with no evidence, a cve- or url-typed citation in the wrong shape, an evidence item missing supports, attestation: false, a dsgai_mapping id off the schema enum, and a filename that disagrees with its own trace_id. tests/test_validate.py runs validate.py as a subprocess against a fresh copy of schema.json/example.json/validate.py plus one fixture at a time, asserting the exit code and a message substring specific to that fixture - not by importing validate.py's functions, so a refactor of its internals can't quietter this suite while still changing behavior. test_every_fixture_file_is_covered checks the fixture directory and the test's own case list agree, so a fixture added without a matching case (or vice versa) fails loudly instead of testing nothing. Wired into the existing CI workflow as one more step; SHA-pinned actions and path filters are untouched from what was already reviewed.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Optional follow-up you suggested on #62: "with CI now running the validator, a small tests/ of known-bad fixtures would keep them from rotting."
tests/fixtures/valid_minimal.jsonis the smallest trace that satisfiesschema.json. Every other fixture is that same trace with exactly one field changed, named for the one thing it breaks: a span naming itself as its own parent, a danglingparent_span_id, a backwardst_offset_ms, a bearer token, an internal hostname, a credential-shapedkey=valueassignment, two routable-looking public IPv4 addresses in one string, a non-ISOdate_addedor spantimestamp, a non-hypothetical tier with no evidence, acve- orurl-typed citation in the wrong shape, an evidence item missingsupports,attestation: false, adsgai_mappingid off the schema enum, and a filename that disagrees with its owntrace_id- sixteen fixtures, matching the sixteen cases you ran by hand during review.tests/test_validate.pyrunsvalidate.pyas a subprocess against a fresh copy ofschema.json/example.json/validate.pyplus one fixture at a time, asserting the exit code and a message substring specific to that fixture. It doesn't importvalidate.py's functions, so a refactor of its internals can't quietly stop testing something while still changing behavior. A separate test checks the fixture directory and the test file's own case list agree, so a fixture added without a matching test case fails loudly instead of testing nothing.Wired into the existing CI workflow as one more step. The SHA-pinned actions and path filters from #62 are untouched.
Verification
Caught a real bug in the harness itself before trusting it: the first version invoked the real (empty)
validate.pyfrom the source tree instead of the copy staged into each fixture's temp directory, sincevalidate.pyresolves its paths from its own file location rather than from cwd. All sixteen cases were silently passing by testing nothing until the exit code (0, not 1) gave it away. Fixed; all 18 tests now genuinely exercise what they claim to.