Skip to content

Route exclusion-match messages through logging (quiet library, unchanged CLI)#593

Merged
kmcquade merged 3 commits into
masterfrom
feat/exclusion-message-logging
Jun 14, 2026
Merged

Route exclusion-match messages through logging (quiet library, unchanged CLI)#593
kmcquade merged 3 commits into
masterfrom
feat/exclusion-message-logging

Conversation

@kmcquade

Copy link
Copy Markdown
Collaborator

What

Makes the Excluded prefix: / Excluded suffix: messages from is_name_excluded quiet for library consumers while keeping CLI output byte-for-byte identical.

Why

is_name_excluded emitted prefix/suffix matches via utils.print_grey — a raw print() that bypasses logging entirely. These fire during collection construction (RoleDetailList / ManagedPolicyDetails) for every service-linked role and AWS-managed policy, so any library parse of get-account-authorization-details (e.g. constructing AuthorizationDetails / PolicyDocument) floods stdout with no way to silence it via standard logging. The exact-match branch already used logger.debug; only prefix/suffix were inconsistent.

How

  • exclusions.py: route the prefix/suffix branches through a _report_exclusion() helper — logger.debug by default (library, silenceable via standard logging), print_grey when a toggle is on.
  • bin/cli.py: the Click group callback enables the toggle for the duration of the invocation and restores the prior value via ctx.call_on_close() — so CLI output is unchanged and an in-process CLI run does not leak printing state into later library use.
  • A ContextVar (not a module global) holds the toggle — lint-clean (PLW0603) and naturally scoped/thread-safe.

Behavior

  • Library: constructing AuthorizationDetails with a service-linked role now prints nothing to stdout (messages go to logger.debug).
  • CLI: cloudsplaining scan on the example data still prints the grey Excluded prefix lines (verified: 20 lines, exit 0).
  • Note: the download CLI command never emitted these (it's pure boto3 and never builds scan objects); the leak came from the library parsing path, which is what's now quiet.

Tests (TDD)

test/shared/test_exclusion_output.py — library-quiet (prefix + suffix), the logger.debug path, CLI printing, the returned previous value, and an in-process CLI regression proving the toggle is restored after a run. test/conftest.py adds an autouse reset fixture (defense-in-depth).

Review trail

  • Opus plan fact-check → yes-with-fixes (folded in).
  • Codex adversarial review → flagged the sticky-global leak; fixed via the scoped ctx.call_on_close() restore + regression test.
  • Codex code review of the diff → no actionable findings.

The implementation plan is included at docs/plans/2026-06-14-exclusion-message-logging.md.

Gates

just pre-push green: lint, unit-tests (130 passed), type-check, test-js (47 passing), safety-scan.

🤖 Generated with Claude Code

kmcquade and others added 3 commits June 14, 2026 17:20
…ged CLI)

is_name_excluded() printed "Excluded prefix/suffix" lines via utils.print_grey
(a raw print()), bypassing logging entirely. These fire during collection
construction (RoleDetailList / ManagedPolicyDetails) for every service-linked
role and AWS-managed policy, so any library parse of GetAccountAuthorizationDetails
floods stdout with no way to silence it.

Route the prefix/suffix branches through a _report_exclusion() helper: logger.debug
by default (library — silenceable via standard logging) and print_grey when a
ContextVar toggle is on. The CLI group callback enables the toggle for the duration
of the invocation and restores the prior value via ctx.call_on_close(), so CLI
output is byte-for-byte unchanged and an in-process CLI run does not leak printing
state into later library use (the Codex adversarial-review finding).

Using a ContextVar instead of a module global keeps the override scoped/thread-safe
and avoids the discouraged `global` statement.

TDD: new test/shared/test_exclusion_output.py covers library-quiet (prefix+suffix),
the logger.debug path, CLI printing, the returned previous value, and an in-process
CLI regression proving the toggle is restored. test/conftest.py adds an autouse
reset fixture as defense-in-depth.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add docs/plans/exclusion-message-logging.md: make the "Excluded prefix/suffix"
lines from is_name_excluded quiet for library consumers (logger.debug) while
keeping CLI output byte-for-byte identical via a module-level toggle the CLI
group callback flips on. Fact-checked by an Opus review subagent (verdict:
yes-with-fixes); the three required fixes are folded in.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
v2 of the plan. The Codex adversarial review (verdict: needs-attention) flagged
the v1 sticky process-global toggle: an in-process caller that runs the CLI and
then uses the library would keep printing exclusion matches, violating the
quiet-by-default library contract.

Fix: scope set_exclusion_output(True) to the Click invocation and restore the
prior value via ctx.call_on_close(); set_exclusion_output now returns the
previous value. Add an embedded-CLI regression test (TDD item 5) that runs the
CLI in-process then asserts the library is quiet afterward.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@kmcquade kmcquade merged commit c0546f7 into master Jun 14, 2026
14 checks passed
@kmcquade kmcquade deleted the feat/exclusion-message-logging branch June 14, 2026 21:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant