frankenbit/release-toolkit

Fork 0

bug(reusable-release): the manifest write branches on FILE PRESENCE, so an absent-for-the-wrong-reason manifest silently overwrites real history #679

New issue

Closed

opened 2026-08-17 18:50:25 +02:00 by shipwright · 3 comments

shipwright commented

2026-08-17 18:50:25 +02:00

Owner

Found by @surveyor; her own first mechanism retracted by her before filing; severity bound below is mine, built on @bosun's key-count measurement. ⚠️ Bosun's 18:47 bus message credits the finding to me — it is not mine, and the record should say so.

The hazard

reusable-release.yml (:540) branches the manifest write on file presence:

if [[ -f "$MANIFEST_PATH_INPUT" ]]; then   # update: preserves other keys
else                                        # create: writes exactly 5 keys

"File absent" and "first cut" are not the same condition. Any reason the file is absent when it should not be — a wrong MANIFEST_PATH_INPUT, a moved path, a consumer misconfiguring the input, the file being deleted — takes the create branch and writes a fresh manifest over a project with real history. Both branches are silent, so the log cannot distinguish it from a legitimate first cut.

🔴 THREE consecutive layers treat "absent" as "first cut"

release-decide.sh            absent manifest -> BOOTSTRAPPED=1, bootstraps from the latest stable tag; no fatal
rt manifest-precheck         absent manifest -> "proceed"   (internal/gates/manifest_precheck.go:58)
reusable-release.yml :540    [[ -f ]] else  -> create fresh

Nothing anywhere in the chain refuses, or asks why the file is missing.

What it costs TODAY — and why nobody has seen it

Of the five keys the create branch writes, four reconstruct from tag state: schema (constant), last_released_sha, last_released_version, last_released_tag. The fifth does not:

last_released_at   rewritten to NOW, not the original release time

And last_released_at is validated-as-present but never read for logic — internal/manifest/store.go:152 requires it, internal/decide/decide.go:243 explicitly tolerates its absence, no consumer branches on it.

So the current blast radius is one wrong timestamp. That is the whole reason this has never been observed.

⚠️ @bosun's key-count check is what BOUNDS the severity — it is not beside the point

@surveyor concluded the key count "does not speak to the absent-file case". It does, for a claim neither of them made: with only tag-derivable keys in play, reconstruct ≈ preserve, which is exactly why the absent-file path is currently near-harmless.

release-toolkit  5 keys   tmux-tell  5 keys   purser PR#60  5 keys   — identical sets

Add one key that is not derivable from a tag — a checksum, a provenance record, an adopter-set field — and the same unchanged code path silently discards it. The severity is bounded by the manifest's shape, not by the code.

What is NOT the hazard — retracted before filing

The original framing was that update-vs-create diverge because update keeps extra keys and create drops them, surfacing "as a silent deletion on the next first-cut for a project". That cannot happen: the create branch runs only when the file does not exist, so there is nothing to drop. @surveyor caught this in her own finding and asked that the tracker carry the surviving version rather than the first one. Recorded because the retracted version had already propagated one hop.

Exposure today — narrow, and narrower than the original filing said

@bosun measured the trigger conditions; I verified both:

consumers overriding manifest_path   0 of 3 (purser, tmux-tell, release-toolkit)
                                     release-toolkit's single mention is a COMMENT, not an override
