Skip to content

seed-repo-template.sh destroys files it does not own: wipes the 420-line managed secrets .gitignore baseline and overwrites the drift-allowlisted ci.yml #1812

Closed

Description

@don-petry

Story

As someone re-seeding petry-projects/repo-template to clear a template-drift failure,
I want the seeder to leave files it does not own alone,
so that a routine drift fix cannot quietly delete the org's push-protection baseline.

What happens

scripts/seed-repo-template.sh writes its full scaffold unconditionally. Two of those writes
destroy content the seeder is not the owner of:

1. .gitignore — the managed secrets baseline is replaced by a 16-line stub (security-relevant)

petry-projects/repo-template@main carries an 11,412-byte, 420-line .gitignore opening with:

# >>> BEGIN petry-projects secrets baseline (managed by .github — do not edit) >>>
# ...
# First layer of defense in the Push Protection Standard (standards/push-protection.md).

The seeder replaces the whole file with a 145-byte generic stub (.DS_Store, node_modules/,
*.log, …). The marker says the file is managed by a different process and must not be edited —
the seeder does not know that and clobbers it wholesale.

Consequence: every repo created from the template afterwards inherits a .gitignore with no secrets
baseline at all
, silently removing the first layer of the Push Protection Standard.

2. .github/workflows/ci.yml — overwritten despite being explicitly allowlisted

scripts/template_stub_drift.sh documents ci.yml as the one intentionally-customized file:

# ci.yml is the ONE shipped stub that intentionally carries real per-stack
# build/test steps and is customized per consumer ... A byte-identity guard
# would false-positive on the template's richer ci.yml default, so it is excluded
readonly -a TEMPLATE_DRIFT_ALLOWLIST=( ".github/workflows/ci.yml" )

The drift checker and the seeder therefore disagree about who owns ci.yml: the checker says
"customized, don't compare", the seeder says "mine, overwrite" (+20/-53 — it replaces the richer
default with the thin stub).

Evidence

Observed on petry-projects/repo-template PR #198 (branch seed-scaffold/2026-09-13) on 2026-09-13.
The seeder ran repeatedly that day and each run re-applied both destructive writes. I restored both
files on the branch by hand — the PR now carries only the intended drift fixes — but the next seed run
will redo it.

Acceptance Criteria

  1. The seeder never writes a file whose content is owned by another managed process. .gitignore
    is the live case: detect the BEGIN petry-projects secrets baseline marker and leave the file
    alone, or compose the stub around the preserved baseline block. Deleting a block marked
    "do not edit" must not be reachable.
  2. The seeder honours TEMPLATE_DRIFT_ALLOWLIST. A path the drift checker excludes as
    intentionally-customizable must not be overwritten by the seeder. Derive it from the one list in
    template_stub_drift.sh — do not copy the paths into a second literal, which is the drift this
    pair of scripts already demonstrates.
  3. Seeding an already-seeded repo is a no-op for unchanged files. The branch above accrued ~100
    commits from repeated runs because every file is rewritten every time. Write only when content
    actually differs.
  4. DRY_RUN=true reports what would be destroyed, not just what would be written. The current
    dry-run output ("would create branch … write the scaffold … open one PR") gave no indication that a
    420-line managed file was about to become 16 lines. A destructive write must be visible before it
    happens — that is the whole purpose of the dry run.
  5. Covered by tests: seeding a fixture repo whose .gitignore carries the managed marker preserves
    it; seeding one whose ci.yml is customized leaves it untouched; a second seed of an unchanged
    fixture writes nothing. Offline, stubbed — no network.

Dev Notes

References


Delivery brief — 2026-09-21 (read this first; implement it, do not re-ask)

The motivating urgency is gone, which makes this safer to do properly. #1795 is fixed and merged: a failing template-drift no longer deadlocks merges, so nobody needs to re-run the seeder in a hurry to clear it. Fix ownership properly rather than racing a re-seed.

