fix: correct capacity drift, clamp payload length, pixel-align parallel chunks

[pableeee]

Addresses the findings from a full review of the working tree. Three substantive bugs, plus hygiene.

1. Capacity drift broke test-visual outright

cmd/steg carried its own imageCapacity() still using the pre-c69a626 overhead of 44 bytes (4-byte encrypted nonce), while the library had moved to 56 (16-byte plaintext salt). Every steg test-visual run failed on its first image:

Error: encode ch=1 bpc=1: steg: payload too large (468 bytes, capacity 456 bytes)

steg capacity also over-reported every cell by 12 bytes. The arithmetic now lives only in steg.CapacityBytes / steg.CapacityForDims with an exported Overhead constant, so the two copies cannot drift apart again.

2. Unauthenticated length field sized an allocation

The container length is decrypted but not yet MAC-verified when it is read, so a wrong password yields an essentially random uint32 and make([]byte, length) would reserve up to 4 GiB before any read failed. ReadPayload now takes a maxPayload bound and DecodeParallel applies the same check inline.

Visible on a wrong password:

Error: payload length 2122572625 exceeds maximum 98252: wrong password or corrupt image

TestExcessiveLength previously allocated 2 GiB on every run; it now asserts the rejection instead.

3. Parallel encode could silently corrupt an image

Chunk sizes were aligned to lcm(8, bitsPerPixel)/8 bytes, but the payload starts at bit 160 and 160 mod 3 = 1. With the default 3 channels, every chunk boundary landed mid-pixel, so the two workers on either side both loaded, modified, and stored that pixel — the later store discarding the other's bits, leaving an image that fails MAC verification on decode.

The shared mutex did not prevent this: each At and Set was already serialised, but the load-modify-store sequence was not atomic.

Verified the overlap arithmetically before changing anything:

config	bitsPerPixel	boundaries sharing a pixel
3ch x 1bpc (default)	3	8/8
3ch x 2bpc	6	8/8
3ch x 4bpc	12	8/8
3ch x 8bpc	24	8/8
1ch, 2ch (any bpc)	1, 2, 4, 8, 16	0/8

firstChunkShift now offsets the first chunk so every later boundary falls on a pixel boundary.

Note on testing: this is a lost update between two mutex-protected accesses, so the race detector cannot see it, and a round-trip test passes by luck most of the time — the existing parallel tests used single-chunk payloads and never hit a boundary at all. TestChunkBoundariesArePixelAligned therefore checks the invariant arithmetically, with TestParallelMultiChunkRoundTrip covering the end-to-end path at full capacity across five configurations.

Also fixed

• Dispatch deadlock — both loops could block forever on jobChan if every worker had already exited on error. They now select on an abort channel closed by the first failure.
• Sub-image addressing — the pixel sequence was generated over Bounds().Max rather than Dx/Dy and never offset by Bounds().Min, so a carrier with a non-zero origin read zeroes and dropped writes outside its own bounds. Confirmed the new tests fail without the offset.
• steg decode created the output file before decoding, leaving a zero-byte file behind on a wrong password. It now decodes first.
• Password handling — --password exposes the passphrase in the process table and shell history. resolvePassword falls back to STEG_PASSWORD and then an interactive prompt, and refuses to run passwordless when stdin is not a terminal.
• go vet ./... failed on cursors: ReadByte/WriteByte were declared with uint8 rather than byte, tripping stdmethods. Generated gomock recorders trip the same check unfixably, so make vet excludes mocks/.
• Stale comments — container basePos described as "after nonce" (it is after the salt), parallel.go calling the salt encrypted (it is plaintext), and cipher's mixIndex typo for minIndex.

CI

Adds .github/workflows/test.yml running vet, tests, the race detector, and a build on pull requests and non-master pushes. Until now the only workflow ran on pushes to master, so a broken change was caught only after merge — when it blocked the release job.

The second commit adds the previously-untracked attack analysis tooling and writeup. attack2 and attack3 called fmt.Println with arguments already ending in a newline, which fails vet and therefore go test ./...; committing them as-is would have broken the release job on the next push to master.

Verification

gofmt clean, make vet clean, go build ./..., go test ./... passes, go test -race ./... passes, govulncheck 0 reachable, CLI round-trips confirmed sequential, parallel, and both interop directions.

Behavior change worth a look

Making --password optional means scripts relying on cobra rejecting a missing -p will now prompt or read STEG_PASSWORD instead.

🤖 Generated with Claude Code