The gap
The smoke tests added in #4047 connect as sa. Every check therefore has permission to read everything, and the Azure job skips the cross-database checks outright. So nothing in CI exercises the case the #SkipChecks mechanism exists for: an account that cannot read msdb or master metadata, asking for the checks that would fail to be skipped.
That is not hypothetical. A regression of exactly this kind shipped inside #4045 and survived every check we have.
What went wrong in #4045, and why nothing caught it
That PR hoisted cross-database sub-queries out of IF predicates so Azure could compile the module. On dev, the sub-query sat after the skip test in the same predicate:
IF NOT EXISTS (SELECT 1 FROM #SkipChecks WHERE DatabaseName IS NULL AND CheckID = 202)
AND EXISTS (SELECT * FROM msdb.INFORMATION_SCHEMA.COLUMNS c ...)
After hoisting, the msdb read ran first and unconditionally, and the skip test came afterwards. Asking to skip CheckID 202 no longer prevented the msdb access. The same applied to 178, 105, 116 and 191.
Every layer of verification we have missed it:
- Boxed smoke tests — passed. As
sa, a cross-database read never fails, so the check simply ran when it should not have.
- Azure smoke test — passed.
EngineEdition = 5 blocks those reads anyway, so the defect was invisible.
- A findings comparison of released vs rewritten sp_Blitz on 2017 and 2025 — reported the two identical. Also run as
sa, so both sides produced the same output.
- Three rounds of human and automated review before Copilot flagged it, and even then as a suppressed comment.
The defect class is quiet by nature. A check that reads something it should have skipped raises no error and changes no output when the caller happens to be privileged. It only surfaces as sp_Blitz aborting for the one user who explicitly asked for that check to be turned off — which is the user least able to diagnose it.
Proposed test
A step that runs sp_Blitz as a deliberately under-privileged login and asserts it completes:
- Seed creates a login with
VIEW SERVER STATE and no user in msdb.
- Populate a skip table with the CheckIDs whose probes touch
msdb / master — currently 202, 178, 105, 116, 191.
- Connect as that login, run
sp_Blitz with @SkipChecksDatabase / @SkipChecksSchema / @SkipChecksTable pointed at it.
- Assert it completes with no SQL error.
Under today's code that passes. Revert any one of the five skip guards and it should fail with a permissions error, which is the property worth locking in.
Worth being careful about
The reason this was not added inside #4045 is that a naive version is brittle. sp_Blitz does a lot as a non-sysadmin, and plenty of other checks will complain for reasons unrelated to this one. The test needs a skip list wide enough that unrelated permission noise does not fail the build, but narrow enough that removing a hoisted probe's guard still fails it. Getting that boundary right is the actual work here, and it deserves its own change rather than being appended to an unrelated one.
The harness also currently runs every step through one sa credential set, so it needs a way to run a step as a different login.
Context
Found by Copilot while reviewing #4045, which fixed the five affected guards. Filed so the coverage gap does not stay open now that the specific bug is closed.
The gap
The smoke tests added in #4047 connect as
sa. Every check therefore has permission to read everything, and the Azure job skips the cross-database checks outright. So nothing in CI exercises the case the#SkipChecksmechanism exists for: an account that cannot readmsdbormastermetadata, asking for the checks that would fail to be skipped.That is not hypothetical. A regression of exactly this kind shipped inside #4045 and survived every check we have.
What went wrong in #4045, and why nothing caught it
That PR hoisted cross-database sub-queries out of
IFpredicates so Azure could compile the module. Ondev, the sub-query sat after the skip test in the same predicate:After hoisting, the
msdbread ran first and unconditionally, and the skip test came afterwards. Asking to skip CheckID 202 no longer prevented themsdbaccess. The same applied to 178, 105, 116 and 191.Every layer of verification we have missed it:
sa, a cross-database read never fails, so the check simply ran when it should not have.EngineEdition = 5blocks those reads anyway, so the defect was invisible.sa, so both sides produced the same output.The defect class is quiet by nature. A check that reads something it should have skipped raises no error and changes no output when the caller happens to be privileged. It only surfaces as
sp_Blitzaborting for the one user who explicitly asked for that check to be turned off — which is the user least able to diagnose it.Proposed test
A step that runs
sp_Blitzas a deliberately under-privileged login and asserts it completes:VIEW SERVER STATEand no user inmsdb.msdb/master— currently 202, 178, 105, 116, 191.sp_Blitzwith@SkipChecksDatabase/@SkipChecksSchema/@SkipChecksTablepointed at it.Under today's code that passes. Revert any one of the five skip guards and it should fail with a permissions error, which is the property worth locking in.
Worth being careful about
The reason this was not added inside #4045 is that a naive version is brittle.
sp_Blitzdoes a lot as a non-sysadmin, and plenty of other checks will complain for reasons unrelated to this one. The test needs a skip list wide enough that unrelated permission noise does not fail the build, but narrow enough that removing a hoisted probe's guard still fails it. Getting that boundary right is the actual work here, and it deserves its own change rather than being appended to an unrelated one.The harness also currently runs every step through one
sacredential set, so it needs a way to run a step as a different login.Context
Found by Copilot while reviewing #4045, which fixed the five affected guards. Filed so the coverage gap does not stay open now that the specific bug is closed.