Do NOT run scripts/seed-repo-template.sh against petry-projects/repo-template as part of this work — not even to verify. Every run re-applies both destructive writes. Use offline fixtures only (AC #5 already requires this).

Pre-answered decisions

  1. AC Go-live improvements for PR review agent #2 — derive the allowlist, never copy it. TEMPLATE_DRIFT_ALLOWLIST in scripts/template_stub_drift.sh is the single source. Source it (or parse it from that one file); a second literal list is the exact drift this pair of scripts already demonstrates, and copying it would be delivering the bug the issue describes.
  2. AC test issue from agent #1 — marker-aware, not path-special-cased. Detect the BEGIN petry-projects secrets baseline (managed by .github — do not edit) marker generically: any file carrying a managed-block marker is preserved (or composed around) regardless of its path. Hardcoding .gitignore leaves the next managed file to be discovered the same way — by losing it.
  3. Find the other writer before choosing a boundary (Dev Notes). The baseline appears to be synced from petry-projects/.github. Identify it and state in the PR which process owns which bytes. If you cannot find it, say so explicitly rather than guessing — do not invent a boundary that the real writer will then fight.
  4. AC Optimize review: small-PR and incremental fast paths #4 — a destructive write must be reported by name and magnitude. DRY_RUN=true must print what would be lost (e.g. "would replace 420-line .gitignore with 16-line stub — contains managed baseline"), not just what would be written. Silent destruction under a dry run is the defect that let this reach main.
  5. AC Add @claude delegation, auto-merge, and rebase handling #3 — write only on difference. Compare content before writing so a second seed of an unchanged repo is a true no-op. The ~100-commit branch is the evidence this is not cosmetic.

Do not

  • Do not "fix" this by dropping .gitignore or ci.yml from the seeder's manifest — a fresh repo needs both. Ownership-awareness is the fix.
  • Do not modify petry-projects/repo-template from this PR.
  • Do not touch scripts/template_stub_drift.sh's allowlist contents; you are consuming it, not editing it.

Budget guard

  1. Ownership-awareness (AC test issue from agent #1, Go-live improvements for PR review agent #2) plus the offline fixture tests (AC feat: add Copilot engine support via REVIEW_ENGINE toggle #5). This is the commit that matters — it is what stops the next seed run destroying the push-protection baseline.
  2. Idempotent writes (AC Add @claude delegation, auto-merge, and rebase handling #3).
  3. Dry-run destructive reporting (AC Optimize review: small-PR and incremental fast paths #4).

If budget runs low, stop after step 1 with a green PR and file the rest as a follow-up. A partial fix that preserves managed content is worth far more than a complete one that times out and lands nothing — #1734 lost its work exactly that way.

Security note: this is labeled security because the destroyed file is the first layer of the Push Protection Standard. Treat a regression here as a security regression, and say in the PR description what now prevents it.

Activity

  1. github-actions commented on Sep 13, 2026

    @github-actions
    Contributor

    Thank you for this exceptionally thorough report — the evidence trail, acceptance criteria, and dev notes make the scope very clear. Given the security implications of silently stripping the push-protection baseline from any repo seeded after this bug was introduced, the most important open question is: have you audited downstream repos already created from repo-template during the window the seeder was running this way, to determine how many are currently missing the secrets .gitignore baseline? That blast-radius answer will shape whether this needs an emergency remediation pass across existing repos in addition to the seeder fix itself.

  2. added
    dev-leadFor dev-lead agent pickup
    and removed
    needs-human-reviewFlagged by automated PR review agent
    on Sep 21, 2026
  3. don-petry commented on Sep 21, 2026

    @don-petry
    CollaboratorAuthor

    Dev-Lead Implementation Plan

    Issue: #1812 — seed-repo-template.sh destroys files it does not own

    Scope

    Make scripts/seed-repo-template.sh ownership-aware so it never clobbers a managed-block-marked file (the org secrets baseline in .gitignore) or a drift-allowlisted file (ci.yml), writes only on real difference, and reports destructive intent under DRY_RUN.

    Ownership boundary (Dev Notes)

    The .gitignore L1 secrets baseline is org-managed and synced from petry-projects/.github per the gitignore-standard (two-layer model: L1 secrets baseline = org-managed verbatim block delimited by >>> BEGIN … (managed by .github — do not edit) >>> / <<< END … <<<; L2 = per-repo ecosystem/OS extension). The seeder owns only an L2-style generic stub and must never overwrite the L1 managed block. Boundary stated: any file carrying a managed by … do not edit marker is owned by the syncing process; the seeder preserves it.

    Implementation Checklist

    Tests to Write (offline, stubbed — AC #5)

    • seeding a fixture .gitignore carrying the managed marker preserves it (no PUT)
    • seeding a customized/allowlisted ci.yml that already exists leaves it untouched
    • a second seed of an unchanged fixture writes nothing (idempotent no-op)
    • a fresh repo (file absent) still gets .gitignore and ci.yml created
    • DRY_RUN reports the managed .gitignore would be preserved, not silently replaced
    • allowlist is derived from template_stub_drift.sh, not a second literal

    Test Command

    bats tests/test_seed_repo_template.bats (plus bash .dev-lead/scripts/dev-lead-lint.sh)

  4. don-petry commented on Sep 21, 2026

    @don-petry
    CollaboratorAuthor

    Dev-Lead: Implementation Complete — PR #1868

    The implementation for issue #1812 is durable: commits are pushed and a pull request is open. This record is backed by verifiable artifacts you can confirm in one click.

    Review happens on the PR. Acceptance criteria and test results are evidenced by the PR's diff and CI checks — not asserted here. If a later stage fails, this claim is retracted in place.

  5. added a commit that references this issue on Sep 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugBug reportsdev-leadFor dev-lead agent pickupsecuritySecurity-related PRs and issues

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions