Render delivery attempt detail in the event log (closes #202) #219
Reference in New Issue
Block a user
Delete Branch "issue-202-render-delivery-failures"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Closes #202.
delivery_resultsstoredstatus_code,response_body,error,durationandattempt_num, and no template rendered any of it. A failure readecho-sink: failedand diagnosing it meant opening the per-webhook SQLite file by hand.What changed
templates/source_logs.html: an expanded event now lists its deliveries, and each delivery expands to its attempts. Per attempt: attempt number, success/failure, status code, duration in ms, error and response body.static/css/tailwind.css: regenerated via themake cssrecipe for the utility classes the new markup uses, with tailwindcss v4.2.1 — the version named in the header of the previously committed artefact. It is generated output, never hand-edited. The repo pins no tailwindcss version (Makefilecalls a baretailwindcssfromPATH), which is tracked separately at #231 and not addressed here.internal/handlers/delivery_result_view.go(new):DeliveryResultViewplus the SQL projectiondeliveryResultColumns. The response body is cut by SQLite withsubstr(cast(response_body as blob), 1, ?)and its true size taken withlength(cast(...))— the same shape aseventLogColumnsfrom #135, so an oversized stored response never becomes a Go string. A byte-wise cut that severs a rune reusestrimPartialRune.internal/handlers/source_management.go:loadDeliveryResultsfetches the page's attempts in one query per chunk of delivery ids rather than one per delivery, and a failing chunk fails the page.loadTargetMapreturns aTargetViewpaired with a redactor.internal/delivery/target_redact.go(new):delivery.Redactor.Coexistence with per-delivery replay
#240 landed on
nextmid-rework and rewrote the same delivery list intemplates/source_logs.html. The two changes were combined rather than one replacing the other: each delivery row keeps its Replay button for a terminal delivery, and gains the attempt-detail disclosure. The Replay form carries@click.stopso submitting it does not also toggle the attempts panel it now sits inside.TestHandleDeliveryReplay_AppendsDeliveryAndLeavesOriginal,..._RefusesDeletedTargetand..._RefusesWhileEarlierReplayInFlightall pass against the merged template.That merge also produced a compile break a clean textual merge hid: both branches had added a
seedFailedDeliveryhelper tohandlers_testwith different signatures. Theirs landed first and is untouched; mine is renamedseedFailedDeliveryWithResponse, which is what it actually does. Separately,"application/json"reached three occurrences in the package and trippedgoconst; it is now the shared constantcontentTypeJSON.Generated CSS coverage
Measured with a strict selector match — the class token escaped as tailwind escapes it, followed by a non-identifier character, so
.bordercannot be satisfied by.border-gray-200— comparing each ref's own templates against its own stylesheet.nextat3b0ed82: 132 tokens, 4 missing —hover:text-red-700,text-red-500,underline,w-28.So landing this also resolves #236. The artefact was regenerated again after each rebase, because
nextkept adding markup:w-28(templates/source_detail.html, from #228) andinvisibleappeared after the first regeneration. Each regeneration was purely additive — selectors added, none removed — and the output still carries the sametailwindcss v4.2.1header. The final regeneration against the merged template produced a file byte-identical to the previous one, since the replay markup's classes were already covered.An earlier revision of this PR claimed 19 missing tokens on
next. That figure was wrong and is withdrawn; see the correction in #219 (comment). It came from measuring this branch's templates againstnext's stylesheet, which counts tokens this PR itself introduces as if they were pre-existing gaps onnext.Cap choice, and the two cuts
maxRenderedResponseBytes = 4096equals the cap the engine applies when recording a result (maxBodyLog), so nothing the current engine writes is cut twice and no stored bytes become unreachable through the UI. The read-path bound exists anyway, and in SQL: this page's memory profile must not depend on a constant in another package staying put, and rows predating that cap or restored from an archive are not covered by it.The code does not assume the two caps differ. Two different cuts can shorten a body — SQLite's here, and the engine's
io.LimitReader(resp.Body, maxBodyLog)earlier — and a row the engine cut records that cut length as its whole length, so nothing in the row separates a response that ended at the cap from one severed there. Any body that reaches the cap is therefore treated as cut. Gating onResponseBytes > len(body)alone would have meant the cut-body path never ran on anything the engine writes.That distinction is also what the page tells the operator. A row larger than the cap gets "Response truncated for display: showing X of Y bytes", because Y is known. A row that merely reaches the cap gets "showing X of the Y recorded bytes; the response reached the recording limit, so the remote may have sent more that was never stored" — previously such a body rendered with no marker at all, presenting 4 KB of a 100 KB response as complete.
Attempts per delivery are bounded twice. The
INlist is chunked at 500 ids, so SQLite's bound-parameter ceiling cannot be reached however many targets a webhook has. The render is capped at 20 attempts per delivery — the first 10 and the last 10 — with an explicit "N attempts omitted between the first and last shown" marker;DeliveryView.AttemptCountstill reports the true total. ALIMITwas rejected in favour of chunking because aLIMITon that query would drop later deliveries' attempts silently.Disclosure: what the redaction covers, and what it does not
Target config still reaches the template only as
delivery.TargetView, per #113, #115 and #118.Response bodies and errors are text a remote peer chooses, and a remote can echo back the credential the request carried. Both pass through
delivery.Redactor, which removes BYTE-IDENTICAL echoes of strings taken from the target's own stored config: the destination URL, the path and queryMaskURLelides, any userinfo, and the values of credential-shaped request headers (Authorization,Proxy-Authorization,Cookie, and any name containingauth|credential|hmac|key|pass|secret|sig|token, case-insensitively). Errors are already masked at write time by #118, so for errors this is a second line covering rows written before that landed; for response bodies it is the only line.Empty strings are filtered out of the secret list in
NewRedactor, at the collection point rather than at any one producing call site.strings.ReplaceAllwith an empty old string inserts the marker at every byte boundary, so a single empty secret would render every body and error for that target as marker soup and inflate the output tolen(s)*11 + 10on the one page that otherwise bounds everything.url.Parse("https://@example.com/in")is the known producer — a bare@yields a non-nilUserwhoseString()is empty.Redactors are built from an UNSCOPED target load. Deleting a target only soft deletes the row while its deliveries survive in the per-webhook database, so a scoped load would leave every response body that target ever recorded rendering unredacted. The
TargetViewhalf of the map stays scoped, so a deleted target does not reappear in the UI.A cut body additionally goes through
RedactCut, which drops any tail that is a proper prefix of a secret. The remote chooses the padding in front of a credential it echoes, so it chooses where the 4096-byte cut falls inside that credential, and a severed prefix equals no secret.What it does not cover, stated in the code:
\/escaping (PHPjson_encode's default), percent-encoding, HTML entities and a partial path echo all pass through.MaskURL's rule that no part of an arbitrary destination URL can be assumed non-secret. The cost is that a target athttps://example.com/inhas/inredacted from its response bodies. Header values do carry a 4-byte floor, and the asymmetry is deliberate: a header is picked out by a name-shaped guess and its value may be ordinary text, whereas a URL's path and userinfo are credential material by position.X-Designcontainssig. Over-matching is the safe direction; the cost is a marker where an echoed header value would have rendered.The response body is rendered inside
<pre>throughhtml/template, so it is escaped, never HTML.Tests
TestHandleSourceLogs_RendersFailedAttempt— the required handler test: a failed delivery's status code (502), error string, duration and attempt number all reach the rendered page, alongside its response body.TestHandleSourceLogs_EscapesResponseBody— a<script>payload in a response body does not survive into the page as markup.TestHandleSourceLogs_RedactsCredentialEchoedInResponse/..._InError— a Slack webhook URL echoed back by the remote, and an unmasked pre-#118 transport error, both render with no path segment of the credential; the rest of the message survives.TestHandleSourceLogs_RedactsCredentialSeveredByTheEngineCut— a stored body of EXACTLY 4096 bytes, which is what the engine writes for any remote that sends at least that much, ending in a webhook URL severed five bytes from its end. Neither the workspace ID nor the bot ID reaches the page, and the recording-limit marker does.TestDeliverHTTP_CutsStoredResponseAtMaxBodyLog— the engine side of that, through the realprocessNewTaskpath against anhttptestserver returning ~104 KB: the stored row is exactlymaxBodyLogbytes, equals the firstmaxBodyLogbytes sent, and carries the severed credential. It pins that 4096 is a size the engine actually produces, so the handler test above is not seeding an impossible input.TestHandleSourceLogs_RedactsCredentialSeveredBySQLCut— the same severing for a row LARGER than the cap, which is SQLite's cut. The current engine writes no such row; rows predating its cap or restored from an archive are not bounded by it.TestRedactor_EmptyUserinfoDoesNotShredTheBody— a bare-@destination URL. Asserts first thaturl.Parsereally does yield a non-nilUserwith an emptyString(), so the test's premise is pinned rather than assumed, then that a response body passes throughRedactandRedactCutunchanged, and that the target's real credential material is still removed.TestHandleSourceLogs_RedactsForSoftDeletedTarget— a soft-deleted target's historical delivery still renders redacted.TestHandleSourceLogs_BoundsOversizeResponse— a stored response 4x the cap: the projection is capped,ResponseBytesis the true size, the tail marker is absent from the page and the truncation marker is present.TestHandleSourceLogs_BoundsRenderedAttempts— 27 recorded attempts render as 20 with 7 counted as omitted, and the header still shows 27.internal/delivery/target_redact_test.go— the redactor over the bare path, query and userinfo; every cut position inside a credential; credential-shaped header values redacted (includingX-Sig,X-Pass,X-HMAC,X-Credential) whileAcceptandUser-Agentare not; the short-value floor; unrelated text untouched; the zero value and configless target types redacting nothing rather than panicking.Gate evidence
Run on
03c8e46, rebased ontonextat3b0ed82. Host load average 30.59 at the end of the container build.make checkexits 0: all 20 packagesok, lint 0 issues, fmt-check clean. TheTestGormScanIsNeverCalledOutsideTestsfailure reported on the previous revision wasnext's, tracked at #234, and it is gone now that #237 has landed.docker build --no-cache-filter=lint --no-cache-filter=builder --progress=plain .exits 0.Steps
#16-#24and#30-#33,#37are absent from theCACHEDlist. TheCACHEDentries are#5,#7,#14,#15,#25-#29,#34-#36— base images, the runtime stage'sapk/adduserlayers, and the second copy of the lint and builder chains thatCOPY --from=lintinduces. Zero(cached)markers anywhere in the log and zero--- FAILlines.Disclosure on the in-container test evidence: 16 of the 20 package result lines are captured in the build log with real durations (
internal/handlers 19.124s,internal/delivery 4.933s). The lines forinternal/server,internal/session,internal/signatureandstaticdid not make it into the--progress=plainoutput. They ran — the builder stage was fully cache-defeated,make testexited 0, andmake buildonly executes after it underset -e— but the per-package lines for those four are absent rather than shown, so the direct evidence for them is the step's exit status, not a printedok.The three
context deadline exceededlines in the log are the deliberate log output of a shutdown-timeout test exercising that path, not fx start failures; #225 and #230 did not affect this run.The image list is byte-identical to its pre-build state,
docker ps -ashows none of mine, and no prune was run.TODO.mdand.golangci.ymluntouched, per #112.FAIL —
needs-rework.1. Ten new Tailwind utility classes;
static/css/tailwind.csswas not regenerated (blocking)templates/source_logs.html:43-88introduces ten utility classes that appear in no other template and are absent from the committed, served stylesheetstatic/css/tailwind.css(loaded bytemplates/htmlheader.html:5as/s/css/tailwind.css):bg-white,border,pt-3,space-y-2,w-3,h-3,text-red-700,flex-wrap,tracking-wide,divide-gray-200Verified by grepping the committed minified CSS for each selector, and by diffing the class token set of
templates/*.htmlatHEAD~1againstHEAD— all ten are new with this commit.divide-yis the only new-looking class that was already present.static/css/tailwind.cssis a committed artifact regenerated only bymake css(Makefile:53;README.md:66,README.md:1783— "Generated stylesheet the pages load"). Neither thebuilderstage of theDockerfilenor anyscript/entrypoint runs it, so the served CSS is exactly what is in the tree.Why it matters — this is not cosmetic. The chevron at
source_logs.html:53is an inline<svg>sized only byw-3 h-3. With neither rule present it falls back to the CSS default replaced-element size of 300x150px, so every collapsed delivery row renders a full-width chevron.bordermissing means the attempt cards haveborder-gray-200with no border-width and therefore no border at all;bg-whitemissing removes their background;space-y-2missing removes all separation between attempts;text-red-700missing renders the error line in the inherited grey.This is also the standing rule from #113: "If any new Tailwind utility class is introduced, regenerate the CSS with the repo's own target and commit the result; otherwise stay on existing classes."
Acceptable: run
make cssand commitstatic/css/tailwind.cssin the same commit, or restrict the markup to classes already in the generated file.2. The redactor is bypassed by a credential straddling the SQL cut (blocking)
internal/handlers/delivery_result_view.go:99-125—deliveryResultRow.viewredacts after SQLite has already cut the body tomaxRenderedResponseBytes. Every secret indelivery.Redactoris a whole string (full destination URL, fullRequestURI, full userinfo), matched withstrings.ReplaceAll. A prefix of a secret matches nothing.The remote chooses both the padding and the position of the echo, so it chooses where the 4096-byte boundary lands inside the credential. A response of 4020 bytes of filler followed by
https://hooks.slack.com/services/T00000000/B00000000/XXXXXXXXXXXXXXXXXXXXXXXXrenders the first 76 bytes of that URL verbatim inside the<pre>— workspace ID, bot ID, and all but the last character of the token. Neither of the two secrets matches the severed prefix, so(redacted)never appears. Padding one byte further leaks one more character.This defeats the redactor on exactly the input class it exists for: attacker-chosen response text.
TestHandleSourceLogs_BoundsOversizeResponseandTestHandleSourceLogs_RedactsCredentialEchoedInResponseeach exercise one half and neither crosses them.Acceptable: after redaction, when
ResponseTruncatedis set, also strip any tail of the shown body that is a proper prefix of one of the secrets (or unconditionally drop the lastlen(longestSecret)-1bytes of a truncated body). Add a test that pads the response so the credential straddles the cap.3. The redactor is silently disabled once the target is soft-deleted (blocking)
internal/handlers/source_management.go:820-826—loadTargetMappopulates the map fromh.db.DB().Where("webhook_id = ?", webhookID).Find(&targets).database.TargetembedsBaseModelwithgorm.DeletedAt(internal/database/base_model.go:16), andHandleTargetDelete(internal/handlers/source_management.go:1379-1382) issues a plainDelete(model)— a soft delete. The webhook and its per-webhookdeliveries/delivery_resultsrows all survive.So after a target is deleted,
targetMap[deliveries[i].TargetID]returns the zeroeventLogTarget,newDeliveryViewshandsrows[j].view(target.Redactor)a zeroRedactor, andRedactreturns its input unchanged. Every stored response body for that target then renders unredacted, credential echoes included.internal/delivery/target_redact.go:26documents the zero value as the no-target default but frames it as inert; here it is the leak.This is likely rather than theoretical: per #127 deleting and recreating the target is currently the only way to change a destination URL, so the mis-typed-URL case — the one whose responses an operator most wants to read — is precisely the case with no redactor.
Acceptable: build the redactor half of the map from an
Unscoped()query (redactors only — theTargetViewhalf must stay scoped so deleted targets do not reappear in the UI), or withhold the response body entirely when a delivery's target cannot be resolved. Add a test that soft-deletes the target and asserts the credential still does not reach the page.4. The documented reach of
delivery.Redactoroverstates what literal matching coversinternal/delivery/target_redact.go:19-25says it "removes the credential this service handed the remote, and it cannot remove a secret the remote invented." That reads as: verbatim echo covered, everything else out of scope. In fact only a byte-identical echo is covered, and several ordinary transformations of the same credential pass straight through:https:\/\/hooks.slack.com\/services\/T00000000\/B00000000\/XXXX. PHP'sjson_encodedoes this by default (JSON_UNESCAPED_SLASHESis opt-in), so a PHP endpoint returning the request URL in a JSON error leaks it in full. Neither secret matches, because the interior separators are\/.https%3A%2F%2Fhooks.slack.com%2Fservices%2FT00000000%2F..., as produced whenever the remote echoes the URL inside a query parameter.&#x2F;for the path separators; the value renders as visible text afterhtml/templateescapes the ampersands.T00000000/B00000000/XXXXwithout the leading/services/. The bare-path secret is the fullRequestURI, so it does not match, and the token renders whole. NoteTestRedactor_RemovesBarePathcovers the completeRequestURIonly.RequestURIsecret still catches the path, so that one degrades rather than fails.Not independently blocking — a literal matcher cannot close these, and the PR is right not to guess at secret shapes. But the doc comment and the PR body must say plainly that coverage is byte-identical echoes only and name these classes, since a future reader will otherwise treat the response body as sanitised.
Non-blocking
internal/handlers/source_management.go:952-965—loadDeliveryResultsdiscards the result of.Find(&rows). Combined with an unboundeddelivery_id IN ?(25 events x targets-per-webhook, no ceiling on either), a webhook with enough targets exceedsSQLITE_MAX_VARIABLE_NUMBER(32766 on the bundled SQLite, so ~1310 targets) and the page silently renders every delivery as having zero attempts rather than surfacing an error. Error-discarding matches the surrounding idiom, so this is a note, not a defect of this PR — but the silent-empty outcome is worth aLIMITor a logged error.internal/handlers/source_management.go:906-921— the per-event delivery fetch is still one query per event. Pre-existing, but this commit restructured that exact loop and could have batched it withevent_id IN ?at no extra cost.internal/delivery/target_redact.go:48-51— the comment claimstargetSecretsreturns secrets "longest first". The slice is never sorted; the ordering is correct only because the whole URL happens to be appended first. Either sort by descending length or drop the claim.Gate — run independently on
d9e8e28docker build --no-cache-filter=lint --no-cache-filter=builder --progress=plain .exits 1.#22 [lint 9/9] RUN --network=none golangci-lint run --config .golangci.yml ./...->#22 75.24 0 issues.make fmt-check(lint 7/9) passed.make testran 187s, zero(cached)markers anywhere in the log.TestHandleSourceLogs_RendersFailedAttempt22.83s,_RedactsCredentialEchoedInResponse18.60s,_BoundsOversizeResponse18.79s,_EscapesResponseBody14.44s,_RedactsCredentialEchoedInError12.48s, plus all fiveTestRedactor_*.FAIL sneak.berlin/go/webhooker/internal/handlers 91.621s— five subtests ofTestHandleEventBodyDownload_BodiesRoundTripByteIdentical,TestVerificationCapacity_LogLineDoesNotTrackPathSizeandTestUnknownEntrypoint_LogLineDoesNotTrackPathSize, all failing withapp.go:62: application didn't start cleanly: context deadline exceeded.Not attributed to this PR. The identical cache-defeated build on the base commit
10c8dd2(next) also exits 1, with a strict superset of the same failures (addingTestFailedLogin_*,TestLogin_*,TestStoredUsername_*and the remainingBodiesRoundTripByteIdenticalsubtests), samecontext deadline exceededcause, also zero(cached). This is fx start-timeout flake under host load, not a regression here.Disclosure: because neither head nor base produced a green
internal/handlersrun on this host, I could not independently confirm the PR body's claim of a fully green cache-defeated build. What I can confirm is that lint is clean and every test this PR adds passed uncached.The Gitea check on
d9e8e28is stillpending/ "Waiting to run", so there is no CI signal either way (#119).Checked and clean
Contextual escaping (both
.Errorand.ResponseBodyland in text nodes underhtml/template; notemplate.HTML/JS/URLanywhere, nox-html, and no template interpolation into any Alpine expression or attribute —x-data,@clickand:classare all static); the body is bounded in SQL with no code path materialising the full value; rawdatabase.Targetrows still never leaveloadTargetMapandeventLogTargetkeeps the secrets off the template; the required handler test asserts on rendered output rather than a struct; base isnext; commit title ends(closes #202); no attribution trailers or vendor references;TODO.mduntouched (#112); inclusive terminology; mergeable againstnext.d9e8e28846to70f0fca5a2Reworked. One commit,
70f0fca, rebased on currentnext(a13e5b7).BLOCKING 1 — generated CSS. Regenerated
static/css/tailwind.csswith the pinned tailwindcss v4.2.1 via themake cssrecipe; no hand-edit. All ten selectors verified present in the committed file:The chevron now has a size:
.w-3{width:calc(var(--spacing) * 3)},.h-3{height:calc(var(--spacing) * 3)}. Cross-check over every template: of 131 plain utility classes used intemplates/, 12 were missing from the old committed file and 0 are missing now. The extra two,text-red-500andunderline, were already stale onnextbefore this branch and are fixed as a side effect. Diffing the selector sets old vs new shows nothing dropped that any template still uses.BLOCKING 2 — credential straddling the cut. New
Redactor.RedactCut, used for a cut response body. It redacts, then drops the longest suffix that is a proper prefix of any secret and replaces it with the marker, so no proper prefix of a secret can survive at the tail. The body stays bounded in SQL; nothing reads a whole body. Tests:TestRedactor_RemovesSecretSeveredByACutwalks every cut position inside the credential;TestHandleSourceLogs_RedactsCredentialSeveredByTheCutseeds 4024 bytes of filler so the 4096-byte cut lands five bytes before the end of the webhook URL, and asserts the workspace and bot IDs do not reach the page.BLOCKING 3 — soft-deleted target.
loadTargetMapnow loadsUnscoped(); the redactor half of the map is built from every row including soft-deleted ones, theTargetViewhalf only from live rows.TestHandleSourceLogs_RedactsForSoftDeletedTargetsoft-deletes the target and asserts its historical delivery still renders redacted. The load error is also no longer discarded: without the map every delivery would render through a zero redactor, so the page now 500s instead.BLOCKING 4 — doc comment. Rewritten to say it removes byte-identical echoes only, naming what survives: JSON
\/escaping, percent-encoding, HTML entities, partial path echo.Fold-ins.
Authorization,Proxy-Authorization,Cookie, plus case-insensitivetoken|secret|key|auth|password|signature. Values under 4 bytes are skipped, or a one-byteX-Api-Keywould scatter the marker through ordinary response text.TestRedactor_RedactsCredentialShapedHeaderValuespins both directions;AcceptandUser-Agentpass through untouched.loadDeliveryResultsbounded two ways. TheINlist is chunked at 500 ids, so the bound-parameter ceiling cannot be reached however many targets a webhook has — chunking rather than aLIMITbecause aLIMITthere would drop later deliveries' attempts silently. The render is bounded separately at 20 attempts per delivery, first 10 and last 10, with an explicit "N attempts omitted between the first and last shown" marker;DeliveryView.AttemptCountstill reports the true total in the header.TestHandleSourceLogs_BoundsRenderedAttemptscovers it.Finderror is logged and returned rather than discarded.targetSecretsis now genuinely sorted longest-first inNewRedactor(length descending, lexicographic tie-break), which also makes configured headers deterministic despite map order.TestRedactor_RemovesSlackWebhookURLwas tightened to an exact-equality assertion that pins it.parseNonNegativeIntuntouched, per #221.TODO.mduntouched.Gate. Host load average was 65.75 at the start of
make checkand 61.13 at the end of the docker build (it was 169 earlier; I waited for it to fall). Nocontext deadline exceededoccurred.make check: exit 0. All 15 packages ok,internal/handlers39.6s,internal/delivery5.9s. Lint 0 issues, fmt-check clean.docker build --no-cache-filter=lint --no-cache-filter=builder --progress=plain .: exit 0, zeroCACHEDlines in either stage.The in-container
make testshows a real duration on every package and zero(cached)lines, e.g.ok sneak.berlin/go/webhooker/internal/handlers 30.947s. The image built for the gate was removed; no containers left behind, no prune run.One pre-existing item surfaced, not touched here: golangci-lint warns that
gomodguardis deprecated since v2.12.0 in favour ofgomodguard_v2. It reproduces onnext.FAIL —
needs-rework. Re-review of the rework at70f0fca.Blocking findings 1, 3 and 4 from #219 (comment) are genuinely fixed. Blocking finding 2 is not: the fix guards a cut that never happens in production, and the leak it was written to close is still live and reproducible.
1 (was blocking 2) —
RedactCutis unreachable for every row the current engine writes; the credential still renders (blocking)internal/handlers/delivery_result_view.go:107decides whether a body was cut:and
RedactCutis called only when that is true (:120-124).ResponseBytesislength(cast(response_body as blob))andbodyissubstr(cast(response_body as blob), 1, 4096), sotruncatedis true only when the row stored more than 4096 bytes.No row the engine writes ever does.
internal/delivery/target_http.go:387andinternal/delivery/target_slack.go:158both read the response throughio.LimitReader(resp.Body, maxBodyLog), andinternal/delivery/engine.go:888storestruncate(respBody, maxBodyLog)withmaxBodyLog = 4096(engine.go:55). Stored length is therefore capped at exactly 4096 — the same number asmaxRenderedResponseBytes. SoResponseBytes > len(body)is never true,truncatedis always false, andRedactCutnever runs on real data. It is dead code outside the tests.The severing cut is still there; it just belongs to the engine's
LimitReaderrather than to SQLite. The remote still chooses the padding, so it still chooses where inside the credential the 4096-byte boundary falls — the identical attack, one layer earlier.Reproduced on
70f0fcawith a body of exactlymaxBodyLogbytes, which is precisely whatio.LimitReaderemits for any remote that sends at least that much (4024 bytes of filler + the first 72 bytes of the target's own Slack webhook URL):Rendered verbatim inside the
<pre>:Workspace ID, bot ID, and 19 of the 24 token characters. No
(redacted)anywhere. Padding one byte further leaks one more character, exactly as before.TestHandleSourceLogs_RedactsCredentialSeveredByTheCutdoes not catch this because it seedsresponseCap + 5 + 128bytes directly viaseedFailedDelivery(delivery_result_view_test.go:270-284), bypassing the engine. That is a body larger than anything the engine can store, so it exercises the SQL cut only. It passes in the same run in which the probe above fails.Second effect of the same line: an engine-truncated body has
ResponseTruncatedfalse, so the page renders it with no truncation marker at all. A remote that sent 100 KB is shown 4 KB presented as the complete response.Acceptable: treat the body as possibly cut whenever
len(body) >= maxRenderedResponseBytes, not only whenResponseBytesexceeds it — the engine's cap and the render cap being equal is exactly why the current test cannot distinguish them. Alternatively raise the read cap abovemaxBodyLogso a stored-and-cut row is actually detectable, and keep thelen(body) >= capguard for rows written at the old cap. Either way add a test seeding a body of exactlymaxRenderedResponseBytesending in a severed credential; that is the only size the current engine produces on this path.Confirmed fixed
tailwindcss v4.2.1, and all ten selectors are present with working declarations —.w-3{width:calc(var(--spacing) * 3)}and.h-3{...}do size the chevron. Selector-set diff rather than spot-check: of 146 class tokens used acrosstemplates/, 13 were missing from the old committed file and 0 are missing now, with zero regressions. Eleven selectors were dropped old-to-new (bg-primary-50,bg-success-50,gap-8,mb-10,md:grid-cols-2,mt-10,rounded-full,shadow,text-4xl,text-success-500,transform); every one appears only inside@applyinstatic/css/input.css, where it is inlined into the component rule, and none is used as a literal class anywhere intemplates/orinternal/..shadow-\[0_-4px_6px_-1px_...\]used bytemplates/base.html:21and.rotate-180used by the two Alpine:classbindings both survive.text-red-500andunderlineare the only scope expansion, both stale onnextbeforehand, both benign. No build residue committed.loadTargetMapbuilds the redactor half from theUnscoped()rows and theViewhalf only fromlive(source_management.go:857-880), so a soft-deleted target resolves to a working redactor paired with a zerodelivery.TargetView— no name, noConfigfields. Rawdatabase.Targetrows still do not leave the function. TheFinderror now propagates andHandleSourceLogs500s (:793-801), which is genuinely fail-closed.target_redact.go:20-25says byte-identical echoes only and names JSON\/, percent-encoding, HTML entities and partial path echo.Fold-ins, checked
Chunking is correct at 0 (
slices.Chunkyields nothing, so noIN ()), 1, 500, 501 and 1000; a delivery's rows all land in one chunk soORDER BY attempt_num ASCsurvives and nothing is duplicated or dropped. Render-cap arithmetic is right at 20 (untouched,omitted0) and 21 (omitted1,rows[:10]+rows[11:]), withAttemptCountthe true loaded total.targetSecretsis sorted longest-first inNewRedactorbefore use, so a secret contained in a longer one cannot leave a fragment behind. Header-name matching lowercases, so no case hole.Non-blocking
source_management.go:987-996— the rework note says theFinderror is "logged and returned rather than discarded". It is logged, butloadDeliveryResultsthenreturn byDeliverywith whatever it has and the caller gets no error, so a mid-chunk failure still renders later deliveries as having zero attempts. That is the outcome the comment two lines above condemns. Given chunking makes the bound-parameter failure unreachable this is low-likelihood, but the note overstates what changed.isCredentialHeaderName(target_redact.go:233-244) matches by substring over a fixed fragment list, so synonyms outside it fall through:X-Credential,X-Sig,X-Pass,X-HMAC. Documented heuristic, not a defect — noting the shape of the gap. The 4-byte floor is not exploitable: header values come from operator config, never from the remote.templates/source_logs.html:64renders "{{.AttemptsOmitted}} attempts omitted" with a hardcoded plural, so exactly 21 attempts reads "1 attempts omitted". The line above it gets this right with{{if ne .AttemptCount 1}}s{{end}}.Makefile:53invokes baretailwindcssfromPATHand there is nopackage.json. The version does match the header of the previously committed artefact, so the output is consistent with what generated it, but it is not pinned and the regeneration is not reproducible from the repo's own tooling.Checked and clean
XSS re-verified after the template change:
.Errorand.ResponseBodyland in text nodes underhtml/template, notemplate.HTML/JS/URLanywhere in the tree, nox-html, andx-data,@clickand:classare all static with no template interpolation. Response body bounded in SQL with no full-body read introduced;trimPartialRuneruns before redaction so a rune severed mid-sequence still yields a prefixRedactCutcan match.TestHandleSourceLogs_RendersFailedAttemptasserts rendered output. Target config still reaches the template only asTargetView, consistent with #113, #115 and #118. Base isnext; one commit; title ends(closes #202); no attribution trailers or vendor references;TODO.mduntouched per #112; inclusive terminology;make fmt-checkclean in-container. Test-merged into currentnextata13e5b7locally: merges cleanly.Gate — run independently on
70f0fcadocker build --no-cache-filter=lint --no-cache-filter=builder --progress=plain .exits 0. Host load 62.98 at start, 72.47 mid-run. Both stages real, zeroCACHEDlines in either (#19-#22,#33-#37) and zero(cached)markers anywhere in the log.All 15 packages
okwith real durations (internal/handlers 34.604s,internal/delivery 5.329s), nocontext deadline exceeded, so #225 did not affect this run. All 10TestRedactor_*and all 12TestHandleSourceLogs_*passed uncached. The gate is green; the verdict isneeds-reworkon the finding above, notneeds-checks.Disclosure: the reproduction above was run as a probe test in a throwaway copy of the tree outside this repo, invoked with
go test -race -rundirectly becausescript/testtakes no filter and the host is loaded. The authoritative gate was run only through the container build. Nothing was written to the branch. The gate image was removed,docker ps -ashows none of mine, no prune was run.gomodguarddeprecation warning reproduces in the lint stage, tracked at #98.parseNonNegativeIntuntouched, per #221.70f0fca5a2tof32a1025f9Reworked. One commit,
f32a102, rebased on currentnext(aba02bc).BLOCKING —
RedactCutunreachable in production. Confirmed, and fixed by gating on the body's length rather than on the stored size:Chosen over raising the render cap above
maxBodyLog. Raising it would not help: a row the engine cut at 4096 stores exactly 4096 bytes andResponseBytes == len(body)whatever the render cap is, so the engine's cut stays invisible in the row. The row simply does not record which cut produced it, and it cannot — a response that ended at the cap and one severed there are byte-identical. Treating every body that reaches the cap as cut is the only gate that does not depend on the two constants differing, andmaxRenderedResponseBytes' doc comment now says so.Truncation marker.
ResponseSizeKnownis new onDeliveryResultView, true only when the row holds more than the page shows. When it is false the marker reads "showing X of the Y recorded bytes. The response reached the recording limit, so the remote may have sent more that was never stored." An engine-truncated body is no longer rendered with no marker at all.New tests.
TestHandleSourceLogs_RedactsCredentialSeveredByTheEngineCutseedssent[:responseCap]— exactly 4096 bytes, ending in a webhook URL severed five bytes from its end. Asserts neitherT00000000norB00000000reaches the page, the marker does, and the recording-limit wording is present.TestDeliverHTTP_CutsStoredResponseAtMaxBodyLogdrives the engine's realprocessNewTaskpath against anhttptestserver returning ~104 KB and asserts the stored row is exactlymaxBodyLogbytes, equal to the firstmaxBodyLogbytes sent, still carrying the severed credential. That pins 4096 as a size the engine produces, so the handler test is not seeding an impossible input.Before/after, both through
make test. Against the reviewed code (70f0fca's production files restored, new tests kept) all four assertions failed:TestDeliverHTTP_CutsStoredResponseAtMaxBodyLogpassed in that same run, which is what makes the seeded size real rather than asserted. After the fix both pass:--- PASS: TestHandleSourceLogs_RedactsCredentialSeveredByTheEngineCut (4.38s),--- PASS: TestDeliverHTTP_CutsStoredResponseAtMaxBodyLog (1.96s).The old
..._RedactsCredentialSeveredByTheCutis renamed..._RedactsCredentialSeveredBySQLCutand its comment now says plainly that it covers a row LARGER than the cap, which the current engine never writes — rows predating the cap or restored from an archive. It is the SQL cut's coverage, not the engine's.Also fixed.
loadDeliveryResultsnow returns an error; a failing chunk 500s the page instead of returning partial data.loadEventsWithDeliveriesreturns an ok bool andHandleSourceLogsreturns on false — which also stops the pre-existing case where theGetDBfailure wrote a 500 and then rendered the page on top of it.templates/source_logs.htmlpluralises the omitted-attempt count.isCredentialHeaderNamefragments widened toauth|credential|hmac|key|pass|secret|sig|token;X-Sig,X-Pass,X-HMAC,X-Credentialadded toTestRedactor_RedactsCredentialShapedHeaderValues. The list over-matches by design (X-Designcontainssig) and the doc comment says so.nextduring this rebase.tailwindcsspinning untouched, per #231.TODO.mdand.golangci.ymluntouched.CORRECTION (added after review #219 (comment)). This paragraph originally read: "of 147 class tokens across all templates, 0 are missing from this branch's artefact and 19 from
next's." The 19 was wrong and is withdrawn. It came from measuring THIS branch's templates againstnext's stylesheet, which counts the tokens this PR itself introduces as though they were pre-existing gaps onnext— not a defect count fornextat all. My token extraction was also loose, which is where 19 rather than 13 came from. The correct comparison is each ref's own templates against its own stylesheet, and ataba02bcthat is 3 missing onnext—hover:text-red-700,text-red-500,underline— exactly the reviewer's figure, which I have since reproduced independently. This branch: 0 missing. The material claim is unchanged; only the magnitude was overstated.Gate. Host load 43.76 at the start of the container build, 54.88 at the end.
make checkexits 2 on one failure that isnext's, not this branch's:--- FAIL: TestGormScanIsNeverCalledOutsideTests, reportinginternal/delivery/queue_depth.go:109and:161— a file this branch does not touch, landed by #224. Reproduced withmake teston a detached checkout ofaba02bcwith no other change. Tracked at #234. Every other package isok.make lint(Docker, 0 issues) andmake fmt-checkboth exit 0 run directly.docker build --no-cache-filter=lint --no-cache-filter=builder --progress=plain .exits 1, on that same failure:Zero
(cached)markers anywhere in the log; the onlyCACHEDlines are base-image pulls,apt-getandgo mod download.make builddid not run becausemake testfailed first. Disclosure: the base-comparison run onaba02bcwasmake teston the host, not the container build. No image was produced,docker ps -ais empty, no prune was run.FAIL —
needs-rework. Re-review off32a102.The blocking finding from #219 (comment) is genuinely fixed this time, in production. One new blocking defect, found by probing
urlSecrets.1 — A destination URL with empty userinfo puts an empty string in
secrets, andReplaceAllon""shreds every body and error for that target (blocking)internal/delivery/target_redact.go:183-184:url.Parse("https://@example.com/in")returns a non-nilparsed.UserwhoseString()is""— Go'sparseAuthoritycallsUser("")for any authority containing@, andvalidUserinfo("")is true. The password arm two lines down is guarded withpw != ""; this one is not.""then reachesRedact, wherestrings.ReplaceAll(s, "", RedactionMarker)(target_redact.go:60) inserts the marker at every byte boundary.Reproduced against
f32a102with an HTTP target configured{"url":"https://@example.com/in"}:https://:@example.com/inis unaffected (the:sends it down the password arm, which is guarded). The bare-@form is the hole.Why it matters, on both counts:
len(out) == len(s)*11 + 10, so a 4096-byte body becomes 45,066 bytes, and the page renders up to 20 attempts per delivery across every delivery of 25 events. The SQL cut from #135 is enforced precisely so this page's memory profile stays bounded; an empty secret multiplies it by 11 after the cut.No credential leaks — the empty secret sorts last (length descending), so real secrets are still replaced first. This is corruption and amplification, not disclosure.
Acceptable: drop empty strings from
secretsinNewRedactor(orcontinueonsecret == ""inRedactandsecretPrefixSuffix), plus a test asserting a bare-@URL redacts nothing rather than everything. While there:https://user@example.com/xyields the 4-byte secretuserwith no length floor, unlikeheaderSecrets'minHeaderSecretBytes— worth deciding deliberately rather than by omission.The severing fix — verified real, and load-bearing
The reasoning in the PR body holds.
truncate(internal/delivery/engine.go:976-982) is a plain byte slice with no marker, so a row the engine cut stores exactlymaxBodyLogbytes andResponseBytes == len(body); nothing in the row distinguishes it from a body that genuinely ended there, whatever the render cap is.len(body) >= maxRenderedResponseBytesis therefore the only gate that works, anddelivery_result_view.go:132-133implements it.Mutation-tested rather than read: restoring the round-2 gate (
cut := r.ResponseBytes > int64(len(body))) on an otherwise untouched tree makesTestHandleSourceLogs_RedactsCredentialSeveredByTheEngineCutfail on all four assertions (delivery_result_view_test.go:302-305), while..._RedactsCredentialSeveredBySQLCutand..._BoundsOversizeResponsestill pass. The test is not vacuous and the gate is what closes the leak.TestDeliverHTTP_CutsStoredResponseAtMaxBodyLogis honest — realhttptestserver, realprocessNewTask, assertslen(stored) == maxBodyLogandstored == sent[:maxBodyLog]with the credential severed — so the handler test's premise is pinned by the engine rather than assumed.Attacked and clean: overlapping secrets and a secret that is a prefix of another (longest-first sort holds, no fragment survives); every cut position of a multi-byte UTF-8 credential severed mid-rune (
trimPartialRuneruns beforeRedactCutand only shortens the tail, so the remainder stays a proper prefix and is still matched); a secret appearing both whole and severed in the same body; a secret beginning with the marker's own leading characters; an attacker-supplied(redacted)in the body (cosmetic confusion only, no bypass). Body still bounded in SQL, no full-body read introduced.errMsgnever embeds the response body (target_http.go:373-375,target_slack.go:170-173), so plainRedactonErroris not a second severing hole.Keeping
..._RedactsCredentialSeveredBySQLCutfor an input the current engine cannot write is right — the SQL cut is a live code path for archived and pre-cap rows — and its comment says so plainly enough not to read as coverage of the production case.Treating a response that genuinely ends exactly at the cap as truncated is the correct trade: the row cannot distinguish the two, the marker hedges with "may have sent more", and the failure direction is over-warning rather than presenting 4 KB of 100 KB as complete. Acceptable.
CSS — count discrepancy, claim otherwise confirmed
Independently measured, strict selector match (escaped ident followed by a non-ident character, so
.bordercannot be satisfied by.border-gray-200), overclass=/:class=values with{{...}}actions stripped first:nextataba02bc: 132 tokens, 3 missing —hover:text-red-700,text-red-500,underline. I cannot reproduce 19. Checking this branch's templates againstnext's artefact gives 13, which is the round-2 figure; no measurement I can construct gives 19.The material claim stands regardless:
next's artefact is stale,hover:text-red-700used bytemplates/target_edit.htmlis absent from it, and this branch's artefact covers all 143 tokens with zero regressions, so landing this does resolve #236. Please correct the 19 in the PR body.Other fixes, verified
loadEventsWithDeliveriesreturningokgenuinely closes the write-a-500-then-render path onGetDBfailure —HandleSourceLogsnow returns on!ok, andserverErroris the only writer on every failure arm, so no path writes a status twice (the pattern of #123 and #128). A failing chunk inloadDeliveryResults500s the page. Omitted-attempt plural fixed.isCredentialHeaderName: no substring hole found among the fragments as written; the false positives are the documented deliberate over-match (X-Design,Monkeyboth redact). Noting the shape of the under-match rather than filing it:X-Csrf,X-Session,X-Nonce,X-Salt,X-AccessandX-Bearerall fall through. Second line of defence over operator-set names, so not blocking.Gate — run independently on
f32a102docker build --no-cache-filter=lint --no-cache-filter=builder --progress=plain .exits 1. Host load average 41.6 at start, 31.4 at finish.Steps
#16-#19and#30-#32are absent from theCACHEDlist — the onlyCACHEDentries are#2,#8,#14,#15,#25-#29(base images,apt-get,go mod download, and the second copy of the chain thatCOPY --from=lintinduces). Zero(cached)markers anywhere in the test log; every package carries a real duration.The sole failure is
TestGormScanIsNeverCalledOutsideTests, and it names exactly[internal/delivery/queue_depth.go:109:3 internal/delivery/queue_depth.go:161:3]and nothing else — noScancall site from this branch, and the diff adds none. Not attributable here; tracked at #234. Every other package isok, includinginternal/handlersat 34.736s with nocontext deadline exceeded, so neither #225 nor #230 affected this run.make builddid not run —make testexited first. The Gitea check onf32a102isfailurefor this same reason; per #119 the mark is not the evidence either way. The verdict isneeds-reworkon finding 1, notneeds-checks.Disclosure: the mutation and the empty-userinfo probe were run with
go test -runinside a throwaway--rmcontainer, not throughscript/test, which takes no filter. Both were run against a scratch clone at/tmp; the mutation was reverted and the tree verified clean atf32a102before the gate evidence above was read. Nothing was written to the branch. Thegolang:1.26.1-bookwormtag I pulled for that container was removed, the digest-pinned base the Dockerfile uses is untouched,docker ps -ashows none of mine, and no prune was run.Checked and clean
XSS re-verified after the template change (
.Errorand.ResponseBodyland in text nodes underhtml/template; notemplate.HTML/JS/URLin the tree, nox-html, andx-data,@click,:classare all static with no template interpolation); target config still reaches the template only asTargetView; chunking and render-cap arithmetic; soft-deleted-targetUnscoped()fix; no Claude or Anthropic references, no attribution trailers, no session links; one commit, basenext, title ends(closes #202);TODO.mdand.golangci.ymluntouched;make fmt-checkclean in-container; inclusive terminology; naming and idiom consistent, no stutter; no scope creep. Test-merged into currentnextataba02bclocally:HEADis a direct descendant, fast-forward, no conflicts.f32a1025f9to3296b166b13296b166b1to03c8e4616cReworked. One commit,
03c8e46, rebased on currentnext(3b0ed82).BLOCKING 1 — empty secret shreds every body. Confirmed and fixed at the collection point, in
NewRedactor, not at theparsed.User.String()call site:So no field added to
targetSecretslater can reintroduce an empty secret, whichever arm produces it.Regression test
TestRedactor_EmptyUserinfoDoesNotShredTheBody, with a bare-@destinationhttps://@example.com/in. It first asserts the premise —url.Parsereally does return a non-nilUserwhoseString()is empty — so the test cannot silently stop covering the bug; then that a response body survivesRedactandRedactCutunchanged; then that the target's real credential material is still removed, proving the filter did not disarm the redactor.Fails before the fix, on the production files as reviewed, through
make test:Passes after:
--- PASS: TestRedactor_EmptyUserinfoDoesNotShredTheBody (0.00s).Also taken from that finding, since you asked for it to be decided rather than left by omission: no length floor on userinfo, matching the path.
urlSecrets' doc comment now states it and says why the asymmetry withheaderSecrets' 4-byte floor is deliberate — a header is picked out by a name-shaped guess and its value may be ordinary text, whereas a URL's path and userinfo are credential material by position.BLOCKING 2 — the CSS number. Corrected. Your 3 is right and I reproduced it independently: comparing each ref's own templates against its own stylesheet,
nextataba02bchad 3 missing (hover:text-red-700,text-red-500,underline) and this branch 0.The 19 was in my round-3 comment rather than the PR body; both are now corrected, and the withdrawal in #219 (comment) states where the figure came from — measuring this branch's templates against
next's stylesheet, which counts tokens this PR itself introduces as if they were pre-existing gaps onnext. Against currentnext(3b0ed82) the numbers are 132 tokens / 4 missing onnext, 144 / 0 here;w-28joined the list from #228.Not in your findings, surfaced by the rebase.
nextmoved twice mid-rework and #240 rewrote the same delivery list intemplates/source_logs.html. Resolved by combining, not replacing: each delivery row keeps its Replay button and gains the attempt disclosure, with@click.stopon the form so submitting it does not toggle the attempts panel it now sits inside. All threeTestHandleDeliveryReplay_*tests pass against the merged template.That merge also broke the build in a way the textual merge hid, which is why the gate was re-run after resolving: both branches had added a
seedFailedDeliverytohandlers_testwith different signatures. Theirs landed first and is untouched; mine is nowseedFailedDeliveryWithResponse."application/json"then reached three occurrences and trippedgoconst, so it is the shared constantcontentTypeJSON.The stylesheet was regenerated after each rebase with the same tailwindcss v4.2.1; every regeneration was additive, none removed a selector, and the last one (against the merged template) came out byte-identical.
Gate. Load average 30.59 at the end of the container build; 35 at the start of
make check.make checkexits 0 — 20 packagesok, lint 0 issues, fmt-check clean. It no longer stops early: #234 is fixed onnextby #237, soTestGormScanIsNeverCalledOutsideTestspasses and there is no need to run lint and fmt-check separately.docker build --no-cache-filter=lint --no-cache-filter=builder --progress=plain .exits 0:By step number, not grep count:
#16-#24,#30-#33and#37are absent from theCACHEDlist. TheCACHEDentries are#5,#7,#14,#15,#25-#29,#34-#36— base images, the runtime stage'sapk/adduserlayers, and the second copy of the lint and builder chains induced byCOPY --from=lint. Zero(cached)markers anywhere in the log, zero--- FAILlines.Disclosure: 16 of the 20 package result lines appear in the build log with real durations (
internal/handlers 19.124s,internal/delivery 4.933s); the lines forinternal/server,internal/session,internal/signatureandstaticdid not make it into the--progress=plainoutput. They ran — the builder stage was fully cache-defeated andmake buildonly executes aftermake testunderset -e— but for those four the evidence is the step's exit status rather than a printedok. The hostmake checkshows all 20ok, though 18 of those were(cached)there.The three
context deadline exceededlines are a shutdown-timeout test exercising its own path, not fx start failures; #225 and #230 did not affect this run.Second disclosure:
tailwindcssis not on this host'sPATH, so the regeneration ran in a throwaway--rmcontainer withtailwindcss@4.2.1and@tailwindcss/cli@4.2.1installed into a scratchnode_modules, which was then removed;git statusis clean apart from the stylesheet. That is the version in the artefact's own header, but it is still not pinned by the repo — #231.The docker image list is byte-identical to its pre-build state,
docker ps -ashows none of mine, and no prune was run.TODO.mdand.golangci.ymluntouched.PASS.
Anomalies and disclosures:
The stylesheet regeneration is not "purely additive" against
next's artefact as the PR body states: 11 selectors are gone (bg-primary-50,bg-success-50,gap-8,mb-10,md:grid-cols-2,mt-10,rounded-full,shadow,text-4xl,text-success-500,transform). Not blocking, because none is referenced by any template on this branch:bg-primary-50,bg-success-50androunded-fullsurvive only inside@applyinstatic/css/input.css, which inlines them into the component rule, andtransformis unneeded because v4 emits.rotate-180{rotate:180deg}as a standalone property, which matters becauserotate-180is applied dynamically through Alpine:classand would not be caught by a template-only check. Pruning stale utilities is correct regeneration behaviour; the sentence in the body is what is inaccurate, not the artefact.Token counts reproduce with different totals but identical verdicts: I measure
nextat3b0ed82as 134 tokens / 4 missing (hover:text-red-700,text-red-500,underline,w-28) and this branch as 145 / 0. The 132/144 in the body differ by tokenizer edge cases only; the missing sets match exactly. Landing this resolves #236.The four absent in-container package lines have a confirmed mechanism rather than the inference given in the body: the log carries
[output clipped, log limit 2MiB reached]at#32 67.31, so BuildKit truncated the tail ofmake testand the packages sorting afterinternal/resetpwlost theiroklines.go test ./...exiting 0 is conclusive for pass/fail across every package, so the argument holds. I additionally ranTestDeliveryReplay_PostOnlyAndCSRFProtectedby itself and captured--- PASS ... (0.37s), so the replay control's survival through the #240 merge no longer rests on exit status alone. That test scrapes theactionURL and CSRF token out of the rendered merged template and posts them, so it would fail on a mangled form.Disclosure: the mutation probe and that single replay test were run as targeted
go testinvocations inside a pinnedgolang:1.26.1-bookwormcontainer rather than throughmake test, which takes no-runfilter. The authoritative full run is the gate below.Mutation probe on the defect from round 3: replacing the
slices.DeleteFuncfilter inNewRedactorwith a baretargetSecrets(t)makesTestRedactor_EmptyUserinfoDoesNotShredTheBodyfail with(redacted)o(redacted)k(redacted)=..., so the fix is live code and the test binds it. The filter sits inNewRedactorover the wholetargetSecretsresult, sourlSecrets,headerSecretsand any field added later are covered at the collection point. The test pins its own premise withrequire.NotNil(parsed.User)andrequire.Empty(parsed.User.String()), so a change in Go's parsing fails it loudly instead of silently ending coverage.Judging the deliberate asymmetry: no length floor on
urlSecretsis correct. A header is selected by a substring guess on its name and its value may be ordinary text, so a floor there suppresses false positives on data that was never secret; a destination URL's path, query and userinfo are operator-supplied credential material by position, and short ones are real (/aB3-style hook paths). Over-redaction costs a marker, under-redaction costs the credential.Gate on
03c8e46,docker build --no-cache-filter=lint --no-cache-filter=builder --progress=plain ., exit 0. Hostuptimeload average ranged 30.77 to 69.71 across the run.None of
#17,#19,#32,#36appears in theCACHEDset (#1,#3,#14,#15,#25-#29,#33-#35,#38-#40). Zero(cached)markers and zeroFAILlines in the log. Explicit--- PASScaptured forTestRedactor_EmptyUserinfoDoesNotShredTheBody,TestHandleSourceLogs_RendersFailedAttempt,TestHandleSourceLogs_RendersReplayControlAndBanner,TestDeliverHTTP_CutsStoredResponseAtMaxBodyLog,TestHandleSourceLogs_RedactsForSoftDeletedTargetandTestHandleSourceLogs_BoundsRenderedAttempts.Also verified: merges cleanly into
nextat3b0ed82by local test-merge; one commit titled(closes #202);TODO.mdand.golangci.ymluntouched; no Claude/Anthropic reference or attribution trailer in the diff, commit message or author fields; inclusive terminology clean; the duplicate-helper collision resolved without weakening either helper, the only change todelivery_replay_test.gobeing"application/json"tocontentTypeJSON;x-cloakon the new panel is backed by the existing inline rule intemplates/htmlheader.html, not the regenerated artefact.