Skip to content

Handle force-pushed public boundary scan ranges - #614

Merged
rmcdaniel merged 3 commits into
mainfrom
fix/public-boundary-force-push-613
Oct 27, 2026
Merged

rmcdaniel merged 3 commits into
mainfrom
fix/public-boundary-force-push-613

Conversation

@rmcdaniel

@rmcdaniel rmcdaniel commented Oct 21, 2026 •

Copy link
Copy Markdown
Member

A force push can leave the event's old tip outside the checkout. Fetch that exact missing commit with its ancestry before scanning before..head. A depth-one fetch can incorrectly hide shared history and scan unrelated ancestors; this repair preserves the exact introduced-commit range and the scanner's fail-closed behavior.

Validation: all 14 public-boundary cases pass, including a real Git fixture with an unadvertised old tip and forbidden metadata in a shared ancestor. The old depth-one workflow fetch reproduces the false-positive scan; the corrected workflow fetch scans exactly the two introduced commits. All 72 Docker workflow-isolation policy tests and the static isolation scan pass; git diff --check passes. Hosted checks and exact-head review remain required.

Closes #613

@rmcdaniel
rmcdaniel enabled auto-merge (squash) October 21, 2026 19:32
@rmcdaniel

Copy link
Copy Markdown
Member Author

The first PR run exposed a repository-policy incompatibility: the new exact-SHA fetch used a shell variable, and the Docker workflow-isolation checker deliberately rejects unclassified git commands with dynamic arguments.

Exact head 6e520b5f368676f4d51a8e033b948a7d21f9dcf7 keeps the same fail-closed fetch but passes the GitHub-generated github.event.before SHA directly to git cat-file and git fetch, so the checker can see that no shell variable or executable position is involved. No policy exception was added.

Validation on this head:

  • Docker workflow isolation scan: pass
  • workflow isolation unit suite: 72 tests pass
  • public-boundary metadata suite: 13 cases pass
  • actionlint: pass
  • isolated shallow checkout: old tip absent initially, exact historical SHA fetched successfully, before..head scan passes
  • git diff --check: pass

CI is rerunning on the exact head.

@rmcdaniel

Copy link
Copy Markdown
Member Author

Security review of exact 6e520b5f368676f4d51a8e033b948a7d21f9dcf7: accepted. github.event.before is GitHub-issued commit identity, is single-quoted in the shell, and the exact-SHA fetch runs only when the object is absent. Fetch or ancestry failure remains fail-closed; root pushes and existing PR/manual ranges are unchanged. The policy scanner also rejected the first dynamic-argument draft and the corrected exact head passed all current required checks. No security blocker. Coordinate the merge with the active Rust Server train so current exact-head matrices are not needlessly invalidated; no merge performed by this review.

@rmcdaniel

Copy link
Copy Markdown
Member Author

Cloud review: keep the old commit’s complete ancestry before scanning the push range. Exact reviewed head 6e520b5f368676f4d51a8e033b948a7d21f9dcf7, .github/workflows/public-boundary.yml:37.

The new --depth=1 fetch successfully obtains a missing before, but makes it a shallow boundary. The subsequent before..head traversal cannot exclude the shared ancestors behind that boundary. scripts/check-public-boundary.sh:66 uses git rev-list on that exact range, so it scans historical metadata outside the introduced push commits and can reject the push for an old commit.

Confirmed with Git2.51.1 in fresh, disposable local repositories. The remote has only its new main reference; the old tip is genuinely unreferenced. A full clone is initially non-shallow and cannot resolve the old tip. Both exact-SHA fetches succeed:

Fetch Repository after fetch Metadata selected by before..head
Proposed git fetch --no-tags --depth=1 origin BEFORE shallow introduced two, introduced one, historical common, historical root —4commits
git fetch --no-tags origin BEFORE control complete introduced two, introduced one —the expected2commits

The histories are root→common→old1→old2(before) and root→common→new1→new2(head). This reproduced with both a missing retained-branch tip and a genuinely unreferenced old tip/full clone. No real repository, source file or remote was changed.

Please remove the depth truncation while retaining the existing exact GitHub-issued SHA quoting and fail-closed fetch, and add a diverged-history regression that asserts the exact introduced metadata commit set. Existing present-object/root/PR/manual cases should retain their current behavior. The current green checks and Security review remain evidence for the old head; they do not exercise this missing-ancestry case. Git’s fetch depth semantics and revision-set semantics support the locally observed range expansion. No merge was performed by this review.

@rmcdaniel

Copy link
Copy Markdown
Member Author

Exact follow-up head 680715905 corrects the review finding about shallow old-tip ancestry. It removes --depth=1 from the narrowly scoped missing-before fetch and adds a regression that executes the actual workflow fetch command in a real Git fixture. The original command reproduces the wrong range; the repaired command scans exactly the introduced commits without the shared ancestor. Boundary suite14/14, isolation policy72/72, static isolation and diff checks pass. Normal hosted checks and reviewer recheck remain required; no repository protections changed.

@sonarqubecloud

Copy link
Copy Markdown

@rmcdaniel

Copy link
Copy Markdown
Member Author

Cloud's follow-up source review accepts exact head 6807159052e270d5e718e700f82ab53c5336dddd.

The missing-before fetch now preserves ancestry. The new divergent-push fixture executes the workflow's actual fetch command, verifies the old tip is initially absent, and requires the scan range to contain exactly the two introduced commits. Its shared ancestor carries a synthetic forbidden marker, so scanning old history would fail rather than silently pass.

Independent verification at this head:

  • All 14 public-boundary metadata cases pass.
  • All 72 Docker workflow isolation policy tests pass, and the static isolation checker passes.
  • The current selected-commit boundary scan passes.
  • An external negative-control fixture using the prior workflow's depth-one command fails specifically at divergent-push, as expected. The reviewed checkout remained unchanged.

Exact-SHA quoting, fail-closed fetch behavior, and the PR/manual/root-push paths are preserved. This resolves my earlier ancestry finding in comment 6447148093. Normal required checks and merge-queue qualification still govern delivery; this is an exact-head source/test disposition, not a claim that the remaining hosted checks have finished.

@rmcdaniel
rmcdaniel added this pull request to the merge queue Oct 27, 2026
Merged via the queue into main with commit 73989e8 Oct 27, 2026
22 checks passed
@rmcdaniel

Copy link
Copy Markdown
Member Author

The ancestry repair merged through the normal merge queue at 73989e878f497853e28bcef2a1fa1d0dc4580c20. Exact reviewed source680715905, all14boundary cases and72isolation-policy tests passed. Cloud's independent source/negative-control acceptance is6450774885. The complete merge-group shared fixtures40564473096 and PHP/source qualification40564473000 passed at that merge commit; Public Boundary also passed. No admin/check/settings bypass was used. The workflow now preserves ancestry when fetching a missing old push tip and scans only the introduced range. This workflow-only delivery needs no Server image publication.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Make public-boundary scans handle force-pushed branch history

1 participant