Skip to content

A cdclint:ignore marker on the line that adds a column records a decision, so the finding stops failing the run - #21

Open
avison9 wants to merge 3 commits into
mainfrom
feat/ignore-marker
Open

avison9 wants to merge 3 commits into
mainfrom
feat/ignore-marker

Conversation

@avison9

@avison9 avison9 commented Sep 26, 2026

Copy link
Copy Markdown
Owner

What it changes

Leaving a PII column off the include list is right, and schema-before-connector raises it all the same. So RefuseRadar keeps its PR gate at --fail-on error, and a forgotten column does not block either. A per-rule filter (#19) cannot tell the two apart; the decision has to be recorded against the column, where the pull request that makes it shows it:

ALTER TABLE users
    ADD COLUMN ssn_hash TEXT, -- cdclint:ignore schema-before-connector: PII, never streamed
    ADD COLUMN nickname TEXT;

The ssn_hash finding stops failing the run and is listed with its reason; nickname is still raised.

Proven on RefuseRadar

The probe pattern (a column added to reports in a temporary migration, removed afterwards), with the gate RefuseRadar wants:

$ cdclint ... --base origin/main --fail-on warning --disable source-column-not-captured
warning schema-before-connector db/migrations/9999_cdclint_probe.sql:2
  this change adds public.reports.cdclint_probe_pii to a captured table without adding it to column.include.list ...
  fix: add ... in the same change, or, if it is left off on purpose, say so on the line that adds it: -- cdclint:ignore schema-before-connector: <why>
exit=1

# the same line with: -- cdclint:ignore schema-before-connector: probe, PII left off on purpose
ok: nothing to report outside the disabled rules and what cdclint:ignore acknowledges
acknowledged (cdclint:ignore): schema-before-connector db/migrations/9999_cdclint_probe.sql:2: probe, PII left off on purpose
not shown (--disable): source-column-not-captured 161
exit=0

The rules of a marker

  • Scope. After code, it covers its own line only; alone on a line, it covers the line below. Writing ignore-marker-pii's expectation by hand caught the first version, "this line or the next", acknowledging nickname through the ssn_hash marker: the forgotten column the gate exists for.
  • One or more rules, comma-separated; -- or MySQL's #; in migrations and sink DDL (down migrations skipped).
  • The reason is required: it is the record of the decision.
  • Misuse is a warning (ignore-marker, in engine.Rules so it can be --disabled): no reason, a rule name that does not exist, or a marker covering no finding.
  • Never "unused" for schema-before-connector: that rule judges the change, so its marker goes quiet once the pull request merges; nor for a rule that did not run (no --base, or disabled). The README shows naming both rules to keep the column out of the inventory afterwards.
  • Output. Acknowledged findings do not count for the exit code and are listed with their reason; JSON keeps its array and notes them on stderr, as --disable does. When everything was acknowledged or disabled, the output says so rather than that the files agree.
  • The diff rule's fix now points at the marker instead of "let this warning stand as the record of that (it blocks only under --fail-on warning)".

Verified

  • Corpus, expectations written by hand: ignore-marker-pii (the diff rule, one column acknowledged and one raised) and ignore-marker-mistakes (no reason, an unknown rule, a stale marker, and a marker on the line above a finding). The corpus test now runs through the same lint() as the command line.
  • The fix-text change regenerated 7 fix lines in 5 diff entries; the diff was read and changes nothing else.
  • internal/ignore tests: both comment styles, a colon-less marker has no reason, down files not scanned, trailing vs standalone scope, and which unused markers are reported.
  • gofmt -l . clean, go vet, go test ./... pass; RefuseRadar as above.

Merging

Stacked on #20, which is on #19. Merge #19, #20, then this. After a release, RefuseRadar's tools/cdclint.sh can move the PR gate to --fail-on warning, as its own comment asks.

…what it left out

A repository that has read its inventory of uncaptured columns does not
want it on every run: RefuseRadar prints 161 source-column-not-captured
lines and set 2026-10-21 to decide whether they stay. --disable RULE
(comma-separated or repeated) drops a rule's findings from the output
and from the exit code.

A filter must not make a run look cleaner than it was, so the text
output ends with what was left out ("not shown (--disable):
source-column-not-captured 161"), a run whose every finding was hidden
says "ok: nothing to report outside the disabled rules" rather than
that the files agree, and --format json keeps its array shape and puts
the same note on stderr. An unknown rule name is an error listing the
rules, not a filter that silently hides nothing; engine.Rules is that
list, and the corpus test fails if any finding's rule is missing from
it.

Disabling schema-before-connector skips the base altogether, as if
--base were not given. Filtering its findings afterwards would also
lose the columns it raised, because source-column-not-captured leaves
raised columns out of its inventory, so they would appear nowhere.

The action gains a disable input. run() now takes its output streams,
so the tests drive the real command line: hiding a rule, a disabled
error no longer failing the run, commas and repeats, an unknown name,
the base skipped, and JSON's stderr note.
…TER's

In a multi-line ALTER TABLE every added column was positioned at the
statement's first line, so three columns added by one migration were
reported at the same line (RefuseRadar #963's three reports columns all
at :6). The readers parse actions with their whitespace folded, so an
action cannot be found in the file verbatim; each added column's name is
now searched for in the statement text, as a whole word, after the
table's name and after the previous action, and the column takes that
line. When the name cannot be found the statement's line stays, as
before.

This is also what lets a marker on the line that adds a column refer to
that column's finding.

Two corpus entries change, line numbers only: diff-connector-captures-
one-of-two (:5 to :7, movement_cleared_by's own line) and
diff-connector-touched-other-table (three findings at :6 become :7, :8,
:9, now in file order instead of message order). Every message is
unchanged.
…sion, so the finding stops failing the run

Leaving a PII column off the include list is right, and the diff rule
raises it all the same: RefuseRadar keeps its PR gate at --fail-on
error because at warning such a pull request could not merge, and so a
forgotten column does not block either. A per-rule filter cannot tell
the two apart. The decision has to be recorded against the column:

    ADD COLUMN ssn_hash TEXT, -- cdclint:ignore schema-before-connector: PII, never streamed

A marker names one or more rules and a reason. After code it covers its
own line; alone on a line it covers the line below. The trailing case
must not reach further: writing this entry's expectation by hand showed
that "this line or the next" made the ssn_hash marker also acknowledge
nickname on the next line, the forgotten column the gate exists for.

The reason is required, since it is the record. A marker with no
reason, naming a rule that does not exist, or covering no finding is an
ignore-marker warning. A schema-before-connector marker is never called
unused: that rule judges the change, so the marker goes quiet once its
pull request merges; neither is a marker for a rule that did not run.

Acknowledged findings do not count for the exit code and are listed
with their reason; --format json keeps its array and notes them on
stderr, as --disable does. The diff rule's fix now points at the marker
instead of "let this warning stand". The corpus test runs through the
same lint() path as the command line.

Proven on RefuseRadar with the probe pattern: a column added to reports,
--base origin/main --fail-on warning: exit 1 without a marker, exit 0
with one, the reason printed.
@avison9 avison9 self-assigned this Sep 26, 2026
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.

1 participant