Skip to content

fix(OMN-17324): a sibling clone git cannot read is not a sibling clone - #1631

Merged
jonahgabriel merged 1 commit into
devfrom
lakshman/omn-17324-flip-bundle-sibling-repo-guard
Aug 31, 2026
Merged

jonahgabriel merged 1 commit into
devfrom
lakshman/omn-17324-flip-bundle-sibling-repo-guard

Conversation

@Patel230

@Patel230 Patel230 commented Aug 31, 2026 •

Copy link
Copy Markdown
Contributor

Closes OMN-17324.

test_verify_flip_bundle::test_passes_on_committed_infra_flips fails on pristine dev for any user of the shared .201 box who does not own the omnibase_infra clone there, reddening the whole governed suite and blocking EVERY omnibase_core push.

Root cause, from a real run

_resolve_infra_root() walks up from the test file looking for a sibling omnibase_infra and accepts the first candidate whose scripts/ci/adequacy_receipts is a readable directory. On .201 a collaborator clone lives at /data/omninode/<user>/omnibase_core, so the walk reaches /data/omninode and finds omnibase_infra there — owned by another user, mode drwxrwxr-x, therefore world-readable and accepted.

Git then refuses every query against it:

fatal: detected dubious ownership in repository at /data/omninode/omnibase_infra

That is git safe.directory (CVE-2022-24765) declining a repo owned by a different uid. So _git_ref_exists(repo_root, "origin/dev") exits non-zero, load_base_baseline returns None, and _assert_twin_baseline fails closed — correctly, on a question it was never able to ask.

Not a missing artifact, not wrong ref derivation, not a shallow clone, not a genuinely shrunk baseline.

The fix

Fail-closed on an unresolvable baseline is correct and is unchanged.

is_dir() tests filesystem readability; the assertions need git to answer ref queries. Those are different properties, and the skipif reason — "omnibase_infra canonical clone not present" — already anticipates unavailability. It was testing for the wrong kind.

_resolve_infra_root now requires a candidate to be both shaped like the clone AND readable by git as the current user, and skips a failing candidate rather than returning it, so the search continues to one that qualifies. When none does it returns None and the infra assertions skip, which is the documented behaviour.

The probe scrubs GIT_DIR/GIT_WORK_TREE/GIT_INDEX_FILE/GIT_COMMON_DIR via scrub_git_location_env (OMN-14891). Load-bearing rather than boilerplate: git exports those into every hook environment and they override both cwd= and git -C, so an unscrubbed probe would answer about the worktree that invoked the pre-push hook instead of about the candidate — reporting True for a repo it never looked at. Caught by the repo own gate on the first commit attempt.

Regression proof on .201, same box, same account, detached worktree:

BEFORE (origin/dev):  2 failed, 24 passed
AFTER  (this branch): 26 passed, 3 skipped   exit 0

SKIPPED [2] omnibase_infra canonical clone not present (e.g. isolated core CI)
SKIPPED [1] no omnibase_infra clone resolvable here

Environmental and standing rather than a regression: only two commits separate the reported dev from current dev and neither touches this machinery.

29 passed locally, ruff clean, ruff format clean. Governed pre-push green — 2104 passed, 1 skipped, no PREPUSH_* override, no --no-verify, no skip token.

The one mypy --strict error on this path (verify_flip_bundle.py:881, canonical_handler_shape yaml not explicitly exported) is pre-existing on pristine origin/dev and untouched here.

Tests

Three. A counter-test pins a directory shaped like the infra clone but unusable by git — a uid mismatch cannot be simulated portably, so it pins the property the old guard lacked rather than the exact mechanism. A positive test stops the counter-test being satisfiable by a function returning False unconditionally, which would silently skip the infra coverage everywhere and retire it instead of fixing it. An invariant test asserts whatever the walk-up selects is always git-readable.

Unblocks the OMN-16998 re-push.

Evidence-Ticket: OMN-17324
Evidence-Source: OCC#7880

test_passes_on_committed_infra_flips failed on pristine dev for any user of the
shared .201 box who did not own the omnibase_infra clone there, reddening the
whole governed suite and blocking EVERY omnibase_core push.

Root cause, from a real run rather than inference.

`_resolve_infra_root` walks up from the test file looking for a sibling
omnibase_infra and accepts the first candidate whose
`scripts/ci/adequacy_receipts` is a readable directory. On .201 a collaborator's
clone lives at /data/omninode/<user>/omnibase_core, so the walk reaches
/data/omninode and finds omnibase_infra there -- owned by another user, mode
drwxrwxr-x, therefore world-readable and accepted.

