feat(preserve): compiled floor under every removal path, plus the register declaration - #104
Merged
Conversation
…ister declaration The class-1 corpus a running factory carries — its worktree and run state, the pack's customization and output trees, wave checkouts, and every git ref — had no declared protection and no enforcement that did not depend on a containment predicate being correct. Per drbothen/vsdd-factory#413 that corpus doubles as the data model for the future service, so it is a product contract rather than housekeeping, and it should be guarded before a public release rather than after. Two halves, per aae-orc-d3nq.42. The register (sideshow-packs/registry/<pack>-pack-support.yaml) is authoritative and carries the full three-class taxonomy for vsdd-factory and bmad. This adds the floor beneath it: the subset that holds on a machine with the binary and a ledger but no register on hand, which is every consumer machine. internal/preserve matches class 1 by path COMPONENT rather than prefix, so protected state at any depth refuses, and by SHAPE rather than pack name, so the _<pack>-custom / _<pack>-output convention protects a pack sideshow has never seen. That last property is the point: a floor that only covered packs it recognized would fail exactly where it matters. The floor is checked independently of the containment predicate in RemoveRepoArtifacts rather than folded into it. Containment asks whether a path is inside the area sideshow manages; the floor asks whether it is something nobody may delete. Two independent checks that must both pass means a bug in the first is survivable while the second holds. It also covers RenderBoundVariant, whose os.RemoveAll takes destDir as a parameter and had no containment above it at all — the one place a miswired caller could have recursed through anything it pointed at. Git-side protection is verb behavior, not a path list: a branch or a ref is not something a containment predicate can see. Sideshow's git usage is already read-only by construction (measured: rev-parse and status only), so the change is a guard test that keeps it that way rather than a fix. Scope note: the ticket names uninstall as a target and sideshow has no uninstall verb. The removal paths that actually run inside a user's repo are disable's ledger replay and sync reconciliation, and those are what the floor covers. Tests: 22 table cases on the predicate including near-misses that must NOT match; typed-error and set-preflight guards; the floor firing in the real removal path against ledger rows shaped to slip past containment; and the acceptance criterion — a mid-wave factory is bit-identical across enable and disable, run with the running-factory guard overridden, since the case worth pinning is the one past policy. The two removal-path guards were run with the floor removed and fail there. gofmt, go vet and golangci-lint clean; full suite passes. Refs: aae-orc-d3nq.42
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.
The class-1 corpus a running factory carries — its worktree and run state, the pack's customization and output trees, wave checkouts, and every git ref — had no declared protection and no enforcement that did not depend on a containment predicate being correct. Per drbothen/vsdd-factory#413 that corpus doubles as the data model for the future service, so it is a product contract rather than housekeeping, and it is worth guarding before a public release rather than after.
Last of the code items on the vsdd-factory release gate.
Two halves
The register (
sideshow-packs/registry/<pack>-pack-support.yaml) is authoritative and now carries the full three-class taxonomy for both vsdd-factory and bmad. bmad's class 1 is just the custom and output trees; declared anyway rather than omitted, because an absent section reads as "not yet considered" and the two contracts should be comparable at a glance.The floor (
internal/preserve) is what holds beneath it, on a machine with the binary and a ledger but no register on hand — which is every consumer machine.Design
Component matching, not prefix. Protected state at any depth refuses, so a nested checkout or a worktree several levels down is covered.
Shape matching, not pack name. The
_<pack>-custom/_<pack>-outputconvention protects a pack sideshow has never seen. A floor that only covered packs it recognized would fail exactly where it matters most.Independent of containment, deliberately.
RemoveRepoArtifactsalready confines removal to the harness root. Containment asks "is this inside the area I manage"; the floor asks "is this something nobody may delete". Two independent checks that must both pass means a bug in the first is survivable while the second holds. The hostile tests use ledger rows shaped to resolve under.claude/, so they slip past containment and only the floor stops them.RenderBoundVariantgets it too. Itsos.RemoveAlltakesdestDiras a parameter and had no containment above it at all. The bound variant is class 2 and clearing it is correct, but that was the one place a miswired caller could have recursed through whatever it pointed at.Git protection is verb behavior. A branch or a ref is not something a containment predicate can see. Sideshow's git usage is already read-only by construction (measured:
rev-parseandstatusonly), so this ships a guard test that keeps it that way rather than a fix.Scope note
The ticket names
uninstallas a target. Sideshow has nouninstallverb. The removal paths that actually run inside a user's repo aredisable's ledger replay and sync reconciliation, and those are what the floor covers. Flagged here rather than silently reinterpreted.Tests
vsdd-factory-customwithout the underscore,_vsdd-factorywithout the suffix,my.factory.noteswherefactoryis a substring rather than a component).The two removal-path guards were run with the floor removed and fail there.
gofmt,go vetandgolangci-lintclean; full suite passes.Cost of merge
Additive. No existing behavior changes for any path that was not already refusing, and the existing containment and factory-guard tests are untouched and still green.
Refs:
aae-orc-d3nq.42