Secret-leak checks in tests look for a sentinel that a timestamp can't contain - #4860
Merged
Merged
Conversation
…an't contain The deadlock drill-down tests planted 4721 and 9034 and then searched whole JSON responses for those digits. A wall-clock timestamp such as 11:20:24.4721267 holds them in its fractional seconds, so one run failed with nothing leaking. The planted literals are now qx4721 and qx9034 (letters outside a-f), which no timestamp, decimal id or hex hash can contain. Every assertion is kept.
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.
Why
PgTargetDeadlockDrillDownTests.APlantedCounterDeltaBesideFourCapturedRows_CarriesTheExemplarDrillDown_AndTheRefrozenAdvicefailed once on a run where nothing leaked. Its checkAssert.DoesNotContain("4721", analysis, ...)is meant to prove that the planted literalheld-4721was normalized away. Butanalysisis the whole analyze_server JSON, and that JSON also holds wall-clock timestamps. On the failing run the timestamp2026-09-30T11:20:24.4721267Zhas 4721 in its fractional seconds, so the bare substring matched a clock reading, not a secret.The same can happen to any bare short number checked against text that holds a timestamp, a generated id or a hash.
What changes
Test files only. No product code changes.
The sentinel
Every planted secret literal in
Darling/Darling.Tests/PgTargetDeadlockDrillDownTests.cskeeps its digits and gains letters outside a-f. A timestamp, an id or a hash holds only decimal digits or hex digits a-f, so none of them can contain the sentinel:'4721''qx4721''9034''qx9034''held-4721''held-qx4721'Replacethat builds the expected graph (line 422), the raw victim statement (line 461)"4721","9034"in the leak loop"qx4721","qx9034"DoesNotContain("4721", analysis)DoesNotContain("qx4721", analysis)DoesNotContain("4721", stored)DoesNotContain("qx4721", stored)A one-line comment where the pattern is first used (line 121) says why. The 16-digit card numbers and the hashes stay as they were: a timestamp or an id cannot hold them. Every assertion the tests had is kept. All the planted values sit inside string literals, which the normalizer replaces with
?whatever they hold, so the expected normalized text does not change.The sweep of other bare short-number negative checks
git grep -nE 'DoesNotContain\("[0-9]{3,6}"' -- 'Darling/Darling.Tests/*.cs' 'Lite.Tests/*.cs'found 26 lines besides the two above. For each, the question was whether the searched text can hold a run-varying number. Such numbers are a wall-clock time, a generated id, a hash of run-varying input and a duration measured at run time. None of the 26 can hold one, so none changed. The four loop forms the grep cannot see are classified after the table.2404270250432755360010076789411160000081923600300006000090000030000300005701425000041111024PgDisplay.Bytesof three fixed byte counts57014errorfield, which the line above already asserts equals the fixed timeout message5701475541119034100The loop forms:
4721,9034(and a name and two card numbers)101,20210157014,57P04Assert.False(IsRetryable(...)), a classification and not a text searchTest plan
Darling.TestsandLite.Testsbuild with0 Warning(s).PgTargetDeadlockDrillDownTests:Total: 9, Errors: 0, Failed: 0, Skipped: 1, Not Run: 0(the skipped one is the live test).TsqlConventionGuardTests:Total: 14, Errors: 0, Failed: 0.DocCommentHygieneTests:Total: 77, Errors: 0, Failed: 0.RED 1, the exemplar path keeps the raw literal.
NormalizeStoredDeadlockExemplarswas changed to leave the victim statement and the graph text as stored, then restored. The unit test fails at its exact-value assertion, which comes before the sentinel loop is reached:RED 2, a leak only the sentinel loop can see.
NormalizeStoredDeadlockExemplarswas changed to leave the prosenoteas stored (every exact-value assertion still holds), then restored. The changed leak loop catches it:Both plants were restored:
git diffshows the one test file.Full
Darling.Testssuite, once, with no live database:Darling.Tests Total: 18891, Errors: 0, Failed: 0, Skipped: 1190, Not Run: 1, Time: 94.116s. The skips are live tests that need PostgreSQL, and the one not run is an explicit test.The two live checks (lines 424 and 501) run only in CI against PostgreSQL. They passed in CI at 5b6da3a against PostgreSQL. They were not run here, so no RED was shown for them.
Lite.Testssuite: no Lite test changed, so only its build was run here. CI ran the Lite shards green.CHANGELOG
SECTION: None