Git then refuses every query against it:

    fatal: detected dubious ownership in repository at
    '/data/omninode/omnibase_infra'

That is git's safe.directory protection (CVE-2022-24765) declining a repo owned
by a different uid. So `_git_ref_exists(repo_root, "origin/dev")` exits
non-zero, `load_base_baseline` returns None, and `_assert_twin_baseline` fails
closed -- correctly, on a question it was never able to ask.

Reproduced: 2 failed / 24 passed in the collaborator account on .201; 26 passed
where the sibling clone is owned by the same user. Only two commits separate the
reported dev from current dev and neither touches this machinery, so this is
environmental and standing, not a regression.

The defect is the guard, not the fail-closed. `is_dir()` tests filesystem
readability; the assertions need git to be able to answer ref queries. Those are
different properties, and the skipif's own reason -- "omnibase_infra canonical
clone not present" -- already anticipates unavailability. It was testing for the
wrong kind.

`_resolve_infra_root` now requires a candidate to be both shaped like the clone
AND readable by git as the current user, and skips a failing candidate rather
than returning it so the search continues to one that qualifies. When none does
it returns None and the infra assertions skip, which is the documented
behaviour.

The probe scrubs GIT_DIR/GIT_WORK_TREE/GIT_INDEX_FILE/GIT_COMMON_DIR via
scrub_git_location_env (OMN-14891). That is load-bearing here rather than
boilerplate: git exports those into every hook environment and they override
both cwd= and `git -C`, so an unscrubbed probe would answer about the worktree
that invoked the pre-push hook instead of about the candidate -- reporting True
for a repo it never looked at, which is the precise failure it exists to
prevent. Caught by the repo's own gate on the first commit attempt. Config is
left intact so a real safe.directory exemption is still honoured.

Three tests. A counter-test pins a directory shaped like the infra clone but
unusable by git; a uid mismatch cannot be simulated portably, so it pins the
property the old guard lacked rather than the exact mechanism. A positive test
stops the counter-test being satisfiable by a function returning False
unconditionally, which would silently skip the infra coverage everywhere and
retire it instead of fixing it. An invariant test asserts whatever the walk-up
selects is always git-readable, so a future candidate source cannot reintroduce
the defect by a route these tests did not anticipate.

29 passed, ruff clean, ruff format clean. The one mypy --strict error on this
path (verify_flip_bundle.py:881, canonical_handler_shape "yaml" not explicitly
exported) is pre-existing on pristine origin/dev and untouched here.
@jonahgabriel
jonahgabriel enabled auto-merge (squash) August 31, 2026 22:03
jonahgabriel pushed a commit to OmniNode-ai/onex_change_control that referenced this pull request Aug 31, 2026
…#7880)

* evidence: OCC companion pass 1 for OmniNode-ai/omnibase_core#1631

* evidence: OCC companion self-bind for #7880

---------

Co-authored-by: node-occ-companion-effect <occ-companion-effect@omninode.ai>
@jonahgabriel
jonahgabriel merged commit a4c600b into dev Aug 31, 2026
235 of 261 checks passed
@jonahgabriel
jonahgabriel deleted the lakshman/omn-17324-flip-bundle-sibling-repo-guard branch August 31, 2026 22:23
@Patel230

Copy link
Copy Markdown
Contributor Author

Closing this out for OMN-17324. No review comments were left on this PR, so this is the record of what landed.

Merged 2026-08-31T22:23:41Z as a4c600bc, one commit (6b12f935) in one test file. _resolve_infra_root() in tests/unit/scripts/ci/test_verify_flip_bundle.py now accepts a sibling omnibase_infra only if git can read it as the current user, and moves on to the next candidate otherwise. Fail-closed on an unresolvable baseline is unchanged.

How it was verified:

  • CI run 33443884227 green on 6b12f935: 145 checks passed, 3 skipped, none failed.
  • OCC companion onex_change_control#7880 merged first, at 22:12:41Z.
  • On .201, same box and account: origin/dev gave 2 failed / 24 passed, this branch 26 passed / 3 skipped, exit 0. Governed pre-push 2104 passed, 1 skipped, no override.
  • Jonah re-ran it on a fresh clone at a4c600bc (on the ticket, 2026-08-31T23:24Z): 2 failed before the fix with a sibling git cannot read, 2 skipped after it, 29 passed with a readable one.

It unblocked the OMN-16998 re-push, which landed as omnibase_core#1632. Nothing is left on this PR.

Thank you!

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.

2 participants