diff --git a/CHANGELOG.md b/CHANGELOG.md index 63ccc83e..58e84e11 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -92,6 +92,12 @@ because it turns other people's test suites red. ### Fixed +- **A damaged file no longer claims to carry its label.** Its manifest entry + said `label_embedded: true`, although the damage comes after the label is + written and can overwrite it. On a text file, `zero-head` zeroes the start of + the label itself. A damaged file now says `false`, in its `properties` too, + with a `label_not_guaranteed` note saying why. An undamaged file is + described as before, and `manifest_version` does not change. - **A recipe asking for more than a million files in all is refused before it takes the memory.** Targets were held to a million each and the run only once all their sizes were laid out, so `tfg validate` took 2.5 GB to refuse diff --git a/internal/damage/damage.go b/internal/damage/damage.go index 0c6641ab..7ea1fe81 100644 --- a/internal/damage/damage.go +++ b/internal/damage/damage.go @@ -19,7 +19,9 @@ // What this package does NOT do is decide whether a damaged file is worth // writing. That is the witness rule - a pair with no judge able to refuse the // result is a pair this tool does not offer - and it belongs with the oracles, -// because it is a measurement rather than a declaration. +// because it is a measurement rather than a declaration. The guard +// TestEveryDamageIsRefusedByTheStructuralCheckInEveryFormat takes that +// measurement for every format and every damage registered here. package damage import ( diff --git a/internal/engine/engine.go b/internal/engine/engine.go index 393cec59..6042a69c 100644 --- a/internal/engine/engine.go +++ b/internal/engine/engine.go @@ -7,6 +7,7 @@ import ( "errors" "fmt" "io/fs" + "maps" "os" "path/filepath" "strings" @@ -728,6 +729,21 @@ func (res *Result) entryFor(f PlannedFile, sha string, materialized bool, failur } label, _ := f.Plan.Properties[format.PropertyLabelEmbedded].(bool) + props := f.Plan.Properties + if label && f.Damaged() { + // The generator wrote the label and a damage came after it. Where the + // label sits differs by format - the first line of a text, pixels in + // a picture, a part inside a package - so whether a damage reached it + // cannot be said once for all of them. Measured 2026-11-09: zero-head + // on txt overwrote the label it sat beside, and the manifest still + // claimed it. So a damaged file never claims its label, and the note + // says why (docs/DAMAGE-ETAP-2-2026-11-09.md section 4.7). + label = false + props = maps.Clone(props) + props[format.PropertyLabelEmbedded] = false + notes = append(notes, res.note("label_not_guaranteed", + core.Says("engine.LabelNotGuaranteed", "The label was written before the file was damaged, so the damage may have overwritten it."))) + } e := manifest.File{ ID: f.ID, @@ -741,7 +757,7 @@ func (res *Result) entryFor(f PlannedFile, sha string, materialized bool, failur Seed: core.SeedLabel(f.Seed), Generator: manifest.GeneratorRef{Name: f.Desc.ID, Version: f.Desc.GeneratorVersion}, Determinism: string(f.Plan.Determinism), - Properties: f.Plan.Properties, + Properties: props, LabelEmbedded: label, Damage: damageFor(f), Notes: notes, diff --git a/internal/guard/damagelabel_test.go b/internal/guard/damagelabel_test.go new file mode 100644 index 00000000..53251924 --- /dev/null +++ b/internal/guard/damagelabel_test.go @@ -0,0 +1,82 @@ +package guard + +import ( + "path/filepath" + "slices" + "testing" + + "github.com/donislawdev/TestingFilesGenerator/internal/damage" + "github.com/donislawdev/TestingFilesGenerator/internal/engine" + "github.com/donislawdev/TestingFilesGenerator/internal/format" + "github.com/donislawdev/TestingFilesGenerator/internal/manifest" +) + +// labelNotGuaranteed is the note a damaged file carries in place of a claim +// to its label. +const labelNotGuaranteed = "label_not_guaranteed" + +// TestADamagedFileNeverClaimsItsLabel holds the manifest to what it can know +// about a label after a damage. +// +// Measured 2026-11-09: zero-head on txt overwrote the label at the start of +// the text, and the manifest still said label_embedded: true. Where a label +// sits differs by format, so the rule is the same for all of them. A damaged +// file says false, in both places the manifest says it, and a note says why. +// +// Every format, each beside a healthy file of the same format and size. The +// healthy one is the control: a format that never claims its label would pass +// this guard without the rule being there, so the guard also counts the +// formats where the claim was really taken away. +func TestADamagedFileNeverClaimsItsLabel(t *testing.T) { + dir := t.TempDir() + chain := damage.Chain{{ID: damage.ZeroHead}} + + withdrawn := 0 + for _, f := range format.All() { + out := filepath.Join(dir, f.ID) + targets := []engine.Target{ + {ID: "good", Format: f.ID, Sizes: engine.Uniform(1, witnessSize), Label: true}, + {ID: "broken", Format: f.ID, Sizes: engine.Uniform(1, witnessSize), Damage: chain, Label: true}, + } + opt := engine.Options{OutDir: out, Seed: 5, ManifestName: engine.DefaultManifestName} + planned, err := engine.Plan(targets, opt) + if err != nil { + t.Fatalf("%s: planning: %v", f.ID, err) + } + res, err := engine.Run(t.Context(), planned, opt) + if err != nil { + t.Fatalf("%s: running: %v", f.ID, err) + } + entries := map[string]manifest.File{} + for _, m := range res.Manifest.Files { + entries[m.TargetID] = m + } + good, broken := entries["good"], entries["broken"] + if good.Name == "" || broken.Name == "" { + t.Fatalf("%s: the run did not write both files", f.ID) + } + + if broken.LabelEmbedded { + t.Errorf("%s: a damaged file claims label_embedded: true.\n"+ + "Reason: the damage came after the label was written and may have overwritten it,\n"+ + "so the manifest cannot know the label is still there.", f.ID) + } + if claim, ok := broken.Properties[format.PropertyLabelEmbedded].(bool); ok && claim { + t.Errorf("%s: a damaged file claims label_embedded: true in its properties, "+ + "while the field beside them says false", f.ID) + } + noted := slices.ContainsFunc(broken.Notes, func(n manifest.Note) bool { return n.Code == labelNotGuaranteed }) + if noted != good.LabelEmbedded { + t.Errorf("%s: the healthy file claims its label: %t, and the damaged one carries the note %s: %t.\n"+ + "Reason: the note says why a label that was written is no longer claimed, so it\n"+ + "belongs exactly where a claim was taken away.", f.ID, good.LabelEmbedded, labelNotGuaranteed, noted) + } + if good.LabelEmbedded { + withdrawn++ + } + } + if withdrawn == 0 { + t.Fatal("no healthy file claimed its label, so this guard took nothing away and proved nothing") + } + t.Logf("the claim was taken away in %d format(s)", withdrawn) +} diff --git a/internal/guard/damagerefused_test.go b/internal/guard/damagerefused_test.go index 5150dc97..2079a48e 100644 --- a/internal/guard/damagerefused_test.go +++ b/internal/guard/damagerefused_test.go @@ -13,53 +13,14 @@ import ( "github.com/donislawdev/TestingFilesGenerator/internal/cli" "github.com/donislawdev/TestingFilesGenerator/internal/damage" "github.com/donislawdev/TestingFilesGenerator/internal/engine" - "github.com/donislawdev/TestingFilesGenerator/internal/format" _ "github.com/donislawdev/TestingFilesGenerator/internal/format/all" "github.com/donislawdev/TestingFilesGenerator/internal/manifest" - "github.com/donislawdev/TestingFilesGenerator/internal/oracle" "github.com/donislawdev/TestingFilesGenerator/internal/recipe" ) -// A damaged file has to be REFUSED, which is the reverse of everything else in -// this package. -// -// Every other oracle guard here asks "does a judge accept what we wrote". This -// one asks the opposite, and it is the whole reason the damage axis is worth -// having: a broken file no reader complains about is a file nobody can write a -// test against, so it is a file this tool has no business producing. -// -// The formats are the text ones plus png, because those are the ones whose -// judges run without an external tool being installed. That is a limit of what -// can be asked on any machine rather than of the damage: the full sweep across -// all twenty four lives in tools/probes/corruptwitness, which is where the -// witness matrix is measured. -func TestADamagedFileIsRefusedByAJudge(t *testing.T) { - dir := t.TempDir() - - for _, id := range []string{"txt", "md", "json", "xml", "csv", "html", "png"} { - t.Run(id, func(t *testing.T) { - good, broken := writeGoodAndBroken(t, dir, id) - - // The control first. Without it this test passes for a build whose - // judge refuses everything, which would be the loudest possible - // way of proving nothing. - if res := oracle.Strict(id, good); res.Available && res.Err != nil { - t.Fatalf("the undamaged file was refused, so this row says nothing about damage: %v\n%s", - res.Err, res.Output) - } - - res := oracle.Strict(id, broken) - if !res.Available { - t.Skipf("no structural check for %s on this machine", id) - } - if res.Err == nil { - t.Errorf("the damaged file was ACCEPTED. A broken file every reader takes is one "+ - "nobody can write a test against, and the manifest calls it expected: reject.\n%s", - res.Output) - } - }) - } -} +// Whether a damaged file is refused by a judge is asked of every format and +// every damage in damagewitness_test.go, which replaced the seven formats +// asked here until 2026-11-09. // The manifest of a damaged run says what was broken and expects a rejection. // @@ -157,26 +118,6 @@ type discard struct{} func (discard) Write(p []byte) (int, error) { return len(p), nil } -// writeGoodAndBroken produces one file of this format twice, once whole and -// once damaged, and returns both paths. -func writeGoodAndBroken(t *testing.T, dir, id string) (good, broken string) { - t.Helper() - - d, err := format.Get(id) - if err != nil { - t.Fatal(err) - } - size := d.SmallestAccepted(format.Request{}) + 4096 - - goodDir := filepath.Join(dir, id+"-good") - brokenDir := filepath.Join(dir, id+"-broken") - goodMan := runOne(t, goodDir, id, size, nil) - brokenMan := runOne(t, brokenDir, id, size, damage.Chain{{ID: damage.ZeroHead}}) - - return filepath.Join(goodDir, goodMan.Files[0].Name), - filepath.Join(brokenDir, brokenMan.Files[0].Name) -} - // runDamaged is one png of a comfortable size, with the chain given. func runDamaged(t *testing.T, dir, id string, chain damage.Chain) *manifest.Manifest { t.Helper() diff --git a/internal/guard/damagewitness_test.go b/internal/guard/damagewitness_test.go new file mode 100644 index 00000000..bad5a618 --- /dev/null +++ b/internal/guard/damagewitness_test.go @@ -0,0 +1,224 @@ +package guard + +import ( + "fmt" + "path/filepath" + "strconv" + "strings" + "testing" + + "github.com/donislawdev/TestingFilesGenerator/internal/damage" + "github.com/donislawdev/TestingFilesGenerator/internal/engine" + "github.com/donislawdev/TestingFilesGenerator/internal/format" + "github.com/donislawdev/TestingFilesGenerator/internal/oracle" +) + +// witnessSize is the comfortable size every pair is also judged at, beside the +// smallest one it can be given. Measured 2026-11-09: the set of witnesses was +// the same at 20 kB and at 1 MiB in every format, while the smallest size is +// where the answer moved (docs/DAMAGE-ETAP-2-2026-11-09.md section 2.1). +const witnessSize = 20 << 10 + +// damagePoint is one damage with one set of values. +type damagePoint struct { + id string + values damage.Values + label string +} + +// witnessCase is one damaged file and the healthy file of the same format +// and size beside it. +type witnessCase struct { + format string + point damagePoint + size int64 + good string + broken string +} + +// TestEveryDamageIsRefusedByTheStructuralCheckInEveryFormat is the witness +// rule as a guard rather than a measurement taken once. +// +// A damage is offered for a format only when a judge refuses the result, so a +// test written against the file can tell an upload validator that let it +// through. Until 2026-11-09 that was a sweep from September, and the offer +// covered every format, including one added the day after the sweep. Now +// every format in the registry meets every damage in the registry, with its +// default values and each parameter at both ends, at the smallest size the +// pair can be given and at 20 kB. Each file comes from the real engine with +// its label, as both surfaces write by default, and the structural checker +// judges it beside a healthy file of the same format and size. +// +// Red when a damaged file is accepted, and red when the checker falls over +// instead of answering: a crash is a fault in the checker, never a witness. A +// format the checker does not know is red too. Nobody has decided whether a +// damage can be offered there, and the offer must not reach it by default. +// +// Measured at the default settings of each format. A format whose settings +// change the answer (the shape of a log line, the separator of a table) is +// judged here only as it comes by default, and that limit is written down +// in docs/DAMAGE-ETAP-2-2026-11-09.md section 2.2. +func TestEveryDamageIsRefusedByTheStructuralCheckInEveryFormat(t *testing.T) { + dir := t.TempDir() + + var points []damagePoint + for _, d := range damage.All() { + points = append(points, damagePointsOf(t, d)...) + } + if len(points) == 0 { + t.Fatal("the damage registry is empty, so this guard would pass without judging anything") + } + + var cases []witnessCase + formats := format.All() + for _, f := range formats { + if !oracle.StrictKnows(f.ID) { + t.Errorf("%s has no structural check, so no damage has a witness there.\n"+ + "Reason: the offer of a damage reaches every format by default, so a format\n"+ + "nobody can judge would be handed damaged files nothing refuses. Give it a\n"+ + "check in internal/oracle/strict.py before it is registered.", f.ID) + continue + } + cases = append(cases, witnessCasesOf(t, dir, f, points)...) + } + + reqs := make([]oracle.StrictRequest, 0, 2*len(cases)) + for _, c := range cases { + reqs = append(reqs, + oracle.StrictRequest{Format: c.format, Path: c.good}, + oracle.StrictRequest{Format: c.format, Path: c.broken}) + } + got, available, err := oracle.StrictBatch(t.Context(), reqs) + if !available { + t.Skip("the structural checker needs python, and there is none on this machine") + } + if err != nil { + t.Fatal(err) + } + + witnessed := 0 + for i, c := range cases { + good, broken := got[2*i], got[2*i+1] + where := fmt.Sprintf("%s at %d B with %s", c.format, c.size, c.point.label) + if good.Verdict != oracle.StrictOK { + // The control. A checker that refused everything would witness + // every damage, so a pair whose healthy file is refused says + // nothing about the damage. + t.Errorf("%s: the healthy file was answered %s %q, so this pair says nothing about the damage", + where, good.Verdict, good.Reason) + continue + } + switch broken.Verdict { + case oracle.StrictFail: + witnessed++ + case oracle.StrictOK: + t.Errorf("%s: the damaged file was ACCEPTED (%s).\n"+ + "Reason: a damaged file nothing refuses is one no test can be written against,\n"+ + "and its manifest calls it expected: reject. Either the damage is not offered\n"+ + "for this format, or the structural check is missing a rule of the format.", + where, broken.Reason) + default: + t.Errorf("%s: the checker answered %s %q about the damaged file.\n"+ + "Reason: that is a fault in the checker, not a refusal of the file. Counted as\n"+ + "a witness, a checker that falls over on every damaged file would witness\n"+ + "every damage.", where, broken.Verdict, broken.Reason) + } + } + if want := len(formats) * len(points); len(cases) < want { + t.Errorf("%d pair(s) judged, and %d formats by %d damage points need at least %d", + len(cases), len(formats), len(points), want) + } + t.Logf("%d damaged file(s) refused, across %d formats and %d damage point(s)", + witnessed, len(formats), len(points)) +} + +// damagePointsOf is one damage with its defaults, and with each parameter at +// both ends of what it accepts. +func damagePointsOf(t *testing.T, d damage.Descriptor) []damagePoint { + t.Helper() + points := []damagePoint{{id: d.ID, values: damage.Values{}, label: d.ID + " at its defaults"}} + for _, p := range d.Parameters { + var values []string + switch p.Kind { + case format.PropertyInt: + values = []string{strconv.FormatInt(p.Min, 10), strconv.FormatInt(p.Max, 10)} + case format.PropertyChoice: + values = p.Choices + default: + t.Errorf("%s has a parameter %s of kind %s, which this guard does not know how to sweep.\n"+ + "Reason: a value nobody judged is a value whose files may have no witness.", + d.ID, p.Name, p.Kind) + } + for _, v := range values { + points = append(points, damagePoint{ + id: d.ID, + values: damage.Values{p.Name: v}, + label: fmt.Sprintf("%s %s=%s", d.ID, p.Name, v), + }) + } + } + return points +} + +// witnessCasesOf writes, for one format, a healthy and a damaged file of each +// damage point at the two sizes, all in one run of the engine. +func witnessCasesOf(t *testing.T, dir string, f format.Descriptor, points []damagePoint) []witnessCase { + t.Helper() + // With the label, because that is how both surfaces write by default. + smallest := f.SmallestAccepted(format.Request{Label: true}) + + var targets []engine.Target + var cases []witnessCase + sized := map[int64]bool{} + for n, p := range points { + chain := damage.Chain{{ID: p.id, Values: p.values}} + floor, _, err := chain.Floor() + if err != nil { + t.Fatalf("%s: %v", p.label, err) + } + for _, size := range witnessSizes(max(smallest, floor)) { + id := fmt.Sprintf("p%d-%d", n, size) + targets = append(targets, engine.Target{ID: id, Format: f.ID, Sizes: engine.Uniform(1, size), Damage: chain, Label: true}) + cases = append(cases, witnessCase{format: f.ID, point: p, size: size, broken: id}) + if !sized[size] { + sized[size] = true + targets = append(targets, engine.Target{ID: fmt.Sprintf("good-%d", size), Format: f.ID, Sizes: engine.Uniform(1, size), Label: true}) + } + } + } + + out := filepath.Join(dir, f.ID) + opt := engine.Options{OutDir: out, Seed: 5, ManifestName: engine.DefaultManifestName} + planned, err := engine.Plan(targets, opt) + if err != nil { + t.Fatalf("%s: planning the witness files: %v", f.ID, err) + } + res, err := engine.Run(t.Context(), planned, opt) + if err != nil { + t.Fatalf("%s: writing the witness files: %v", f.ID, err) + } + paths := map[string]string{} + for _, m := range res.Manifest.Files { + if m.Failed { + t.Fatalf("%s: target %s failed: %s", f.ID, m.TargetID, m.Error) + } + paths[m.TargetID] = filepath.Join(out, filepath.FromSlash(m.Path)) + } + for i := range cases { + c := &cases[i] + c.good, c.broken = paths[fmt.Sprintf("good-%d", c.size)], paths[c.broken] + if c.good == "" || c.broken == "" || !strings.HasPrefix(c.broken, out) { + t.Fatalf("%s: the run wrote no file for %s at %d B", f.ID, c.point.label, c.size) + } + } + return cases +} + +// witnessSizes is the smallest size a pair can be given, and the comfortable +// one when that is larger. +func witnessSizes(smallest int64) []int64 { + if smallest >= witnessSize { + return []int64{smallest} + } + return []int64{smallest, witnessSize} +} diff --git a/internal/guard/mutation/entries.py b/internal/guard/mutation/entries.py index fa8189f9..e41d77be 100644 --- a/internal/guard/mutation/entries.py +++ b/internal/guard/mutation/entries.py @@ -5866,7 +5866,7 @@ "internal/engine/parallel.go", "\tif f.Damaged() {\n\t\tdamaged, opened, err := f.Target.Damage.Open(counter)", "\tif f.Damaged() && false {\n\t\tdamaged, opened, err := f.Target.Damage.Open(counter)", - "TestADamagedFileIsRefusedByAJudge", + "TestEveryDamageIsRefusedByTheStructuralCheckInEveryFormat", ), ( "the manifest stops recording what was broken about a file", @@ -14382,9 +14382,9 @@ "TestTheEngineHandsTheStopToAContainerItIsPlanning", ), ( - "a broken picture inside a page ends the structural check with a traceback", + "a broken picture inside a page stops counting against the page", "internal/oracle/strict.py", - " except Exception as exc:\n fail(f\"picture {n} is not a good {kind} file: {type(exc).__name__}: {str(exc)[:120]}\")\n", + " if said.word != \"OK\":\n fail(f\"picture {n} is not a good {kind} file: {said.reason}\")\n", "", "TestTheStructuralCheckGivesAVerdictOnABrokenPicture", ), @@ -17015,4 +17015,144 @@ 'SKIPPED_MANIFESTS = frozenset({".github/requirements-semgrep.txt"})', "TestTheDependencyGateRefusesALicenceThisProjectCannotDistribute", ), + ( + "the structural checker stops following the central directory to each local header, so an archive with a zeroed head passes", + "internal/oracle/strict.py", + ' if local + 30 > cd_at or data[local:local + 4] != b"PK\\x03\\x04":', + " if False:", + "TestTheStructuralCheckerRefusesBrokenBytesInsteadOfStopping", + ), + ( + "the structural checker stops comparing the name in a local header with the central directory", + "internal/oracle/strict.py", + " if local_name != name:", + " if False and local_name != name:", + "TestTheStructuralCheckerRefusesBrokenBytesInsteadOfStopping", + ), + ( + "an Office package skips the ZIP walk, so bytes after its end record pass", + "internal/oracle/strict.py", + " zip_structure(data)\n z = opc_open(data)", + " z = opc_open(data)", + "TestTheStructuralCheckerRefusesBrokenBytesInsteadOfStopping", + ), + ( + "a part of an Office package that cannot be read stops the check with a traceback", + "internal/oracle/strict.py", + " except (zipfile.BadZipFile, zlib.error, EOFError, NotImplementedError) as exc:", + " except KeyError as exc:", + "TestTheStructuralCheckerRefusesBrokenBytesInsteadOfStopping", + ), + ( + "a PNG cut inside its last checksum stops the check with a traceback", + "internal/oracle/strict.py", + ' if pos + 12 + length > len(data):\n fail(f"chunk {kind!r} ends at {len(data)} B, before its 4 B checksum")\n', + "", + "TestTheStructuralCheckerRefusesBrokenBytesInsteadOfStopping", + ), + ( + "a PNG that is only its signature stops the check with a traceback", + "internal/oracle/strict.py", + ' if not seen:\n fail("the signature is followed by no chunk at all")\n', + "", + "TestTheStructuralCheckerRefusesBrokenBytesInsteadOfStopping", + ), + ( + "a check that stops on an exception is counted as a refusal", + "internal/oracle/strict.py", + ' return "CRASH", f"{type(exc).__name__}: {str(exc)[:200]}"', + ' return "FAIL", f"{type(exc).__name__}: {str(exc)[:200]}"', + "TestACheckThatStopsOnAnExceptionIsACrashNotARefusal", + ), + ( + "a check that ends without a verdict is counted as an acceptance", + "internal/oracle/strict.py", + ' return "CRASH", "the check returned without a verdict"', + ' return "OK", "the check returned without a verdict"', + "TestACheckThatStopsOnAnExceptionIsACrashNotARefusal", + ), + ( + "a verdict given inside a check's own error handling is swallowed", + "internal/oracle/strict.py", + "class Verdict(SystemExit):", + "class Verdict(Exception):", + "TestACheckThatStopsOnAnExceptionIsACrashNotARefusal", + ), + ( + "the structural checker stops reading the PNG signature, so a PNG with a zeroed head passes", + "internal/oracle/strict.py", + ' if data[:8] != b"\\x89PNG\\r\\n\\x1a\\n":\n fail("the signature is not a PNG signature")', + ' if False:\n fail("the signature is not a PNG signature")', + "TestEveryDamageIsRefusedByTheStructuralCheckInEveryFormat", + ), + ( + "the structural checker stops reading the PDF header, so a PDF with a zeroed head passes", + "internal/oracle/strict.py", + ' if not data.startswith(b"%PDF-"):', + " if False:", + "TestEveryDamageIsRefusedByTheStructuralCheckInEveryFormat", + ), + ( + "a damaged file claims its label again, as the generator left it", + "internal/engine/engine.go", + " if label && f.Damaged() {", + " if false && label && f.Damaged() {", + "TestADamagedFileNeverClaimsItsLabel", + ), + ( + "a damaged file stops claiming its label in one place and keeps claiming it in its properties", + "internal/engine/engine.go", + " props[format.PropertyLabelEmbedded] = false\n", + "", + "TestADamagedFileNeverClaimsItsLabel", + ), + ( + "a damaged file stops claiming its label without saying why", + "internal/engine/engine.go", + ' notes = append(notes, res.note("label_not_guaranteed",', + ' _ = append(notes, res.note("label_not_guaranteed",', + "TestADamagedFileNeverClaimsItsLabel", + ), + ( + "a part of an Office package packed with a method no reader knows stops the check with a traceback", + "internal/oracle/strict.py", + "EOFError, NotImplementedError) as exc:", + "EOFError) as exc:", + "TestTheStructuralCheckerRefusesBrokenBytesInsteadOfStopping", + ), + ( + "a cross reference offset that is not a number stops the PDF check with a traceback", + "internal/oracle/strict.py", + " if not parts[0].isdigit():", + " if False:", + "TestTheStructuralCheckerRefusesBrokenBytesInsteadOfStopping", + ), + ( + "a label heading never closed stops the HTML check with a traceback", + "internal/oracle/strict.py", + " if label_end < 0:", + " if False:", + "TestTheStructuralCheckerRefusesBrokenBytesInsteadOfStopping", + ), + ( + "a batch line nested past what the JSON decoder goes stops the whole batch", + "internal/oracle/strict.py", + " except (ValueError, KeyError, TypeError, AttributeError, RecursionError) as exc:", + " except (ValueError, KeyError, TypeError, AttributeError) as exc:", + "TestTheStructuralCheckerAnswersARequestItCannotActOnWithAnError", + ), + ( + "a batch request for a path holding a NUL stops the whole batch", + "internal/oracle/strict.py", + " except (OSError, ValueError) as exc:", + " except OSError as exc:", + "TestTheStructuralCheckerAnswersARequestItCannotActOnWithAnError", + ), + ( + "the structural checker runs under a context of its own, so a stopped caller leaves it running", + "internal/oracle/strictbatch.go", + " ctx, cancel := context.WithTimeout(ctx, limit)", + " ctx, cancel := context.WithTimeout(context.Background(), limit)", + "TestAStoppedCallerStopsTheStructuralChecker", + ), ] diff --git a/internal/guard/strictverdict_test.go b/internal/guard/strictverdict_test.go new file mode 100644 index 00000000..ab837522 --- /dev/null +++ b/internal/guard/strictverdict_test.go @@ -0,0 +1,399 @@ +package guard + +import ( + "bytes" + "context" + "encoding/binary" + "encoding/json" + "os" + "os/exec" + "path/filepath" + "strconv" + "strings" + "testing" + + "github.com/donislawdev/TestingFilesGenerator/internal/oracle" +) + +// strictCase is one file handed to the structural checker, with what it has +// to answer. +type strictCase struct { + name string + format string + body []byte + // verdict is the answer wanted, and reason a piece of what it has to say. + // The reason is asked for because a refusal for some other cause would + // pass a guard that read the verdict alone. + verdict string + reason string +} + +// TestTheStructuralCheckerRefusesBrokenBytesInsteadOfStopping holds the three +// faults of the checker found when it became the judge of what a damage may be +// offered for (docs/DAMAGE-ETAP-2-2026-11-09.md section 3): +// +// - an archive whose first bytes are zeroed was accepted, because the check +// read the end records and never followed them to a local header +// - an Office package with bytes after its end record was accepted, because +// the ZIP reader underneath finds that record by searching for it +// - a PNG cut short and a package whose part cannot be read stopped the +// check with an exception, and an exception was counted as a refusal +// +// Each broken file is made from a healthy one this tool writes, and the +// healthy ones are asked first: a checker that refused everything would pass +// the rest of this guard. +func TestTheStructuralCheckerRefusesBrokenBytesInsteadOfStopping(t *testing.T) { + png := writeEncoded(t, "png", 2048, nil) + zip := writeEncoded(t, "zip", 16384, nil) + docx := writeEncoded(t, "docx", 4096, nil) + xlsx := writeEncoded(t, "xlsx", 4096, nil) + pptx := writeEncoded(t, "pptx", 8192, nil) + pdf := writeEncoded(t, "pdf", 4096, nil) + html := writeEncoded(t, "html", 2048, nil) + + cases := []strictCase{ + {"healthy png", "png", png, oracle.StrictOK, ""}, + {"healthy zip", "zip", zip, oracle.StrictOK, ""}, + {"healthy docx", "docx", docx, oracle.StrictOK, ""}, + {"healthy xlsx", "xlsx", xlsx, oracle.StrictOK, ""}, + {"healthy pptx", "pptx", pptx, oracle.StrictOK, ""}, + {"healthy pdf", "pdf", pdf, oracle.StrictOK, ""}, + {"healthy html", "html", html, oracle.StrictOK, ""}, + + {"png with nothing after the signature", "png", png[:8], oracle.StrictFail, "no chunk at all"}, + {"png one byte short", "png", png[:len(png)-1], oracle.StrictFail, "before its 4 B checksum"}, + {"zip with a zeroed head", "zip", zeroed(zip, 4), oracle.StrictFail, "no local file header"}, + {"zip whose local header names another file", "zip", renamed(t, zip), oracle.StrictFail, "and the central directory names"}, + {"docx with a zeroed head", "docx", zeroed(docx, 4), oracle.StrictFail, "no local file header"}, + {"xlsx with a zeroed head", "xlsx", zeroed(xlsx, 4), oracle.StrictFail, "no local file header"}, + {"pptx with a zeroed head", "pptx", zeroed(pptx, 4), oracle.StrictFail, "no local file header"}, + {"docx with bytes after its end", "docx", appended(docx, 64), oracle.StrictFail, "the comment declares 0 B and 64 follow"}, + {"docx with a part that does not match its checksum", "docx", flipped(t, docx), oracle.StrictFail, "cannot be read from the archive"}, + {"docx with a compression method nobody knows", "docx", unknownMethod(t, docx), oracle.StrictFail, "compression method"}, + {"pdf whose cross reference names an offset that is not a number", "pdf", garbledOffset(t, pdf), oracle.StrictFail, "which is not a number"}, + {"html whose label heading is never closed", "html", unclosedLabel(t, html), oracle.StrictFail, "never closed on its line"}, + } + + dir := t.TempDir() + reqs := make([]oracle.StrictRequest, len(cases)) + for i, c := range cases { + path := filepath.Join(dir, strings.ReplaceAll(c.name, " ", "_")+"."+c.format) + if err := os.WriteFile(path, c.body, 0o600); err != nil { + t.Fatal(err) + } + reqs[i] = oracle.StrictRequest{Format: c.format, Path: path} + } + + got, available, err := oracle.StrictBatch(t.Context(), reqs) + if !available { + t.Skip("the structural checker needs python, and there is none on this machine") + } + if err != nil { + t.Fatal(err) + } + for i, c := range cases { + j := got[i] + if j.Verdict != c.verdict || !strings.Contains(j.Reason, c.reason) { + t.Errorf("%s: the checker answered %s %q, and %s saying %q was wanted.\n"+ + "Reason: the checker decides which damage is offered for which format, so a\n"+ + "refusal has to come from the defect itself. A crash is a fault in the\n"+ + "checker, never a refusal of the file.", c.name, j.Verdict, j.Reason, c.verdict, c.reason) + } + } +} + +// strictPlant is run by the guard below. It loads the checker, puts checks that +// misbehave in place of real ones and asks through the same path --batch +// takes, printing one answer per line. +const strictPlant = `import importlib.util, json, sys +sys.dont_write_bytecode = True +spec = importlib.util.spec_from_file_location("tfg_strict", sys.argv[1]) +strict = importlib.util.module_from_spec(spec) +spec.loader.exec_module(strict) + +def raises(data): + raise ValueError("planted") + +def says_nothing(data): + return None + +def refuses_inside_a_handler(data): + try: + strict.fail("planted refusal") + except Exception: + pass + strict.ok("the refusal was swallowed") + +strict.CHECKS["png"] = raises +strict.CHECKS["gif"] = says_nothing +strict.CHECKS["bmp"] = refuses_inside_a_handler +for kind in ("png", "gif", "bmp"): + print(json.dumps(strict.batch_answer(json.dumps({"format": kind, "path": sys.argv[1]})))) +` + +// TestACheckThatStopsOnAnExceptionIsACrashNotARefusal holds the line the +// witness of a damage stands on. The checker answers CRASH when a check stops +// on an exception or ends without a verdict, and never FAIL. Counted as a +// refusal, a checker that falls over on every damaged file would witness every +// damage (docs/DAMAGE-ETAP-2-2026-11-09.md section 3.3). +// +// The third check refuses inside its own `except Exception` and carries on. A +// verdict has to stop the check anyway, which is why it is raised as a +// SystemExit rather than an ordinary exception. +// +// Planted checks, because the real ones are meant to have no crash left in +// them, so no real file can reach these states. +func TestACheckThatStopsOnAnExceptionIsACrashNotARefusal(t *testing.T) { + python, script := strictProgram(t) + plant := filepath.Join(t.TempDir(), "plant.py") + if err := os.WriteFile(plant, []byte(strictPlant), 0o600); err != nil { + t.Fatal(err) + } + // The interpreter is the one found on PATH, the script is the plant this + // guard just wrote, and its one argument is the checker of this repository. + // nosemgrep: go.lang.security.audit.dangerous-exec-command.dangerous-exec-command + out, err := exec.Command(python, plant, script).CombinedOutput() + if err != nil { + t.Fatalf("the planted checks did not run: %v\n%s", err, out) + } + + want := []struct{ verdict, reason string }{ + {oracle.StrictCrash, "ValueError: planted"}, + {oracle.StrictCrash, "the check returned without a verdict"}, + {oracle.StrictFail, "planted refusal"}, + } + lines := strings.Split(strings.TrimSpace(string(out)), "\n") + if len(lines) != len(want) { + t.Fatalf("%d answer(s) for %d planted checks:\n%s", len(lines), len(want), out) + } + for i, w := range want { + var j oracle.StrictJudgement + if err := json.Unmarshal([]byte(strings.TrimSpace(lines[i])), &j); err != nil { + t.Fatalf("answer %d is not an answer: %v: %s", i+1, err, lines[i]) + } + if j.Verdict != w.verdict || !strings.Contains(j.Reason, w.reason) { + t.Errorf("planted check %d was answered %s %q, and %s saying %q was wanted.\n"+ + "Reason: a check that falls over is a fault in the checker. It must never\n"+ + "read as a refusal, and a refusal must never be swallowed by a check's own\n"+ + "error handling.", i+1, j.Verdict, j.Reason, w.verdict, w.reason) + } + } +} + +// TestTheStructuralCheckerAnswersARequestItCannotActOnWithAnError holds the +// other side of the batch: a request the checker cannot act on is answered +// ERROR, in its place, and every request after it is still answered. +// +// Through StrictBatch for what a Go caller can send (a format with no check, +// settings to a check that takes none, a file that is not there, a path with +// a NUL in it), and through the program itself for lines no Go caller writes +// (a line that is not JSON, JSON nested past what the decoder goes). Before +// 2026-11-10 the last two of the first group and the nested line stopped the +// whole batch. The healthy file between them is the control: a batch that +// answered ERROR to everything would pass the rest. +func TestTheStructuralCheckerAnswersARequestItCannotActOnWithAnError(t *testing.T) { + dir := t.TempDir() + good := filepath.Join(dir, "good.zip") + if err := os.WriteFile(good, writeEncoded(t, "zip", 16384, nil), 0o600); err != nil { + t.Fatal(err) + } + asked := []struct { + req oracle.StrictRequest + answer string + reason string + }{ + {oracle.StrictRequest{Format: "nothing-of-that-name", Path: good}, oracle.StrictError, "there is no check"}, + {oracle.StrictRequest{Format: "zip", Path: good, Settings: map[string]string{"a": "b"}}, oracle.StrictError, "takes no settings"}, + {oracle.StrictRequest{Format: "zip", Path: good}, oracle.StrictOK, ""}, + {oracle.StrictRequest{Format: "zip", Path: filepath.Join(dir, "never-written.zip")}, oracle.StrictError, "cannot be read"}, + {oracle.StrictRequest{Format: "zip", Path: good + "\x00.zip"}, oracle.StrictError, "cannot be read"}, + {oracle.StrictRequest{Format: "zip", Path: good}, oracle.StrictOK, ""}, + } + reqs := make([]oracle.StrictRequest, len(asked)) + for i, a := range asked { + reqs[i] = a.req + } + got, available, err := oracle.StrictBatch(t.Context(), reqs) + if !available { + t.Skip("the structural checker needs python, and there is none on this machine") + } + if err != nil { + t.Fatalf("one request the checker cannot act on stopped the whole batch: %v", err) + } + for i, a := range asked { + if got[i].Verdict != a.answer || !strings.Contains(got[i].Reason, a.reason) { + t.Errorf("request %d (%s %q) was answered %s %q, and %s saying %q was wanted", + i+1, a.req.Format, a.req.Path, got[i].Verdict, got[i].Reason, a.answer, a.reason) + } + } + + python, script := strictProgram(t) + lines := []string{ + "this is not a request", + strings.Repeat("[", 200000) + strings.Repeat("]", 200000), + `{"format": "zip", "path": ` + strconv.Quote(good) + `}`, + } + // The interpreter is the one found on PATH, the script is the checker of + // this repository, and the input is the lines above. + // nosemgrep: go.lang.security.audit.dangerous-exec-command.dangerous-exec-command + cmd := exec.CommandContext(t.Context(), python, script, "--batch") + cmd.Stdin = strings.NewReader(strings.Join(lines, "\n") + "\n") + out, err := cmd.Output() + if err != nil { + t.Fatalf("a line the checker cannot read stopped the whole batch: %v", err) + } + answers := strings.Split(strings.TrimSpace(string(out)), "\n") + if len(answers) != len(lines) { + t.Fatalf("%d line(s) asked and %d answered:\n%s", len(lines), len(answers), out) + } + for i, want := range []string{oracle.StrictError, oracle.StrictError, oracle.StrictOK} { + var j oracle.StrictJudgement + if err := json.Unmarshal([]byte(strings.TrimSpace(answers[i])), &j); err != nil { + t.Fatalf("answer %d is not an answer: %v", i+1, err) + } + if j.Verdict != want { + t.Errorf("line %d was answered %s %q, and %s was wanted", i+1, j.Verdict, j.Reason, want) + } + } +} + +// TestAStoppedCallerStopsTheStructuralChecker holds that the checker runs +// under its caller's context. A guard stopped by its time limit has to stop +// the Python process with it, rather than leave it running under a context +// of its own. +func TestAStoppedCallerStopsTheStructuralChecker(t *testing.T) { + dir := t.TempDir() + good := filepath.Join(dir, "good.zip") + if err := os.WriteFile(good, writeEncoded(t, "zip", 16384, nil), 0o600); err != nil { + t.Fatal(err) + } + reqs := []oracle.StrictRequest{{Format: "zip", Path: good}} + + // The control: the same request under a live context is answered. + if _, available, err := oracle.StrictBatch(t.Context(), reqs); !available { + t.Skip("the structural checker needs python, and there is none on this machine") + } else if err != nil { + t.Fatalf("the request was not answered under a live context: %v", err) + } + + stopped, cancel := context.WithCancel(t.Context()) + cancel() + if got, _, err := oracle.StrictBatch(stopped, reqs); err == nil { + t.Errorf("a stopped caller got %d answer(s) back. The checker ran under a context of its own, "+ + "so a guard that is stopped leaves it running", len(got)) + } +} + +// strictProgram is the interpreter on PATH and the checker of this +// repository, for a guard that runs the program itself. +func strictProgram(t *testing.T) (python, script string) { + t.Helper() + python, err := exec.LookPath("python") + if err != nil { + t.Skip("the structural checker needs python, and there is none on this machine") + } + here, err := os.Getwd() + if err != nil { + t.Fatal(err) + } + script, ok := oracle.StrictScriptUnder(here) + if !ok { + t.Fatalf("the structural checker is not above %s", here) + } + return python, script +} + +// zeroed is the file with its first n bytes set to zero. +func zeroed(b []byte, n int) []byte { + out := append([]byte{}, b...) + clear(out[:n]) + return out +} + +// appended is the file with n bytes of a fixed pattern after its end. +func appended(b []byte, n int) []byte { + out := append([]byte{}, b...) + for i := range n { + out = append(out, byte(i*37+11)) + } + return out +} + +// firstEntry finds the first local file header of an archive, and where its +// name and its data start. +func firstEntry(t *testing.T, b []byte) (name, data int) { + t.Helper() + at := bytes.Index(b, []byte("PK\x03\x04")) + if at < 0 || at+30 > len(b) { + t.Fatal("the archive has no local file header to change, so this case would test nothing") + } + nameLen := int(binary.LittleEndian.Uint16(b[at+26:])) + extraLen := int(binary.LittleEndian.Uint16(b[at+28:])) + if nameLen == 0 || at+30+nameLen+extraLen >= len(b) { + t.Fatal("the first entry has no name or no data, so this case would test nothing") + } + return at + 30, at + 30 + nameLen + extraLen +} + +// renamed is the archive with one letter of the name in its first local +// header changed, so the header and the central directory disagree. +func renamed(t *testing.T, b []byte) []byte { + out := append([]byte{}, b...) + name, _ := firstEntry(t, out) + out[name] ^= 0x01 + return out +} + +// unknownMethod is the package with the compression method of its first +// central directory entry set to one no reader knows. +func unknownMethod(t *testing.T, b []byte) []byte { + out := append([]byte{}, b...) + at := bytes.Index(out, []byte("PK\x01\x02")) + if at < 0 || at+12 > len(out) { + t.Fatal("the package has no central directory entry to change, so this case would test nothing") + } + binary.LittleEndian.PutUint16(out[at+10:], 77) + return out +} + +// garbledOffset is the PDF with the first digit of object 1's offset in its +// cross reference table replaced by a letter. +func garbledOffset(t *testing.T, b []byte) []byte { + out := append([]byte{}, b...) + // On a line of its own, because "startxref" near the end holds the word too. + at := bytes.LastIndex(out, []byte("\nxref\n")) + if at >= 0 { + at++ + } + // Past the keyword, the count line and the free entry of object 0. + for range 3 { + next := bytes.IndexByte(out[max(at, 0):], '\n') + if at < 0 || next < 0 { + t.Fatal("the PDF has no cross reference entry for object 1, so this case would test nothing") + } + at += next + 1 + } + if at >= len(out) || out[at] < '0' || out[at] > '9' { + t.Fatal("the entry for object 1 does not start with a digit, so this case would test nothing") + } + out[at] = 'x' + return out +} + +// unclosedLabel is the page with the end tag of its label heading taken off. +func unclosedLabel(t *testing.T, b []byte) []byte { + at := bytes.Index(b, []byte("")) + if at < 0 { + t.Fatal("the page has no label heading, so this case would test nothing") + } + return append(append([]byte{}, b[:at]...), b[at+len(""):]...) +} + +// flipped is the package with one byte of the first part's data changed, so +// the part no longer matches the checksum its headers carry. +func flipped(t *testing.T, b []byte) []byte { + out := append([]byte{}, b...) + _, data := firstEntry(t, out) + out[data] ^= 0xff + return out +} diff --git a/internal/gui/text/locale/said/en.json b/internal/gui/text/locale/said/en.json index 87ee781e..f576a94c 100644 --- a/internal/gui/text/locale/said/en.json +++ b/internal/gui/text/locale/said/en.json @@ -1004,6 +1004,11 @@ "hash": "sha256-7a1261279b19", "other": "cannot write the instructions to {{.Path}}: {{.Cause}}" }, + "engine.LabelNotGuaranteed": { + "description": "Said by internal/engine - a refusal under the box it is about or at the foot of the form, or a note after a run.", + "hash": "sha256-9d0de3a39db8", + "other": "The label was written before the file was damaged, so the damage may have overwritten it." + }, "engine.MemoryOverrun": { "description": "Said by internal/engine - a refusal under the box it is about or at the foot of the form, or a note after a run. Carries {{.Name}}, {{.Holds}}, {{.Room}}, written the way the program writes them - keep each spelled exactly that way.", "hash": "sha256-168a887a89fa", diff --git a/internal/gui/text/locale/said/pl.json b/internal/gui/text/locale/said/pl.json index 94a194ec..18245add 100644 --- a/internal/gui/text/locale/said/pl.json +++ b/internal/gui/text/locale/said/pl.json @@ -199,6 +199,7 @@ "engine.IDTwice": { "hash": "sha256-724959681477", "other": "nazwa partii {{.ID}} występuje dwa razy" }, "engine.IDTwiceWhy": { "hash": "sha256-ded29dfee98c", "other": "nazwy wskazują partie, więc powtórzenie jest błędem, a nie cichym nadpisaniem" }, "engine.InstructionsNotWritten": { "hash": "sha256-7a1261279b19", "other": "nie można zapisać instrukcji do {{.Path}}: {{.Cause}}" }, + "engine.LabelNotGuaranteed": { "hash": "sha256-9d0de3a39db8", "other": "Etykieta została zapisana, zanim plik uszkodzono, więc uszkodzenie mogło ją nadpisać." }, "engine.MemoryOverrun": { "hash": "sha256-168a887a89fa", "other": "{{.Name}} zajmuje przy zapisie około {{.Holds}}, więcej niż {{.Room}}, które ustawienie pamięci zostawiło zapisywanym plikom, więc został zapisany sam, a przebieg przekroczył ustawienie, dopóki go nie skończył" }, "engine.MemoryUnknown": { "hash": "sha256-25e95b18a15e", "other": "nie udało się odczytać pamięci tej maszyny ({{.Cause}}), więc ten przebieg wziął {{.Limit}}, czyli najwięcej, ile brał każdy przebieg, zanim powstało ustawienie pamięci. Ustaw pamięć sam, żeby wybrać limit" }, "engine.NameAbsolute": { "hash": "sha256-2dc5711eaa7d", "other": "{{.Where}} daje ścieżkę bezwzględną {{.Name}}" }, diff --git a/internal/oracle/strict.py b/internal/oracle/strict.py index 64d54fcd..21ee0eb7 100644 --- a/internal/oracle/strict.py +++ b/internal/oracle/strict.py @@ -11,8 +11,15 @@ complements the tolerant readers rather than replacing them - a file has to pass both. -Usage: python strict.py +Usage: python strict.py [key=value ...] Prints "OK " or "FAIL ". + +Or: python strict.py --batch +Reads one JSON request per line, {"format": ..., "path": ..., "settings": +{...}}, and writes one JSON answer per line, {"path": ..., "verdict": ..., +"reason": ...}, in the order asked. The verdict is OK, FAIL, CRASH (the check +stopped on an exception instead of giving a verdict) or ERROR (a request +this script cannot act on). """ import re @@ -23,14 +30,28 @@ from html.parser import HTMLParser +class Verdict(SystemExit): + """The answer a check gives, raised so that it stops the check where it is. + + It is a SystemExit, so a check's own `except Exception` cannot catch a + verdict given inside it and carry on as if nothing had been said. The word + and the reason travel with it instead of being printed, so one process can + judge many files (--batch) and a check can ask another one in process + (html_picture_verdict) without capturing what it printed. + """ + + def __init__(self, word, reason): + super().__init__(0 if word == "OK" else 1) + self.word = word + self.reason = reason + + def fail(msg): - print("FAIL " + msg) - sys.exit(1) + raise Verdict("FAIL", msg) def ok(msg): - print("OK " + msg) - sys.exit(0) + raise Verdict("OK", msg) def check_png(data): @@ -46,6 +67,8 @@ def check_png(data): body = data[pos + 8:pos + 8 + length] if len(body) != length: fail(f"chunk {kind!r} declares {length} B and the file holds {len(body)}") + if pos + 12 + length > len(data): + fail(f"chunk {kind!r} ends at {len(data)} B, before its 4 B checksum") want = struct.unpack(">I", data[pos + 8 + length:pos + 12 + length])[0] got = zlib.crc32(kind + body) & 0xFFFFFFFF if want != got: @@ -54,6 +77,8 @@ def check_png(data): chunks += 1 pos += 12 + length + if not seen: + fail("the signature is followed by no chunk at all") if seen[0] != b"IHDR": fail("the first chunk is not IHDR") if seen[-1] != b"IEND": @@ -136,6 +161,8 @@ def check_pdf(data): parts = entry.split() if len(parts) < 3 or parts[2] != b"n": continue + if not parts[0].isdigit(): + fail(f"the xref table gives object {i} the offset {parts[0]!r}, which is not a number") obj_at = int(parts[0]) expected = str(i).encode() + b" 0 obj" if data[obj_at:obj_at + len(expected)] != expected: @@ -146,17 +173,66 @@ def check_pdf(data): def check_zip(data): + entries, comment_len = zip_structure(data) + ok(f"{entries} entries, comment {comment_len} B") + + +def zip_structure(data): + """The archive as the specification lays it out, walked from its end. + + The end of central directory record says where the central directory is + and how many entries it holds. Every entry there says where its local + header is, and that has to be a local header carrying the same name, with + its data ending before the central directory starts. Reading the end alone + took an archive whose first bytes were zeroed (measured 2026-11-09): every + record read from the end was intact, and the local header a reader needs + to reach the data was gone. + + Shared with the Office packages, which are ZIP archives first. + Returns the number of entries and the length of the archive comment. + """ end = data.rfind(b"PK\x05\x06") - if end < 0: + if end < 0 or end + 22 > len(data): fail("there is no end of central directory record") comment_len = struct.unpack(" end or data[pos:pos + 4] != b"PK\x01\x02": + fail(f"central directory entry {number} of {entries} should start at {pos} and does not") + packed = struct.unpack(" end: + fail(f"central directory entry {number} of {entries} runs past the end record at {end}") + + if local + 30 > cd_at or data[local:local + 4] != b"PK\x03\x04": + fail(f"the central directory sends {name!r} to offset {local}, " + f"where there is no local file header") + local_name_len, local_extra_len = struct.unpack(" cd_at: + fail(f"the data of {name!r} runs from {local} to {data_end}, " + f"past the start of the central directory at {cd_at}") + + if pos != end: + fail(f"the {entries} central directory entries end at {pos} and the end record is at {end}") + return entries, comment_len OCTET = r"(?:0|[1-9][0-9]{0,2})" @@ -722,7 +798,10 @@ def html_settings(text, shape, settings, nl): if re.match(r"]", body): # The label sits outside the levels, so a reader that drops the deep # ones still shows which file it read. - body = body[body.index("" + nl) + len("" + nl):] + label_end = body.find("" + nl) + if label_end < 0: + fail("the label heading is opened and never closed on its line") + body = body[label_end + len("" + nl):] div_open, div_close = "
" + nl, "
" + nl opened = 0 while body.startswith(div_open, opened * len(div_open)): @@ -834,26 +913,20 @@ def html_pictures(text, settings, nl): def html_picture_verdict(kind, n, raw): - """One picture judged by its own format's check. Those end the process with - their verdict, so each is caught here and only a failure is passed on. - - A check can also stop on bytes it did not expect - check_png unpacks a - checksum that is not there when a chunk runs to the very end - and that is - a verdict about the picture too, so it is given as one rather than as a - traceback (review of #178).""" - import contextlib - import io - - said = io.StringIO() + """One picture judged by its own format's check, and only a failure passed on. + + A check that stops on an exception is not caught here. Until 2026-11-09 it + was, and given as a verdict about the picture (review of #178, a PNG whose + last chunk ran to the very end). That check now refuses such a PNG itself, + and an exception left anywhere is a fault in the checker. It has to show + as one (CRASH in --batch), because a crash counted as a refusal is how a + broken checker passes for a working one.""" try: - with contextlib.redirect_stdout(said): - CHECKS[kind](raw) - except SystemExit as exc: - if exc.code not in (0, None): - fail(f"picture {n} is not a good {kind} file: {said.getvalue().strip()}") + CHECKS[kind](raw) + except Verdict as said: + if said.word != "OK": + fail(f"picture {n} is not a good {kind} file: {said.reason}") return - except Exception as exc: - fail(f"picture {n} is not a good {kind} file: {type(exc).__name__}: {str(exc)[:120]}") fail(f"the {kind} check gave no verdict on picture {n}") @@ -1477,6 +1550,12 @@ def opc_check(data, roots, kind, key): """ import xml.etree.ElementTree as ET + # The ZIP layer first, the same walk a plain archive gets. Python's own + # reader finds the central directory by searching back from the end, so + # bytes after the comment and a local header that is not there both reach + # the checks below unnoticed, or stop them with an exception instead of a + # verdict (measured 2026-11-09). + zip_structure(data) z = opc_open(data) names = [i.filename for i in z.infolist()] @@ -1495,7 +1574,7 @@ def opc_check(data, roots, kind, key): fail("there is no _rels/.rels, so nothing points at the document") try: - types = ET.fromstring(z.read("[Content_Types].xml")) + types = ET.fromstring(opc_part(z, "[Content_Types].xml")) except ET.ParseError as exc: fail("[Content_Types].xml is not well formed: %s" % exc) @@ -1527,7 +1606,7 @@ def opc_check(data, roots, kind, key): if not name.endswith(".xml") and not name.endswith(".rels"): continue try: - root = ET.fromstring(z.read(name)) + root = ET.fromstring(opc_part(z, name)) except ET.ParseError as exc: fail("the part %s is not well formed XML: %s" % (name, exc)) roots_found.append((name, root.tag)) @@ -1537,7 +1616,7 @@ def opc_check(data, roots, kind, key): if not name.endswith(".rels"): continue base = name.rsplit("_rels/", 1)[0] - rels = ET.fromstring(z.read(name)) + rels = ET.fromstring(opc_part(z, name)) for rel in rels.findall(OPC_NS_REL + "Relationship"): if rel.get("TargetMode") == "External": continue @@ -1567,6 +1646,21 @@ def opc_check(data, roots, kind, key): % (kind, len(names), pad, len(z.comment), read)) +def opc_part(z, name): + """The bytes of one part, or a verdict when the archive will not give them. + + A part whose checksum does not match, whose compressed data ends early, or + whose compression method no reader knows is a defect in the package, and + it has to read as one rather than as a stack trace from the ZIP reader. + Office packages are written with deflate or stored entries only. + """ + import zipfile + try: + return z.read(name) + except (zipfile.BadZipFile, zlib.error, EOFError, NotImplementedError) as exc: + fail("the part %s cannot be read from the archive: %s" % (name, exc)) + + def opc_resolve(base, target): """Turns a relationship target into a path inside the package.""" import posixpath @@ -4073,7 +4167,80 @@ def rtf_every(text, settings): # any other one is called. TAKES_SETTINGS = {"csv", "txt", "md", "json", "xml", "html", "geojson", "eml", "rtf"} -if __name__ == "__main__": +def run_check(kind, body, settings): + """Runs one check. It ends by raising its Verdict, or by an exception.""" + if kind in TAKES_SETTINGS: + CHECKS[kind](body, settings) + else: + CHECKS[kind](body) + + +def judge(kind, body, settings): + """One file judged: ("OK" | "FAIL" | "CRASH", what was said). + + CRASH is the check stopping on an exception instead of giving a verdict. + It is kept apart from FAIL on purpose. A guard asking whether a damaged + file is refused must not count the checker falling over as a refusal. + """ + try: + run_check(kind, body, settings) + except Verdict as said: + return said.word, said.reason + except Exception as exc: # noqa: BLE001 - every exception here is a fault in a check, named below + return "CRASH", f"{type(exc).__name__}: {str(exc)[:200]}" + return "CRASH", "the check returned without a verdict" + + +def batch_answer(line): + """The answer to one --batch request line, as a dict.""" + import json + + try: + ask = json.loads(line) + kind, path = ask["format"], ask["path"] + settings = ask.get("settings") or {} + if not isinstance(kind, str) or not isinstance(path, str): + # open() takes a number as a descriptor, which is not a file asked for. + raise TypeError("format and path have to be strings") + except (ValueError, KeyError, TypeError, AttributeError, RecursionError) as exc: + # RecursionError is JSON nested deeper than the decoder goes. One bad + # line is one ERROR, and the requests after it are still answered. + return {"path": "", "verdict": "ERROR", "reason": f"the request {line[:80]!r} is not readable: {exc}"} + answer = {"path": path, "verdict": "ERROR"} + if kind not in CHECKS: + answer["reason"] = f"there is no check for {kind!r}" + elif not isinstance(settings, dict) or not all(isinstance(v, str) for v in settings.values()): + answer["reason"] = "settings have to be an object of strings" + elif settings and kind not in TAKES_SETTINGS: + answer["reason"] = f"the {kind} check takes no settings, and was given {sorted(settings)}" + else: + # The whole file, with no limit, on purpose: every check walks offsets + # and checksums across all of it, so a limit would turn a large healthy + # file into a refusal. The files are ones the caller has just written. + # ValueError is a path holding a NUL, which open() will not take. + try: + with open(path, "rb") as handle: + body = handle.read() + except (OSError, ValueError) as exc: + answer["reason"] = f"the file cannot be read: {exc}" + else: + answer["verdict"], answer["reason"] = judge(kind, body, settings) + return answer + + +def main_batch(): + """Many files in one process, so a guard judging every format against every + damage does not start Python a few hundred times.""" + import json + + for raw in sys.stdin.buffer: + line = raw.decode("utf-8", errors="replace").strip() + if line: + sys.stdout.write(json.dumps(batch_answer(line)) + "\n") + sys.stdout.flush() + + +def main_one(): if len(sys.argv) < 3 or sys.argv[1] not in CHECKS: print("FAIL usage: strict.py <" + "|".join(CHECKS) + "> [key=value ...]") sys.exit(1) @@ -4090,7 +4257,17 @@ def rtf_every(text, settings): sys.exit(1) with open(sys.argv[2], "rb") as handle: body = handle.read() - if kind in TAKES_SETTINGS: - CHECKS[kind](body, extra) + # An exception is left to end the process with its traceback, which is + # what a caller reading only OK and FAIL reports as a checker fault. + try: + run_check(kind, body, extra) + except Verdict as said: + print(said.word + " " + said.reason) + sys.exit(said.code) + + +if __name__ == "__main__": + if sys.argv[1:] == ["--batch"]: + main_batch() else: - CHECKS[kind](body) + main_one() diff --git a/internal/oracle/strictbatch.go b/internal/oracle/strictbatch.go new file mode 100644 index 00000000..9c94a477 --- /dev/null +++ b/internal/oracle/strictbatch.go @@ -0,0 +1,123 @@ +package oracle + +import ( + "bufio" + "context" + "encoding/json" + "fmt" + "os/exec" + "strings" + "time" +) + +// StrictRequest is one file handed to StrictBatch. +type StrictRequest struct { + // Format is the ID of the check to run, as for Strict. + Format string `json:"format"` + // Path is the file to judge. + Path string `json:"path"` + // Settings are the key=value words Strict takes after the path. + Settings map[string]string `json:"settings,omitempty"` +} + +// StrictJudgement is the checker's answer about one file. +type StrictJudgement struct { + // Path is the file the answer is about, as it was asked. + Path string `json:"path"` + // Verdict is one of the Strict* verdict constants. + Verdict string `json:"verdict"` + // Reason is what the check said. + Reason string `json:"reason"` +} + +// The four answers the checker gives in batch mode. +const ( + // StrictOK is a file the check accepts. + StrictOK = "OK" + // StrictFail is a file the check refuses, for the reason given. + StrictFail = "FAIL" + // StrictCrash is the check stopping on an exception instead of giving + // a verdict. It is a fault in the checker, never a refusal of the file: + // counted as one, a broken checker passes for a working one + // (docs/DAMAGE-ETAP-2-2026-11-09.md section 3.3). + StrictCrash = "CRASH" + // StrictError is a request the checker could not act on. + StrictError = "ERROR" +) + +// StrictBatch judges many files in one run of the structural checker. +// +// Strict starts Python once per file. A guard that judges every format +// against every damage asks a few hundred times, so this hands the whole list +// to one process and reads one answer per file back, in the order asked. +// +// The second result is false when there is no Python or no checker to run, +// which the caller reports as a skip rather than passing quietly. An error +// means the run itself went wrong - the process failed, or the answers do not +// match the questions one to one - and no verdict in it can be trusted. +// +// The context is the caller's, so a guard that is stopped stops the checker +// with it. A time limit of its own sits under it. +func StrictBatch(ctx context.Context, reqs []StrictRequest) ([]StrictJudgement, bool, error) { + python, ok := inPath("python")() + if !ok { + return nil, false, nil + } + script, ok := strictScriptPath() + if !ok { + return nil, false, nil + } + + var in strings.Builder + for _, r := range reqs { + line, err := json.Marshal(r) + if err != nil { + return nil, true, fmt.Errorf("writing the request for %s: %w", r.Path, err) + } + in.Write(line) + in.WriteByte('\n') + } + + // A minute for the process, and a second for every file on top. + limit := time.Minute + time.Duration(len(reqs))*time.Second + ctx, cancel := context.WithTimeout(ctx, limit) + defer cancel() + + //nolint:gosec // our own script, run against files this tool wrote + // nosemgrep: go.lang.security.audit.dangerous-exec-command.dangerous-exec-command + cmd := exec.CommandContext(ctx, python, script, "--batch") + cmd.Stdin = strings.NewReader(in.String()) + var out, errOut strings.Builder + cmd.Stdout = &out + cmd.Stderr = &errOut + if err := cmd.Run(); err != nil { + return nil, true, fmt.Errorf("the structural checker stopped: %w: %s", err, strings.TrimSpace(errOut.String())) + } + + got := make([]StrictJudgement, 0, len(reqs)) + scan := bufio.NewScanner(strings.NewReader(out.String())) + scan.Buffer(make([]byte, 64*1024), 1024*1024) + for scan.Scan() { + line := strings.TrimSpace(scan.Text()) + if line == "" { + continue + } + var j StrictJudgement + if err := json.Unmarshal([]byte(line), &j); err != nil { + return nil, true, fmt.Errorf("the structural checker answered %q, which is not an answer: %w", line, err) + } + got = append(got, j) + } + if err := scan.Err(); err != nil { + return nil, true, fmt.Errorf("reading the structural checker's answers: %w", err) + } + if len(got) != len(reqs) { + return nil, true, fmt.Errorf("the structural checker's answers do not match its questions: asked %d, answered %d", len(reqs), len(got)) + } + for i := range got { + if got[i].Path != reqs[i].Path { + return nil, true, fmt.Errorf("answer %d is about %q and the question was about %q", i+1, got[i].Path, reqs[i].Path) + } + } + return got, true, nil +}