Repository navigation
feat(OMN-17320): gate denylisted customer identifiers out of this public repo - #3074
Merged
jonahgabriel merged 2 commits intoAug 31, 2026
Merged
Conversation
…lic repo OMN-17288 scrubbed a live tenant slug out of five files in this repo (#3062, 3f10ee5) and established a synthetic-identifier convention in its place. Three hours later an unrelated lane reintroduced the same slug in omnimarket (#2239), and the rebase carried it onto omnimarket#2241 -- the PR whose own acceptance criterion was "zero grep hits" -- with every enforced gate green. Nothing in either repo was looking. The convention was documentation, and documentation lost a race in three hours. Operating Rule #5: detection that is not a gate gets ignored. Why digests and not a plaintext pattern list: this repo is PUBLIC. Writing a forbidden customer identifier into a pattern file here would create exactly the fresh, greppable, current-tree occurrence the class exists to prevent, and would force that file to be exempt from its own rule -- a special file holding the forbidden value, that nobody scans and that people copy from. That is the shape of the incident, not a fix for it. Entries are salted SHA-256 plus a class label and owning ticket; the loader refuses to load an entry carrying a value/literal/ plaintext field. The salt is committed, so this is obfuscation and not secrecy, and that is stated in the file rather than implied: the OMN-17288 values are already public in git history and the operator ruled document-and-accept on that history. What the format buys is FORWARD safety -- the next entry may be a live identifier that has NOT leaked, where a plaintext denylist would be an active disclosure. Matching windows inside each identifier token, so a literal is caught bare, embedded (tenant_<slug>_v2), and inside a path or URL segment. Findings print path:line:col plus match length and entry id, never the value. Encoded forms are deliberately NOT decoded -- OMN-17180 owns that class, and claiming coverage here would be a false claim. One escape hatch, per line, ticket + reason required: # onex-allow-exposed-identifier OMN-XXXXX reason="<concrete reason>" A bare annotation is rejected, matching every other onex-allow class. There is deliberately NO file-level waiver and NO self-exempt file: a whole-file waiver is how a forbidden value survives in a corner nobody reads. The gate is subject to its own rule. Wired in this same PR on both surfaces (Rule #5): - pre-commit hook `exposed-identifier-gate` - CI job `Exposed Identifier Gate (OMN-17320)` in ci.yml, registered in scripts/ci/ci_summary_gate.py::STRICT_GATE_JOBS. That registration is half the mechanism: dev requires exactly one context (CI Summary), and while the default-deny sweep already fails on a FAILING job, an unregistered job that is skipped or deleted yields SUCCESS -- so without it, removing this job would silently restore the unenforced state that produced the recurrence. Evidence: - Incident replay (OMN-15547 convention) over the real pre-scrub artifact, captured from git object 6527db3 (= 3f10ee5^). ONE same-length redaction of the slug, recorded in registry.yaml with the pre-redaction sha256 so the git object can be re-fetched and diffed; every other byte of the 8455 is verbatim and offsets are preserved, so the finding is asserted at the slug's real position (line 7, col 379). An accept-control in the same module requires the SHIPPED denylist to PASS those same bytes, so a reject-everything guard cannot satisfy the case. - Non-vacuity of all five real entries proven OUT OF TREE (committed tests cannot carry this without defeating the gate's own rule): pre-scrub content extracted from git objects identifies the slug, slug-body, uuid and uuid-prefix entries by digest, and the uuid-hex entry is confirmed by transforming the recovered UUID. Scanner exit 1 with 15 findings across bare/embedded/path-segment forms. - 29 tests pass; full-tree scan clean. Ticket: OMN-17320
Contributor
|
| Verdict | Meaning | Blocks merge? |
|---|---|---|
passed |
No critical findings | No |
blocked |
CRITICAL findings found | Yes |
degraded |
All models unavailable (infra) | No (pilot) |
Powered by omniintelligence.review_pairing.cli_review — multi-model adversarial review (OMN-8468/OMN-8524)
jonahgabriel
pushed a commit
to OmniNode-ai/onex_change_control
that referenced
this pull request
Aug 31, 2026
…ibase_infra#3074 (#7850) * evidence(OMN-17320): author OCC companion for OmniNode-ai/omnibase_infra#3074 OCC companion by node_pr_lifecycle_fix_effect (OMN-13317 F1 / OMN-13990 / OMN-14285). Product PR head 982125bce57ad6cb273424de39ddf5647f9506d3. * evidence(OMN-17320): self-bind OCC#7850 + rebind contract_sha256 --------- Co-authored-by: omnimarket-bot <bot@omninode.ai>
jonahgabriel
enabled auto-merge (squash)
August 31, 2026 15:52
jonahgabriel
deleted the
jonah/omn-17320-exposed-identifier-enforcement-gate
branch
August 31, 2026 16:02
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.
What and why
OMN-17288scrubbed a live customer's tenant slug out of five files in this repo (#3062,3f10ee5e) and established a synthetic-identifier convention in its place. Three hours later an unrelated lane reintroduced the same slug in omnimarket (#2239), and the rebase carried it onto#2241— the PR whose own acceptance criterion was "zero grep hits" — with every enforced gate green. Nothing in either repo was looking.The convention was documentation, and documentation lost a race in three hours. Operating Rule #5: detection that is not a gate gets ignored. This PR is the gate.
Ticket: OMN-17320
Companion PR: OmniNode-ai/omnimarket#2243 (byte-identical scanner + denylist)
Design decision: digests, not a plaintext pattern list
This repo is PUBLIC. Writing a forbidden customer identifier into a pattern file here would (1) create exactly the fresh, greppable, current-tree occurrence the gate exists to prevent, and (2) force that file to be exempt from its own rule — a special file holding the forbidden value, that nobody scans and that people copy from. That is the shape of the incident, not a fix for it.
Entries are salted SHA-256 plus a class label and owning ticket; the loader refuses any entry carrying a
value/literal/plaintextfield.Stated plainly rather than glossed: the salt is committed, so this is obfuscation, not secrecy — a reader holding this repo can brute-force a short slug. That is not the property being bought. The OMN-17288 values are already public in git history and the operator ruled document-and-accept on that history; this PR does not try to undo it. What the format buys is forward safety: the next entry added here may be a live identifier that has not leaked, where a plaintext denylist would be an active disclosure.
Wiring — both surfaces, this PR (Rule #5)
exposed-identifier-gateExposed Identifier Gate (OMN-17320)inci.yml, registered inscripts/ci/ci_summary_gate.py::STRICT_GATE_JOBSThat registration is half the mechanism, and is the part specific to this repo:
devrequires exactly one context (CI Summary), so a workflow file alone is not enforcement here. The default-deny sweep already fails CI Summary when the job fails — but an unregistered job that isskippedor deleted yields SUCCESS. Without the registration, deleting this job would silently restore the unenforced state that produced the recurrence. The job is unconditional (if: always()), so a skip is anomalous and never a legitimate opt-out.The CI job scans the full tree, not the diff: a denylisted identifier reaching
devby any route — including a rebase carrying it in, which is exactly what happened on omnimarket#2241 — must red the PR, not only one that edits the offending line.Matching and exemption
Windowed inside each identifier token, so a literal is caught bare, embedded (
tenant_<slug>_v2), and inside a path/URL segment. Findings printpath:line:col+ match length + entry id — never the value. Encoded forms are deliberately not decoded; OMN-17180 owns that class and claiming coverage here would be a false claim.One escape hatch, per line, ticket + reason required:
A bare annotation is rejected, matching every other
onex-allowclass. There is deliberately no file-level waiver and no self-exempt file — a whole-file waiver is how a forbidden value survives in a corner nobody reads. The gate is subject to its own rule.dod_evidence
AC1 — the gate is not exempt from itself.
git grepfor either OMN-17288 literal over the full worktree returns 0 hits, including the scanner and denylist themselves (verified tracked + untracked).AC2 — bare / embedded / path-segment, proven by committed tests over a synthetic denylist entry.
AC3 — non-vacuity against the REAL entries, proven out-of-tree. Committed tests cannot carry this proof without defeating AC1; that split is stated, not glossed. Method: pre-scrub content was extracted from git objects that already exist (
3f10ee5e^, and omnimarket'seb356f07^/e0cb5235^) into a scratch tree outside both repos, and the shipped denylist was used as an oracle to identify the real literals inside it — so the proof was produced without hand-typing a literal anywhere. Result: all 5 entries confirmed live —omn17288-tenant-slug,-slug-body,-tenant-uuid,-uuid-prefixmatched real historical content directly;-uuid-hexwas confirmed by transforming the recovered UUID.This mattered: an earlier pass over only the slug-carrying files matched 2 of 5 entries — the three UUID entries were unproven until the UUID-carrying files were pulled in. A vacuous digest would have shipped silently otherwise.
AC4 — RED-first incident replay (OMN-15547 convention),
tests/ci/test_incident_replay_omn17320.py+tests/incident_replays/registry.yaml. The artifact isdocker/migrations/forward/_ledger/migration-supersessions.tsvexactly as it stood at6527db3f(=3f10ee5e^), immediately before the scrub — one of the five files the OMN-17288 census enumerated.An accept-control in the same module (
test_the_shipped_denylist_does_not_fire_on_this_artifact) requires the SHIPPED denylist to pass those same bytes, so a guard that simply rejects everything cannot satisfy the case.AC5 — annotation with ticket+reason exempts; bare annotation does not. Committed tests.
AC6 — pre-commit hook + CI gate +
STRICT_GATE_JOBSregistration, all in this PR.AC7 — scanner + denylist byte-identical with omnimarket, pinned by
test_cross_repo_fingerprint_pin:Tests: 29 pass locally; the governed pre-push selector escalated to the full suite (
reason=test_infrastructure) and it ran green onh101over the remote leg — no override grant, noPREPUSH_*, no skip token.Scope
Only omnimarket + omnibase_infra — the two repos the OMN-17288 occurrence map names as having had hits. Rolling to the other nine public repos is the G1-FULL shape OMN-16156 deferred post-beta, explicitly not in scope. Git-history rewrite is closed by operator ruling on OMN-17288.
Evidence-Ticket: OMN-17320
Evidence-Source: OCC#7850