test(push): harden the ingestion engine — validators, security guards, helpers#246
Merged
Merged
Conversation
…lpers Pure-additive tests from the CLI test-coverage audit (no production changes), using harnesses that already exist. Closes two P0s + several P1/P2 gaps: - labelSchemaType case/whitespace fallback (P0, label-diversity parity): a regression silently mis-collapses a case-mismatched VARCHAR label and false-rejects a diverse dataset the cluster accepts. - discoverSidecarFiles symlink-dir / symlink-entry / not-a-dir rejects (P0, security): the shared annotations//masks/ walker had 0% guard coverage — arbitrary-local-file-disclosure class. - CheckSchemaColumns missing-column rejection (P1). - TruncateList "… and N more" truncation (P1). - ParseMinSize grammar + surfaces the "min size" flag name (was 0%). - stderrSuffix remote-stderr parenthetical (was 0%). - FinalDestPrefix path-traversal panic guard (security). make ci green. First of a short series from the coverage backlog. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
Author
|
bugbot run |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 6ace36f. Configure here.
saadqbal
approved these changes
Jul 14, 2026
saadqbal
left a comment
Collaborator
There was a problem hiding this comment.
Nice, careful PR 👍 Verified each test against the real functions — the symlink and label-schema-type pins especially are worth having. LGTM.
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
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
First PR from the CLI test-coverage audit — hardens
internal/push(the correctness-critical ingestion/validation engine) with pure-additive tests: no production code changes, using harnesses that already exist. Closes two P0s and several P1/P2 gaps the audit surfaced.All tests live in one new file,
internal/push/coverage_hardening_test.go.What's covered
TestLabelSchemaTypelabelSchemaType(preflight.go:702)LabelDiversityValidator._schema_type_for. A regression here silently mis-resolves a case-mismatched label column's SQL type → mis-collapses a VARCHAR label → false-rejects a diverse dataset the cluster accepts.TestDiscoverSidecarFiles_SecurityGuardsdiscoverSidecarFiles(text.go:256)annotations/today andmasks/(semantic_segmentation) next, so a regression is an arbitrary-local-file-disclosure hole. Mirrors theimages/symlink pins inwalk_test.go.TestCheckSchemaColumnsCheckSchemaColumns(preflight.go:735)TestTruncateListTruncateList(preflight.go:440)TestParseMinSizeParseMinSize(detect.go:54)--min-sizeentry point (0%): grammar + that errors name themin sizeflag, nottarget size.TestStderrSuffixstderrSuffix(teardown.go:204)TestFinalDestPrefixFinalDestPrefix(spec.go:449)Not in this PR
ListDatasets(the 0%data listquery path) needs a small production seam first — it constructsSPDYExecutorinternally rather than accepting anExecutorparam likeTeardowndoes. That lands in the follow-up seam-refactor PR (which also fixesdata_list.go:118routing throughlistDatasetsFn).Test plan
make cigreen (build,-racetests, gofmt, lint, schema).walk_test.go.Part of the coverage-audit backlog — first of a short series (seam refactors + the no-infra batch next).
🤖 Generated with Claude Code
Note
Low Risk
Test-only additions; no runtime behavior changes.
Overview
Adds
internal/push/coverage_hardening_test.gowith pure test coverage forinternal/push—no production changes.The new tests pin
ParseMinSize(valid WxH grammar and errors that say min size, not target size),labelSchemaType(case/whitespace fallback for label SQL types),CheckSchemaColumns(missing columns, sorted names, case-sensitive headers), andTruncateList(offender-list truncation).They also lock in security and error UX:
discoverSidecarFilesrejects missing dirs, a file posing as a subdir, and symlinks (skipped on Windows);stderrSuffixformatting for remote stderr; andFinalDestPrefixpanicking on unsafe table names.Reviewed by Cursor Bugbot for commit 6ace36f. Bugbot is set up for automated code reviews on this repo. Configure here.