Skip to content
Draft
1 change: 1 addition & 0 deletions PLAN.md
Original file line number Diff line number Diff line change
Expand Up @@ -259,4 +259,5 @@ footer: |
| 2607191918 | ✅ | haiku | [Deduplicate isClaimed between internal/schema and requiredstructure](plan/2607191918_arch-fix-isclaimed-dedup.md) |
| 2607242010 | 🔲 | sonnet | [MDS072 external-link-check: SSRF and egress hardening](plan/2607242010_mds072-ssrf-network-hardening.md) |
| 2607242011 | 🔲 | haiku | [Security hardening batch — 2026-07-24](plan/2607242011_security-hardening-batch-2026-07-24.md) |
| 2608020650 | 🔲 | sonnet | [Convert MDS003/MDS005 to KindScopedChecker without a stale-state bug](plan/2608020650_kindscoped-heading-rules.md) |
<?/catalog?>
70 changes: 55 additions & 15 deletions cmd/mdsmith/builddiag.go
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
package main

import (
"bytes"
"context"
"crypto/sha256"
"fmt"
"io"
"os"
Expand Down Expand Up @@ -206,31 +206,71 @@ func verifyTarget(
}
}

// snapshotOutputs reads every declared output of bt from disk into a
// path→bytes map. A missing output maps to nil.
func snapshotOutputs(bt buildTarget) map[string][]byte {
out := make(map[string][]byte, len(bt.target.Outputs))
// outputHash is one declared output's verify-pass fingerprint: ok is
// false when the file is missing or unreadable, true with hash
// holding its sha256 otherwise. A missing output (ok == false) now
// compares unequal to a present-but-empty one (ok == true, the sha256
// of zero bytes) in outputsEqual — a deliberate tightening versus the
// pre-streaming code, whose bytes.Equal(nil, []byte{}) treated the two
// as the same "no content" case. A recipe whose first run leaves an
// output absent and whose second leaves it present-but-empty (or vice
// versa) is exactly the non-determinism --build-verify exists to
// catch, so the stricter comparison is intentional (see
// TestOutputsEqual_MissingVsEmpty_ReturnsFalse).
type outputHash struct {
hash [sha256.Size]byte
ok bool
}

// snapshotOutputs hashes every declared output of bt from disk into a
// path→outputHash map. --build-verify only needs to know whether two
// runs produced identical bytes, so each output is streamed through a
// sha256 hash instead of read whole into memory — a declared output can
// be an arbitrarily large binary or bundle, and holding two full copies
// per verify pass is the "os.ReadFile on huge inputs" anti-pattern
// (docs/development/high-performance-go.md). A missing or unreadable
// output maps to the zero outputHash (ok == false).
func snapshotOutputs(bt buildTarget) map[string]outputHash {
out := make(map[string]outputHash, len(bt.target.Outputs))
for _, rel := range bt.target.Outputs {
abs := filepath.Join(bt.target.Root, filepath.FromSlash(rel))
data, err := os.ReadFile(abs) //nolint:gosec // abs is an in-root declared output
if err != nil {
out[rel] = nil
continue
}
out[rel] = data
out[rel] = hashOutputFile(abs)
}
return out
}

// outputsEqual reports whether two output snapshots hold identical bytes
// for every key.
func outputsEqual(a, b map[string][]byte) bool {
// hashOutputFile streams abs through sha256, never holding its full
// content in memory. A missing or unreadable file returns the zero
// outputHash.
func hashOutputFile(abs string) outputHash {
f, err := os.Open(abs) //nolint:gosec // abs is an in-root declared output
if err != nil {
return outputHash{}
}
defer f.Close() //nolint:errcheck // best-effort close on read-only file

h := sha256.New()
if _, err := io.Copy(h, f); err != nil {
return outputHash{}
}
var oh outputHash
oh.ok = true
// Sum appends to its argument; passing the fixed array's own
// zero-length slice fills it in place instead of allocating a new
// 32-byte slice to then copy from.
h.Sum(oh.hash[:0])
return oh
}

// outputsEqual reports whether two output snapshots hold identical
// content for every key.
func outputsEqual(a, b map[string]outputHash) bool {
if len(a) != len(b) {
return false
}
for k, av := range a {
bv, ok := b[k]
if !ok || !bytes.Equal(av, bv) {
if !ok || av != bv {
return false
}
}
Expand Down
52 changes: 52 additions & 0 deletions cmd/mdsmith/builddiag_memcap_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
package main

import (
"os"
"path/filepath"
"runtime"
"testing"

"github.com/stretchr/testify/require"

buildexec "github.com/jeduden/mdsmith/internal/build"
)

// TestSnapshotOutputs_LargeFile_BoundedMemory pins
// docs/development/high-performance-go.md's "os.ReadFile on huge
// inputs — one giant alloc, all resident — use bufio.Reader with a
// tuned buffer" (Patterns to avoid). --build-verify's snapshotOutputs
// must not hold a declared build output's full content in memory: it
// only needs to know whether two runs produced the same bytes, so a
// streamed content hash serves that without an alloc proportional to
// the output's size.
func TestSnapshotOutputs_LargeFile_BoundedMemory(t *testing.T) {
if testing.Short() {
t.Skip("large-file memory gate skipped in -short mode")
}
const size = 8 * 1024 * 1024 // 8 MiB
root := t.TempDir()
big := make([]byte, size)
for i := range big {
big[i] = byte(i)
}
require.NoError(t, os.WriteFile(filepath.Join(root, "out.bin"), big, 0o644))
bt := buildTarget{
target: buildexec.Target{Root: root, Outputs: []string{"out.bin"}},
}

var before, after runtime.MemStats
runtime.GC()
runtime.ReadMemStats(&before)
snap := snapshotOutputs(bt)
runtime.ReadMemStats(&after)

require.Len(t, snap, 1)
// The whole point of the fix: total heap growth must stay well under
// the output's size, proving the content was streamed through a
// hash rather than read whole into a []byte.
grew := after.TotalAlloc - before.TotalAlloc
if grew > size/4 {
t.Fatalf("snapshotOutputs on an %d-byte output allocated %d bytes; "+
"want well under the file size (no full-file buffering)", size, grew)
}
}
62 changes: 48 additions & 14 deletions cmd/mdsmith/buildpass_diag_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package main

import (
"context"
"crypto/sha256"
"errors"
"os"
"path/filepath"
Expand Down Expand Up @@ -160,51 +161,84 @@ func TestSnapshotOutputs_ExistingFile(t *testing.T) {
}
snap := snapshotOutputs(bt)
require.Len(t, snap, 1)
assert.Equal(t, []byte("hello"), snap["out.txt"])
// hashOf is an independent oracle (sha256.Sum256 directly), not the
// hashOutputFile helper snapshotOutputs itself delegates to — this
// ties the digest to the actual file content, not just to
// snapshotOutputs calling hashOutputFile with the right path.
assert.Equal(t, hashOf("hello"), snap["out.txt"])
}

func TestHashOutputFile_UnreadablePath_ReturnsZeroValue(t *testing.T) {
// os.Open succeeds on a directory; the failure surfaces on the
// subsequent io.Copy read, exercising hashOutputFile's second
// error branch (distinct from the os.Open error in the missing-
// file case above).
root := t.TempDir()
sub := filepath.Join(root, "a-directory")
require.NoError(t, os.Mkdir(sub, 0o755))
got := hashOutputFile(sub)
assert.False(t, got.ok)
}

func TestSnapshotOutputs_MissingFile_ReturnsNil(t *testing.T) {
func TestSnapshotOutputs_MissingFile_ReturnsZeroValue(t *testing.T) {
root := t.TempDir()
bt := buildTarget{
target: buildexec.Target{Root: root, Outputs: []string{"absent.txt"}},
}
snap := snapshotOutputs(bt)
require.Len(t, snap, 1)
assert.Nil(t, snap["absent.txt"])
assert.False(t, snap["absent.txt"].ok)
}

// --- outputsEqual ---

func hashOf(s string) outputHash {
return outputHash{hash: sha256.Sum256([]byte(s)), ok: true}
}

func TestOutputsEqual_IdenticalMaps_ReturnsTrue(t *testing.T) {
a := map[string][]byte{"a.txt": []byte("x"), "b.txt": []byte("y")}
b := map[string][]byte{"a.txt": []byte("x"), "b.txt": []byte("y")}
a := map[string]outputHash{"a.txt": hashOf("x"), "b.txt": hashOf("y")}
b := map[string]outputHash{"a.txt": hashOf("x"), "b.txt": hashOf("y")}
assert.True(t, outputsEqual(a, b))
}

func TestOutputsEqual_DifferentContent_ReturnsFalse(t *testing.T) {
a := map[string][]byte{"a.txt": []byte("x")}
b := map[string][]byte{"a.txt": []byte("y")}
a := map[string]outputHash{"a.txt": hashOf("x")}
b := map[string]outputHash{"a.txt": hashOf("y")}
assert.False(t, outputsEqual(a, b))
}

func TestOutputsEqual_DifferentKeys_ReturnsFalse(t *testing.T) {
a := map[string][]byte{"a.txt": []byte("x")}
b := map[string][]byte{"b.txt": []byte("x")}
a := map[string]outputHash{"a.txt": hashOf("x")}
b := map[string]outputHash{"b.txt": hashOf("x")}
assert.False(t, outputsEqual(a, b))
}

func TestOutputsEqual_DifferentLength_ReturnsFalse(t *testing.T) {
a := map[string][]byte{"a.txt": []byte("x"), "b.txt": []byte("y")}
b := map[string][]byte{"a.txt": []byte("x")}
a := map[string]outputHash{"a.txt": hashOf("x"), "b.txt": hashOf("y")}
b := map[string]outputHash{"a.txt": hashOf("x")}
assert.False(t, outputsEqual(a, b))
}

func TestOutputsEqual_BothNilValue_ReturnsTrue(t *testing.T) {
a := map[string][]byte{"a.txt": nil}
b := map[string][]byte{"a.txt": nil}
func TestOutputsEqual_BothMissingValue_ReturnsTrue(t *testing.T) {
a := map[string]outputHash{"a.txt": {}}
b := map[string]outputHash{"a.txt": {}}
assert.True(t, outputsEqual(a, b))
}

// TestOutputsEqual_MissingVsEmpty_ReturnsFalse pins the deliberate
// tightening documented on outputHash: a first run that leaves a
// declared output absent and a second that leaves it present-but-empty
// (or vice versa) is real non-determinism, so it must not compare
// equal the way bytes.Equal(nil, []byte{}) treated the pre-streaming
// code's os.ReadFile results.
func TestOutputsEqual_MissingVsEmpty_ReturnsFalse(t *testing.T) {
missing := map[string]outputHash{"a.txt": {}}
empty := map[string]outputHash{"a.txt": hashOf("")}
assert.False(t, outputsEqual(missing, empty))
assert.False(t, outputsEqual(empty, missing))
}

// --- printVerdict ---

func TestPrintVerdict_StaleVerdictWritten(t *testing.T) {
Expand Down
52 changes: 52 additions & 0 deletions internal/rules/markdownflavor/alloc_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
package markdownflavor

import (
"strings"
"testing"

"github.com/stretchr/testify/assert"
)

// manyAlertLinesSource builds a document with n GitHub-alert blockquotes,
// each with an indented lazy-continuation line (no "> " prefix in the raw
// source), so fixGitHubAlerts exercises both its skip (marker-line
// removal) and addPrefix (continuation-line rewrite) branches n times
// per call.
func manyAlertLinesSource(n int) string {
var b strings.Builder
for i := 0; i < n; i++ {
b.WriteString("> [!NOTE]\n indented lazy continuation body.\n\n")
}
return b.String()
}

// TestFixGitHubAlerts_LowAllocs pins docs/development/high-performance-go.md's
// "pre-size slices" and "stay in []byte" patterns: fixGitHubAlerts rebuilds
// the whole file line by line, so every unmodified line should pass through
// without a string(line) copy, and the accumulator slice should be
// presized from len(f.Lines) instead of growing via repeated append.
func TestFixGitHubAlerts_LowAllocs(t *testing.T) {
if testing.Short() {
t.Skip("alloc gate skipped in -short mode")
}
if raceEnabled {
t.Skip("alloc gate skipped under -race; the race detector's " +
"instrumentation overhead perturbs the allocation count")
}
r := &Rule{}
f := mkFile(t, manyAlertLinesSource(50))

allocs := testing.AllocsPerRun(50, func() {
r.fixGitHubAlerts(f)
})
// Measured on this addPrefix-heavy 50-alert fixture: 176 allocs on
// the pre-fix code (one string(line) copy per source line plus an
// unsized, growing []string), 70 after — fixGitHubAlerts' own
// rewrite loop (one presized rewritten-line buffer per addPrefix
// line, no string(line) copy for the blank/skip lines that pass
// through unchanged) plus buildAlertSkipMaps' walk, which also
// stays in []byte for its continuation-line scan. Budget with
// headroom over the measured post-fix count.
assert.LessOrEqualf(t, allocs, 90.0,
"fixGitHubAlerts allocs regressed: got %v, want <= 90", allocs)
}
21 changes: 21 additions & 0 deletions internal/rules/markdownflavor/fix_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,27 @@ func TestFixGitHubAlerts_LazyContinuation(t *testing.T) {
assert.Equal(t, "> This is lazy-continuation body.\n", got)
}

// TestFixGitHubAlerts_IndentedLazyContinuation covers the leading-
// whitespace branch of the addPrefix rewrite (rule.go's
// line[:len(line)-len(trimmed)] slice): the continuation line's
// existing indent must be preserved ahead of the re-added "> ".
//
// This test is scoped to a 3-space indent, where preserving it keeps
// the result a valid blockquote. An indented chunk cannot interrupt a
// paragraph (CommonMark §4.4), so a continuation indented 4+ spaces is
// still lazy continuation too — addPrefix fires for it exactly the
// same way — but preserving 4+ spaces of indent ahead of "> " makes
// the rewritten line parse as indented code, not a blockquote,
// silently turning the alert body into a code block. That bug
// predates this PR (byte-identical on origin/main) and this rewrite
// does not change it; it is not covered by a test here because fixing
// it is out of scope for this performance-focused change.
func TestFixGitHubAlerts_IndentedLazyContinuation(t *testing.T) {
src := "> [!NOTE]\n indented lazy continuation body.\n"
got := fixWith(t, "commonmark", src)
assert.Equal(t, " > indented lazy continuation body.\n", got)
}

// TestRuleFixStrikethroughWithNestedInlineSkips guards the robustness
// of delimiterPairEdits: a wrapper containing nested inline markup
// (emphasis, link, code span) cannot be safely unwrapped without
Expand Down
5 changes: 5 additions & 0 deletions internal/rules/markdownflavor/race_off_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
//go:build !race

package markdownflavor

const raceEnabled = false
5 changes: 5 additions & 0 deletions internal/rules/markdownflavor/race_on_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
//go:build race

package markdownflavor

const raceEnabled = true
Loading
Loading