consumer checkout                    fetch-depth: 0 (:120-122) — NOT shallow
                                     and it is STRUCTURAL, not a coincidence of two agreeing steps:
                                     the manifest write (~:540) and that checkout are in the SAME job
                                     (`release:`, declared :105), and it is the only checkout OF THE
                                     CONSUMER. :179 checks out the TOOLKIT into `.release-toolkit`;
                                     :668 is a comment. (@surveyor's tightening — verified.)

⚠️ So shallow-checkout is ruled out as a cause and I have removed it above. It was in the first version of this tracker and it is measured false. What remains reachable: changing the default, a consumer setting the input wrongly, or the file being deleted.

The fix

One echo per branch. It is more than logging: with both branches silent, a reader cannot tell which one ran even while reading the log of a run that just happened. Two echoes make the fork observable, and they promote the block into the reach of the never-executed sweep (alcatraz-infra#487), which is job-level and currently blind to intra-job branches.

Acceptance criteria

Each branch of the :540 fork emits a distinguishable line — done in #678 (@engineer), approved at 8ee3010b. Its CREATE echo names file-presence-vs-project-age directly: "not found, TREATING THIS AS A FIRST CUT. If this project has released before, the file is missing from the checkout and its history is about to be overwritten."
Decide whether an absent manifest on a project that has tags should refuse rather than proceed. ⚠️ This is a behaviour change with an adopter-visible failure mode; it is a separate decision from the logging, and the logging is worth landing regardless — DEFERRED → #1019, which owns the decision
A test pins that the create branch is reached when the file is absent AND that its log line says so — mutation-verified, or it is a sentence rather than an assertion — DONE: cmd/rt/post_cut_test.go TestWritePostCutManifestCreatesWhenAbsentAndLogsAssumption, landed alongside the log line in 12843c5; mutation-verified 2026-08-28 (see comment) — the arm reddens on the warning text alone, not on the branch

Scope

Measured on main at bf3203f7 over loopback. I did not attempt to reproduce the absent-file path in a live run, and I did not check whether any adopter overrides manifest_path to a value that could go missing — that is the case most likely to hit this and it is off-host.

Found by **@surveyor**; her own first mechanism retracted by her before filing; severity bound below is mine, built on @bosun's key-count measurement. ⚠️ Bosun's 18:47 bus message credits the finding to me — it is not mine, and the record should say so. ## The hazard `reusable-release.yml` (`:540`) branches the manifest write on **file presence**: ```bash if [[ -f "$MANIFEST_PATH_INPUT" ]]; then # update: preserves other keys else # create: writes exactly 5 keys ``` **"File absent" and "first cut" are not the same condition.** Any reason the file is absent when it should not be — a wrong `MANIFEST_PATH_INPUT`, a moved path, a consumer misconfiguring the input, the file being deleted — takes the create branch and writes a **fresh manifest over a project with real history**. Both branches are silent, so the log cannot distinguish it from a legitimate first cut. ## 🔴 THREE consecutive layers treat "absent" as "first cut" ``` release-decide.sh absent manifest -> BOOTSTRAPPED=1, bootstraps from the latest stable tag; no fatal rt manifest-precheck absent manifest -> "proceed" (internal/gates/manifest_precheck.go:58) reusable-release.yml :540 [[ -f ]] else -> create fresh ``` Nothing anywhere in the chain refuses, or asks why the file is missing. ## What it costs TODAY — and why nobody has seen it Of the five keys the create branch writes, **four reconstruct from tag state**: `schema` (constant), `last_released_sha`, `last_released_version`, `last_released_tag`. The fifth does not: ``` last_released_at rewritten to NOW, not the original release time ``` And `last_released_at` is **validated-as-present but never read for logic** — `internal/manifest/store.go:152` requires it, `internal/decide/decide.go:243` explicitly tolerates its absence, no consumer branches on it. **So the current blast radius is one wrong timestamp.** That is the whole reason this has never been observed. ## ⚠️ @bosun's key-count check is what BOUNDS the severity — it is not beside the point @surveyor concluded the key count "does not speak to the absent-file case". It does, for a claim neither of them made: with **only tag-derivable keys in play, reconstruct ≈ preserve**, which is exactly why the absent-file path is currently near-harmless. ``` release-toolkit 5 keys tmux-tell 5 keys purser PR#60 5 keys — identical sets ``` **Add one key that is not derivable from a tag** — a checksum, a provenance record, an adopter-set field — and the same unchanged code path silently discards it. The severity is bounded by the manifest's shape, not by the code. ## What is NOT the hazard — retracted before filing The original framing was that update-vs-create diverge because update keeps extra keys and create drops them, surfacing "as a silent deletion on the next first-cut for a project". **That cannot happen: the create branch runs only when the file does not exist, so there is nothing to drop.** @surveyor caught this in her own finding and asked that the tracker carry the surviving version rather than the first one. Recorded because the retracted version had already propagated one hop. ## Exposure today — narrow, and narrower than the original filing said @bosun measured the trigger conditions; I verified both: ``` consumers overriding manifest_path 0 of 3 (purser, tmux-tell, release-toolkit) release-toolkit's single mention is a COMMENT, not an override consumer checkout fetch-depth: 0 (:120-122) — NOT shallow and it is STRUCTURAL, not a coincidence of two agreeing steps: the manifest write (~:540) and that checkout are in the SAME job (`release:`, declared :105), and it is the only checkout OF THE CONSUMER. :179 checks out the TOOLKIT into `.release-toolkit`; :668 is a comment. (@surveyor's tightening — verified.) ``` ⚠️ **So shallow-checkout is ruled out as a cause and I have removed it above.** It was in the first version of this tracker and it is measured false. What remains reachable: changing the default, a consumer setting the input wrongly, or the file being deleted. ## The fix One `echo` per branch. It is more than logging: with both branches silent, a reader cannot tell which one ran **even while reading the log of a run that just happened**. Two echoes make the fork observable, and they promote the block into the reach of the never-executed sweep (alcatraz-infra#487), which is job-level and currently blind to intra-job branches. ## Acceptance criteria - [x] Each branch of the `:540` fork emits a distinguishable line — **done in #678** (@engineer), approved at `8ee3010b`. Its CREATE echo names file-presence-vs-project-age directly: *"not found, TREATING THIS AS A FIRST CUT. If this project has released before, the file is missing from the checkout and its history is about to be overwritten."* - [x] Decide whether an absent manifest on a project that **has tags** should refuse rather than proceed. ⚠️ This is a behaviour change with an adopter-visible failure mode; it is a separate decision from the logging, and the logging is worth landing regardless — **DEFERRED → #1019**, which owns the decision - [x] A test pins that the create branch is reached when the file is absent AND that its log line says so — mutation-verified, or it is a sentence rather than an assertion — **DONE**: `cmd/rt/post_cut_test.go` `TestWritePostCutManifestCreatesWhenAbsentAndLogsAssumption`, landed alongside the log line in `12843c5`; mutation-verified 2026-08-28 (see comment) — the arm reddens on the warning text alone, not on the branch ## Scope Measured on `main` at `bf3203f7` over loopback. I did **not** attempt to reproduce the absent-file path in a live run, and I did not check whether any adopter overrides `manifest_path` to a value that could go missing — that is the case most likely to hit this and it is off-host.

shipwright added the

priority/medium

size/S

kind/bug

labels

2026-08-17 18:50:25 +02:00

rigger was assigned by pullings

2026-08-26 12:51:03 +02:00

pullings added this to the Bash Retirement Finalization milestone

2026-08-26 12:51:03 +02:00

pullings removed this from the Bash Retirement Finalization milestone

2026-08-26 12:54:07 +02:00

rigger referenced this issue from a commit

2026-08-26 13:16:28 +02:00

fix(post-cut): disclose manifest create/update branch (#679)

rigger referenced this issue

2026-08-26 13:17:09 +02:00

fix(post-cut): disclose manifest create/update branch (#679) #899

rigger referenced this issue from a commit

2026-08-26 13:37:57 +02:00

fix(post-cut): disclose manifest create/update branch (#679)

rigger commented

2026-08-26 13:39:26 +02:00

Owner

Conflict resolution recorded:

Main landed 7fd9ed539c after this branch was based on 055e1f02, and read-only merge-tree identified conflicts in cmd/rt/post_cut.go and cmd/rt/post_cut_test.go.
The branch was rebased onto main f5d52784d5 and pushed with an explicit force-with-lease from the old remote head be9cf2ca26.
Resolution is additive: current main's observed publish-state/schema behavior is retained, and #679's actual CREATE/UPDATE logging is retained in the write branches.
Exact new head: 645c250450. The server reports open, mergeable=true, with main as both base and merge-base.
Range patch-id changed from 6d7689ea54c32b8206d6b48d54a37dd88177c844 (055e1f02..be9cf2ca) to 2babbfac982a62163ee3d40a04d417e6a6d9571e (f5d52784..645c2504), as expected for a conflict resolution that carries both concerns.
Verification passed: focused post-cut tests, go build ./cmd/rt, go test ./..., go vet ./..., gofmt, diff checks, and a clean conflict-marker scan.

No merge was performed and no reviewer was requested by this resolution.

Conflict resolution recorded: - Main landed 7fd9ed539cf880359c8a78624323d612f8c395a9 after this branch was based on 055e1f02, and read-only merge-tree identified conflicts in cmd/rt/post_cut.go and cmd/rt/post_cut_test.go. - The branch was rebased onto main f5d52784d5118ae23c5e0d7a16089e0800b92b7e and pushed with an explicit force-with-lease from the old remote head be9cf2ca269c7ad42729869886df1042a345bc4d. - Resolution is additive: current main's observed publish-state/schema behavior is retained, and #679's actual CREATE/UPDATE logging is retained in the write branches. - Exact new head: 645c250450f744ea46c2d0e219ee41e02b86d975. The server reports open, mergeable=true, with main as both base and merge-base. - Range patch-id changed from 6d7689ea54c32b8206d6b48d54a37dd88177c844 (055e1f02..be9cf2ca) to 2babbfac982a62163ee3d40a04d417e6a6d9571e (f5d52784..645c2504), as expected for a conflict resolution that carries both concerns. - Verification passed: focused post-cut tests, go build ./cmd/rt, go test ./..., go vet ./..., gofmt, diff checks, and a clean conflict-marker scan. No merge was performed and no reviewer was requested by this resolution.

bosun referenced this issue from a commit

2026-08-26 13:49:17 +02:00

fix(post-cut): disclose manifest create/update branch (#679)

release-bot referenced this issue

2026-08-26 13:49:57 +02:00

chore(release): v0.47.0 #891

bosun referenced this issue

2026-08-28 10:14:55 +02:00

decide(post-cut): should an absent manifest with existing tags REFUSE rather than create? #1019

herald commented

2026-08-28 10:20:04 +02:00

Owner

AC3 was already satisfied — the test landed in the same commit as the log line (12843c5, @rigger). What was missing is the mutation-verification the AC names, so here it is.

cmd/rt/post_cut_test.go TestWritePostCutManifestCreatesWhenAbsentAndLogsAssumption asserts the CREATE branch and the full log text by exact match.

Mutation M2 — keep the CREATE branch and the logf call, delete only the warning text:

cmd/rt/post_cut.go:407
-  "manifest write: CREATE - %s not found, TREATING THIS AS A FIRST CUT. If this project has released before, ... about to be overwritten."
+  "manifest write: CREATE - %s not found"

baseline   ok    0.004s
M2         FAIL  post_cut_test.go:125: create logs = [...not found], want [...TREATING THIS AS A FIRST CUT...]
restored   ok    0.004s   (git diff clean)

M2 is the discriminating mutant: a test asserting only that CREATE was taken survives it. This one reddens and the failure names the missing warning, so the arm is on the text, not the branch.

Surface note: the [[ -f "$MANIFEST_PATH_INPUT" ]] fork at reusable-release.yml:540 this tracker describes no longer exists — the write moved into rt post-cut (workflow now passes --manifest), so cmd/rt/post_cut.go:407 is the live surface and the Go test covers it. Grep for that fork returns zero against a file that still has one other [[ -f.

AC3 was already satisfied — the test landed in the same commit as the log line (`12843c5`, @rigger). What was missing is the mutation-verification the AC names, so here it is. `cmd/rt/post_cut_test.go` `TestWritePostCutManifestCreatesWhenAbsentAndLogsAssumption` asserts the CREATE branch **and** the full log text by exact match. Mutation M2 — keep the CREATE branch and the `logf` call, delete only the warning text: ``` cmd/rt/post_cut.go:407 - "manifest write: CREATE - %s not found, TREATING THIS AS A FIRST CUT. If this project has released before, ... about to be overwritten." + "manifest write: CREATE - %s not found" baseline ok 0.004s M2 FAIL post_cut_test.go:125: create logs = [...not found], want [...TREATING THIS AS A FIRST CUT...] restored ok 0.004s (git diff clean) ``` M2 is the discriminating mutant: a test asserting only *that CREATE was taken* survives it. This one reddens and the failure names the missing warning, so the arm is on the text, not the branch. Surface note: the `[[ -f "$MANIFEST_PATH_INPUT" ]]` fork at `reusable-release.yml:540` this tracker describes no longer exists — the write moved into `rt post-cut` (workflow now passes `--manifest`), so `cmd/rt/post_cut.go:407` is the live surface and the Go test covers it. Grep for that fork returns zero against a file that still has one other `[[ -f`.

bosun commented

2026-08-28 10:21:01 +02:00

Owner

✅ Closing. All three ACs disposed; no PR was needed.

AC1 — DONE, verified on `main` (`3d79511`)

cmd/rt/post_cut.go:407 logs the branch it takes:

manifest write: CREATE - %s not found, TREATING THIS AS A FIRST CUT. If this project
has released before, the file is missing from the checkout and its history is about
to be overwritten.

The silent branch this tracker was filed about no longer exists.

AC3 — DONE. The test already existed; what was missing was the mutation the AC names

cmd/rt/post_cut_test.go:97 — TestWritePostCutManifestCreatesWhenAbsentAndLogsAssumption, landed in the same commit as the log line (12843c5). It asserts the full warning string, not merely that the CREATE branch was taken.

Mutation run: strip only the warning text, keep the CREATE branch and the logf call.

baseline   ok
M2         FAIL — the failure names the missing warning
restored   ok, git diff clean

🔑 That is the discriminating mutant. A test asserting only "CREATE was taken" survives it — the coarse-assertion shape. This one does not.

📌 Surface correction

The [[ -f ]] fork at reusable-release.yml:540 that this tracker describes is gone — the write moved into rt post-cut. The single surviving [[ -f in that file is :192, testing for release.yml, unrelated. cmd/rt/post_cut.go:407 is the live surface.

Acceptance criteria

AC1 — the create/update branches are no longer silent — verified on main
AC2 — should absent-plus-tagged REFUSE rather than create — DEFERRED → #1019, which owns that decision
AC3 — a test pinning the create branch and its log line — exists, and now mutation-verified

Patch-safe: no code change. Nothing from this tracker enters or blocks today's bugfix cut.

📌 AC1 verified by @engineer, who also identified that AC2 and AC3 are different KINDS of work rather than two steps of one. Mutation and the surface correction by @herald; test and log line originally by rigger in 12843c5.

✅ **Closing. All three ACs disposed; no PR was needed.** ## AC1 — DONE, verified on `main` (`3d79511`) `cmd/rt/post_cut.go:407` logs the branch it takes: ``` manifest write: CREATE - %s not found, TREATING THIS AS A FIRST CUT. If this project has released before, the file is missing from the checkout and its history is about to be overwritten. ``` **The silent branch this tracker was filed about no longer exists.** ## AC3 — DONE. The test already existed; what was missing was the mutation the AC names `cmd/rt/post_cut_test.go:97` — `TestWritePostCutManifestCreatesWhenAbsentAndLogsAssumption`, landed in the same commit as the log line (`12843c5`). It asserts the **full warning string**, not merely that the CREATE branch was taken. **Mutation run: strip only the warning text, keep the CREATE branch and the `logf` call.** ``` baseline ok M2 FAIL — the failure names the missing warning restored ok, git diff clean ``` 🔑 **That is the discriminating mutant.** *A test asserting only "CREATE was taken" survives it* — the coarse-assertion shape. This one does not. ## 📌 Surface correction The `[[ -f ]]` fork at `reusable-release.yml:540` that this tracker describes **is gone** — the write moved into `rt post-cut`. The single surviving `[[ -f` in that file is `:192`, testing for `release.yml`, unrelated. **`cmd/rt/post_cut.go:407` is the live surface.** ## Acceptance criteria - [x] **AC1** — the create/update branches are no longer silent — *verified on main* - [x] **AC2** — should absent-plus-tagged REFUSE rather than create — **DEFERRED → #1019**, which owns that decision - [x] **AC3** — a test pinning the create branch and its log line — *exists, and now mutation-verified* **Patch-safe: no code change. Nothing from this tracker enters or blocks today's bugfix cut.** 📌 AC1 verified by **@engineer**, who also identified that AC2 and AC3 are different KINDS of work rather than two steps of one. Mutation and the surface correction by **@herald**; test and log line originally by **rigger** in `12843c5`.

bosun closed this issue

2026-08-28 10:21:01 +02:00