Record parseSlots exemption and fix stale package doc comments

Documents why parseSlots skips repeat-key rejection, corrects two drifted doc
lines (naming-rule count, Malformed-after-header header validity), and records
two forward obligations for import_plan in CLAUDE.md.
This commit is contained in:
2026-08-02 09:02:23 -04:00
parent 3909b1072c
commit 181b4f2edb
4 changed files with 35 additions and 10 deletions
+20 -4
View File
@@ -25,9 +25,10 @@ landing after the format.
header (so the refusal can name the writer's semver and version) and nothing
else — no manifest, no layout, no half-success. The fields through the writer
semver are FROZEN for all future versions to keep that refusal producible.
- **Every name and path in the format is validated on encode AND decode**,
because a package can arrive from anywhere. Three rules, all in
`package_format`, whose doc comments are the itemized authority:
- **Every name and path in the format is validated on encode AND decode, to
the extent stated below**, because a package can arrive from anywhere.
Three rules, all in `package_format`, whose doc comments are the itemized
authority:
- `isValidEntryName` — a payload's name is a bare file name (no separators,
no `..` component, no drive/UNC/rooted form, no control bytes, no
Windows-reserved character, no trailing dot/space, no DOS device name,
@@ -40,7 +41,13 @@ landing after the format.
design, and is the one field here that can express one. It refuses a `..`
component and every absolute form; `BankModel::add` checks only the latter,
so traversal would otherwise reach a future `import_plan` inside a record
the format vouched for.
the format vouched for. **Scope is traversal and absolute-form only** — no
UTF-8 well-formedness check (unlike `isValidEntryName`), no device-name
check, no case-fold dedup on `relativePath` (unlike `sameEntryName` on the
entry name). Correct for what this field is — a *record* field, not a
filesystem destination; `BankModel::add` owns the rest. Forward contract
for `import_plan`: **the destination file is derived from the entry name,
never from `relativePath`.**
- **Framing only, never a payload.** `bank_package` produces header bytes and
an ordered `{name, offset, length}` layout; it never holds, copies, or hashes
an entry's audio. `decodePackage` proves prefix + payload lengths equal the
@@ -126,6 +133,15 @@ landing after the format.
collision class as the ASCII case fold, which `sameEntryName` does catch. A
table-free fix does not exist, and restricting names to ASCII would be
genuinely over-strict for non-English users. Left open knowingly.
- **`duplicateName` is O(n²) over `entries` on the decode path** — pre-existing
shape (the double loop is unchanged since `af35fc5`; only the comparator
changed). Under the `kMaxManifestBytes` cap (64 MB) a minimal entry is
~100 bytes, so a hostile package can declare ~670k entries — ~2×10¹¹ pair
comparisons, a multi-minute hang on import. It signals an error rather than
UB, so the hostile-input invariant above still holds, but it sits against
this module's "a forged header cannot demand gigabytes" posture. Forward
obligation for `import_plan`: fold this into a sorted vector or hash set
when that track lands; not changed here.
- **Cross-module contract with `src/shell/package`:** a genuinely zero-length
entry cannot round-trip through the filesystem seam there (`appendPayload`
refuses an empty payload — an empty buffer signals an upstream read failure,
+4 -2
View File
@@ -40,8 +40,10 @@ struct EncodedPackage {
std::optional<EncodedPackage> encodePackage(const PackageManifest& m);
// The read side's product. header is meaningful for Readable and TooNew (a
// refusal must still name the writer); manifest, layout, and prefixSize only
// for Readable — TooNew produces NO manifest, so a refused decode cannot
// refusal must still name the writer), and on any Malformed reached after the
// header parsed (a corrupt manifest at this build's own version carries the
// real pair, not the 0/0 unparsed default); manifest, layout, and prefixSize
// only for Readable — TooNew produces NO manifest, so a refused decode cannot
// half-succeed.
struct DecodedPackage {
PackageReadability status = PackageReadability::Malformed;
+3 -3
View File
@@ -1,8 +1,8 @@
#pragma once
// package_format — the RSBK bank-package contract: magic, the version ladder,
// the readability classification, and the entry-name rule. Pure: standard
// library only. The framing codec that acts on this contract is bank_package;
// the manifest grammar is package_manifest.
// the readability classification, and the three naming rules below. Pure:
// standard library only. The framing codec that acts on this contract is
// bank_package; the manifest grammar is package_manifest.
#include <cstdint>
#include <string>
+8 -1
View File
@@ -84,7 +84,14 @@ std::optional<std::string> serializeManifest(const PackageManifest& m) {
namespace {
// Mirrors bank_book_json's private slots parser: [{id, slot}, ...] pairs handed
// to SlotMap::fromEntries, which owns the defensive repair rules.
// to SlotMap::fromEntries, which owns the defensive repair rules. Deliberately
// does NOT reject a repeated "id"/"slot" key the way the root and entry parsers
// below reject theirs — this grammar belongs to core/model's bank_book_json, and
// diverging here would give one wire shape two behaviours in two files. The
// stakes differ too: a repeated "name" decides which file an entry lands on,
// while a repeated "id" here still feeds SlotMap::fromEntries's deterministic
// first-wins/never-double-occupy repair, so no ambiguity survives. Do not
// "finish" the repeat-key rejection here to match the parsers below.
bool parseSlots(json::Reader& r, model::SlotMap& out) {
std::vector<std::pair<std::string, int>> pairs;
if (!r.consume('[')) return false;