fix: serverInfo map data race and XMSSMT nil-deref - #15
Merged
Conversation
computePowNonces mutated serverInfo.RequiredProofOfWork in place under the lock, but getServerInfo hands callers a struct copy that shares that map and handlers read it without the lock — a concurrent map read/write that the race detector flags and that can crash the process with "fatal error: concurrent map read and map write". Build the map fresh and publish it by replacing the reference under the lock so readers always see an immutable snapshot. processAtumRequest logged but swallowed errors from CreateXMSSMTTimestamp, leaving resp.Stamp nil and then dereferencing it at resp.Stamp.ServerUrl. Return an error response to the client instead. Add a -race regression test for the serverInfo map access. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
/dobby the pipeline is failing, fix it |
Author
|
Dobby is on it! 🧦 The pipeline is failing at the Grype image scan step — it found vulnerability matches at critical severity or above in the built Docker image. Dobby will investigate the CVEs and push a fix (base image bump or dependency update) to this branch. |
The Delivery pipeline's "Scan Image" (Anchore/Grype, --fail-on critical) step failed: the built image (scratch + static Go binary) carried 7 critical advisories from golang.org/x/crypto v0.51.0, all fixed in v0.52.0 (GO-2026-5005/5006/5017/5019/5020/5021/5023). - Bump golang.org/x/crypto v0.51.0 -> v0.52.0 (pulls golang.org/x/sys v0.44.0 -> v0.45.0). x/crypto is a direct import (ed25519, sha3). - Bump toolchain go1.25.10 -> go1.25.11 to clear the remaining High stdlib advisories (CVE-2026-42504, GO-2026-5038); below the critical gate but makes the published image vulnerability-clean. Verified locally: go build, go vet, go test -race ./... all pass, and grype --fail-on critical --only-fixed reports no vulnerabilities on the rebuilt static binary. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
rubenhensen
approved these changes
Jun 12, 2026
w-ensink
approved these changes
Jun 12, 2026
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
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.
Two correctness bugs found during the scheduled code-quality audit (atumd has Issues disabled upstream; findings tracked in encryption4all/dobby#88).
1. Data race on
serverInfo.RequiredProofOfWorkcomputePowNoncesmutates the map in place underserverInfoLock, butgetServerInfohands callers a struct copy that shares the same map, and the handlers (serverInfoHandler,processAtumRequest) read that map without holding the lock. That is a concurrent map read/write: the race detector flags it, and in production it can crash the whole process withfatal error: concurrent map read and map writewhen the nonce revolver fires while a request is in flight.Fix: build the map fresh in
computePowNoncesand publish it by replacing the reference under the lock. The map is never mutated in place after publication, so readers always observe an immutable snapshot.2. Nil-pointer dereference on XMSSMT stamp failure
In
processAtumRequest, an error fromstamper.CreateXMSSMTTimestampwas logged but swallowed, leavingresp.Stampnil — then the code fell through toresp.Stamp.ServerUrl = …, dereferencing nil and panicking the request. Now it returns aninternal errorresponse to the client instead.Tests
TestServerInfoConcurrentAccess, a-raceregression test for the map access. Verified it reportsDATA RACEagainst the old in-place mutation and passes with the fix.go build,go vet, andgo test -race ./...all clean locally.Pipeline fix (2026-06-12)
The Delivery workflow was failing at the Scan Image step (Anchore/Grype,
--fail-on critical): thescratchimage's static Go binary carried 7 critical advisories fromgolang.org/x/crypto v0.51.0(GO-2026-5005/5006/5017/5019/5020/5021/5023), all fixed inv0.52.0.golang.org/x/cryptov0.51.0 → v0.52.0 (direct import:ed25519,sha3; pullsgolang.org/x/sysv0.44.0 → v0.45.0).toolchaingo1.25.10 → go1.25.11 to also clear the remaining High stdlib advisories (below the critical gate, but makes the image vulnerability-clean).Verified locally:
go build,go vet,go test -race ./...pass, andgrype --fail-on critical --only-fixedon the rebuilt static binary reports no vulnerabilities at any severity.