Skip to content

feat(OMN-20926): the content-capture producer stores detected secrets and emits references - #2672

Merged
jonahgabriel merged 10 commits into
devfrom
jonah/omn-20926-captured-secret-refs
Oct 13, 2026
Merged

jonahgabriel merged 10 commits into
devfrom
jonah/omn-20926-captured-secret-refs

Conversation

@jonahgabriel

@jonahgabriel jonahgabriel commented Oct 11, 2026 •

Copy link
Copy Markdown
Contributor

OMN-20926: the content-capture producer stores detected secrets and emits references

Operator ruling 2026-10-10 (RULING row 2026-10-10T22:38:48Z, lane=secret-refs-9f8a): a secret the content scrubber finds is stored in the secret store and the record carries secret://onex/captured/<hex> in its place; consumers resolve through the store. Ciphertext never goes on the bus. Mechanism ticket OMN-20926 (parent OMN-20768).

Companion of OmniNode-ai/omnimarket#3747, which owns the contract and both effects. This PR's mirrored contract is re-derived from that branch, so the registry drift check (which reads the omnimarket rev uv.lock pins) goes green once #3747 is released and the lock is bumped. Land #3747 first.

Changes

  • capture_redaction.py: parses the secret_reference block fail-closed, keeps references standing through every scrub exactly as omnimarket's resolver does, and never cuts a reference at a chunk boundary or the size cap. prepare_content_records takes a secret sink: each detected secret is offered to it, a returned reference replaces only the value, and a span the sink does not take crosses as the existing marker. Records carry stored_refs {stored, fallback, reason}: counts and a reason code only.
  • hook_content_capture.py: the sink is omnimarket's captured secret store effect, imported only when a capture finds a secret. A store failure (unreachable, login or write refused, misconfigured) falls back to the marker and journals a hook.health.error naming the reason and count, never a value. A machine with no store configured, or an omnimarket that predates the effect, falls back quietly. Capture is never blocked.
  • generate_event_registry.py: the posture drift check compares the secret_reference block.

Access needed (cloud owner)

Writer (create only) and reader (read only) identities for the captured folder; exact request on OMN-20926. Until they exist every detected secret falls back to the marker.

Verification

  • Local focused: tests/unit/hooks/test_hook_content_capture_secret_refs_omn20926.py, tests/unit/hooks/test_hook_content_capture_omn19551.py, tests/hooks/test_capture_redaction_omn17959.py, tests/hooks/test_capture_redaction_sync_omn18357.py green; drift check passes against the omnimarket branch; pre-commit and pre-push green.
  • Lab: h200 unit lw-7cf5ce7744f841e8 at d3859a4 ran those suites plus tests/hooks/test_deployed_registry_redaction_omn17201.py: 80 passed, 5 skipped, exit 0. The writer's fallback against the .201 dev lane Infisical is proven on omnimarket#3747 (h201 unit lw-903b5b9806dd4e40, outcome login_refused).
  • No tenant-form, RLS or reader-authorization path through onex-api is touched.

Evidence-Ticket: OMN-20926
Evidence-Source: OCC#13623

jonahgabriel pushed a commit to OmniNode-ai/onex_change_control that referenced this pull request Oct 11, 2026
)

* evidence(OMN-20926): author OCC companion for OmniNode-ai/omniclaude#2672

OCC companion by node_pr_lifecycle_fix_effect (OMN-13317 F1 / OMN-13990 / OMN-14285). Product PR head d3859a4cb2db6da5cbf9e41b556bdcf4f43eb78c.

* evidence(OMN-20926): self-bind OCC#13623 + rebind contract_sha256

---------

Co-authored-by: omnimarket-bot <bot@omninode.ai>
@jonahgabriel
jonahgabriel enabled auto-merge (squash) October 11, 2026 01:02
@github-actions

github-actions Bot commented Oct 11, 2026 •

Copy link
Copy Markdown
Contributor

✅ Hostile Reviewer — PASSED

Blocking findings (critical/error): 0
Total findings: 11
Models succeeded: correctness,security-contracts

Findings

Severity Location Finding
warning src/omniclaude/hooks/capture_redaction.py:1 ai-reviewer:correctness:correctness value_span returns match.span() when value_group is not found, but the group may exist with an empty match | In value_span, if group is in match.re.groupindex, it calls match.span(group). If the group matched an empty string (start == end), start >= 0 is true, so it returns (start, end) where start == end. This is a zero-width value span. Then in _scrub_str, match.string[start:end] is an empty string, and tally.sink('') is called. The store would store an empty secret, which is likely unintended. The contra
warning src/omniclaude/hooks/capture_redaction.py:1 ai-reviewer:correctness:security Secret value is passed to the store sink before the reference is validated | In _scrub_str, the code calls tally.sink(match.string[start:end]) and then checks if the returned reference is valid. If the sink is a real store, the secret value has already been sent to the store. If the store is compromised or the reference validation fails, the secret has already been exposed to the store. This is by design (store-then-reference), but the fallback path (when minted is None) means the secret was sent to the sto
info src/omniclaude/hooks/capture_redaction.py:1 ai-reviewer:correctness:performance reference_spans is called for every pattern in _scrub_str, causing O(n*m) complexity | In _scrub_str, for each secret pattern, sub_keeping_references is called, which calls reference_spans(text, contract). reference_spans does a finditer over the entire text for the reference pattern. If there are many secret patterns, the reference pattern is scanned multiple times. This is O(n * m) where n is the number of patterns and m is the text length. For large texts with many patterns, this could be slow. | Evidenc
info tests/unit/hooks/test_hook_content_capture_secret_refs_omn20926.py:1 ai-reviewer:correctness:testing No test for the case where the store sink returns a reference that does not match the pattern | In _scrub_str, if the sink returns a reference that does not match reference.pattern, the marker is used. There is no test for this case. The test test_a_store_failure_falls_back_to_the_marker_loudly tests when the sink returns None, but not when it returns an invalid reference. | Evidence: if minted is None or reference.pattern.fullmatch(minted) is None: return replacement | Fix: Add a test where the FakeStoreEf
info plugins/onex/hooks/lib/hook_content_capture.py:1 ai-reviewer:correctness:architecture The StoreSink class is defined in the hook module but the protocol is in the redaction module, creating a circular dependency risk | StoreSink is defined in hook_content_capture.py and implements the SecretSink protocol from capture_redaction.py. The hook module imports from the redaction module, but the redaction module does not import from the hook module. This is fine. But the StoreSink class is tightly coupled to the omnimarket store effect, which is imported lazily. If the store effect changes its inte
warning plugins/onex/hooks/lib/hook_content_capture.py:1 ai-reviewer:correctness:correctness The _report_store_fallback function uses hashlib.sha256 for the fingerprint, but the secret value is not included, so the fingerprint is not unique per secret | The fingerprint is computed as hashlib.sha256(f'hook_content_capture:secret_store:{sink.refusal}'.encode()).hexdigest()[:16]. This is a constant for a given refusal reason, not unique per secret or per capture. This means all fallback events with the same reason have the same fingerprint, which may cause deduplication in the health system to suppres
info plugins/onex/hooks/lib/hook_content_capture.py:1 ai-reviewer:correctness:security The onex_home function returns /.onex, which may not exist, and the store handler may fail to initialize | The onex_home function returns Path('/.onex').expanduser(). If this directory does not exist, the store handler may fail to initialize. The build_store_handler function does not check for the existence of the directory. The StoreSink class catches ImportError but not other exceptions from the handler initialization. If the handler raises a different exception, it will propagate and crash the capture.
warning src/omniclaude/hooks/capture_redaction.py:1 ai-reviewer:security-contracts:correctness Chunking may exceed the declared step size when a reference straddles a boundary | The function _chunks_keeping_references is documented to split text into chunks of at most step characters. However, when a reference span starts before a cut point and ends after it, _cut_outside_references moves the cut back to the reference start. If the text between the reference start and the original cut point exceeds step characters, the resulting chunk will be larger than step. The code includes a comment acknowledgin
warning plugins/onex/hooks/lib/hook_content_capture.py:1 ai-reviewer:security-contracts:security Secret values may be logged via sink.detail on store failure | In StoreSink.call, when the store handler returns a result with reference=None, the code sets self.detail = result.detail. The docstring for _report_store_fallback explicitly states 'Reason code and count only: never a value, a digest, a store address or a path.' However, it then logs _log(message + (f" ({sink.detail})" if sink.detail else "")). If the omnimarket handler's result.detail contains the secret value or a path to the store (which
info plugins/onex/hooks/lib/hook_content_capture.py:1 ai-reviewer:security-contracts:correctness Logic for counting fallback secrets in capture() is redundant and potentially misleading | In the capture() function, the code iterates over chunks and sums up the 'fallback' count from the stored_refs field in the first chunk. However, prepare_content_records already calculates the total fallback count for the entire record and stores it in the base record's stored_refs field. The loop over chunks is unnecessary for this aggregation because the stored_refs field is attached to the base record, not individu
info tests/unit/hooks/test_hook_content_capture_secret_refs_omn20926.py:1 ai-reviewer:security-contracts:testing Test for chunking does not verify the 'never inside a reference' invariant for all cases | The test test_chunking_never_cuts_a_reference asserts that the joined chunks equal the text and that the count of references is preserved. However, it does not explicitly assert that no chunk contains a partial reference (i.e., a substring that matches the start or end of the reference pattern but is not the full reference). While the count check is a good proxy, a more rigorous test would scan each chunk for partial

Gate semantics (pilot phase)

Verdict Meaning Blocks merge?
passed No critical/error findings No
blocked CRITICAL or ERROR findings found Yes
degraded Reviewer ran but every model failed mid-run No (pilot)
infra_error cli_review failed to run at all (install/crash) after 2 attempts Yes
unavailable_fork_isolation Fork PR — the LAN-only model is unreachable and the fleet is off limits Yes

Powered by omniintelligence.review_pairing.cli_review — node-based adversarial review via HandlerLlmCliSubprocess (OMN-8524)

… and emits references

Operator ruling 2026-10-10 (RULING row 2026-10-10T22:38:48Z,
lane=secret-refs-9f8a): when the content scrubber finds a secret in captured
content, the secret is stored in the secret store and the record carries a
reference in its place. Ciphertext never goes on the bus.

This is the omniclaude half; omnimarket owns the contract and both effects.

- capture_redaction.py (the producer's resolver) parses the contract's new
  secret_reference block fail-closed, keeps references standing through every
  scrub exactly as omnimarket's resolver does, and never cuts a reference at a
  chunk boundary or the size cap. prepare_content_records takes a secret sink:
  each detected secret is offered to it, a returned reference replaces only
  the value (a label and a closing quote stay), and a span the sink does not
  take crosses as the scrub marker. The record carries stored_refs
  {stored, fallback, reason}: counts and a reason code only.
- hook_content_capture.py's sink is omnimarket's captured secret store effect,
  imported only when a capture finds a secret. A store failure falls back to
  the marker and journals a loud hook.health.error (reason code and count,
  never a value); a machine with no store configured, or an omnimarket that
  predates the effect, falls back quietly.
- The mirrored contract is re-derived from omnimarket, and the posture drift
  check compares the secret_reference block too.
… a secret-shaped keyword

The pre-push secret scanner reads a keyword argument named like a secret as a
hardcoded value. The contract block keeps its name; only the resolved field on
omniclaude's RedactionContract changes.
@jonahgabriel
jonahgabriel force-pushed the jonah/omn-20926-captured-secret-refs branch from d3859a4 to c99ab78 Compare October 11, 2026 17:07
jonahgabriel added a commit that referenced this pull request Oct 11, 2026
…d of skipping off macOS (#2692)

The two Darwin-only skipif tests added by omniclaude#2680 skipped on the Linux
runner and sat outside the test-split skip baseline, so every full-width
Tests (Split n/m) run failed the Skip Count Ratchet (OMN-18776): omniclaude
#2664, #2672 and #2683. The baseline is shrink-only, so the tests change, not
the baseline: the parse leg runs bash -n with /bin/bash 3.2 on macOS and the
PATH bash elsewhere, and the positive control asserts macOS refuses the outage
pre-image while any other bash accepts it, which is why the static scan exists.

Onex-Lane: hook-bash32-9f8a
Onex-Session: 087b253dbb0448959a08c95511d7ddb3
Re-derived src/omniclaude/hooks/contracts/capture_redaction.yaml with generate_event_registry.py --sync-capture-contract against omnimarket at a commit containing omnimarket#3747.

The Registry Consistency check had failed on the mirror as drifted (keys secret_patterns, secret_reference, topics).

Result: 7 insertions, 1 deletion in the mirror.

After the sync:
- generate_event_registry.py --check passed (77 event types)
- check_registry_consistency.py passed
- 86 capture tests passed, 2 skipped
… tests

Merged origin/dev into the branch.
Advanced the omnimarket git pin in pyproject.toml and uv.lock from a44caa7efd79 to f7ff837dfb88 so the registry drift check reads an omnimarket that carries omnimarket#3747.
Added contracts/OMN-20926.yaml with four acceptance criteria bound to the focused secret-reference tests and the registry parity check.
Added two tests for sub_keeping_references: a match starting inside a reference keeps the reference and the text after it; a zero-width pattern terminates and leaves references standing.
The mirrored capture_redaction.yaml was re-derived with --sync-capture-contract and did not change.
Focused tests: 78 passed before the two new tests, 16 passed in the secret refs file after.
… merge base

The dod-omn20926-mirror-parity check ran the sync test module, which passes at the merge base, so repo-evidence refused it as an always-pass control. It now runs a test of the new secret_reference posture key: it fails when the generator does not compare that block and passes when it does.
…t fix landed

The pin-test fix merged to dev with its own contracts/OMN-20926.yaml, which collides with this branch's contract of the same name. The merged contract keeps both requirements, evidence commands and dod entries; the pin-test criteria are renumbered AC5 and AC6 so the ids stay unique.
…th dev

contracts/OMN-20926.yaml: the two evidence items dod-omn20926-ac1-pin-tests-fetch-by-sha and dod-omn20926-ac2-pin-ancestry-unshallow now match, field for field, the items already on dev, so repo-evidence / dod-verify treats them as carried from the merge base. Reason: the pin-test fix landed on dev in #2710, so those tests pass at the merge base and a rebound item failed the base control as always-pass. Acceptance ids renumbered: the pin criteria are AC1 and AC2 again as on dev; the capture criteria are AC3 to AC6. No code or test change.
@jonahgabriel
jonahgabriel merged commit 1da9a98 into dev Oct 13, 2026
151 checks passed
@jonahgabriel
jonahgabriel deleted the jonah/omn-20926-captured-secret-refs branch October 13, 2026 09:51
jonahgabriel added a commit that referenced this pull request Oct 13, 2026
…dger check (#2715)

* ci(OMN-20708): add claim_source input and work-ledger check reader

The reusable workflow gains a claim_source input with values file (default) and work-ledger. With work-ledger, the job reads the check "branch-claim / work-ledger" posted on the pull request head by the GitHub App onexbot-occ-writer (omnimarket's node_branch_claim_check_effect, omnimarket#3547). It opens no database connection and checks out no ledger. The job waits up to 900 seconds (input lab_check_wait_seconds). This change is the earlier draft omniclaude#2608 rebased onto dev. The folded branch-claim-gate.yml no longer exists, so its wiring is now in ci.yml.

The caller branch-claim-check.yml names claim_source: file explicitly and gains checks: read. It stays on file until a completed lab check is visible on an omniclaude pull request head; none was visible on omniclaude#2708, #2710, #2711 or #2672 on 2026-10-13. tests/scripts/test_branch_claim_check_workflow.py runs the reader's own Bash with a fake gh and real jq. It no longer skips when jq is missing; it fails. New named cases: ledger_absent_passes, work_ledger_source_runs_no_ledger_step, claimed_elsewhere_refused, branch_claim_check_workflow_is_wired. The test is a named step "Branch-claim check workflow reader (OMN-20708)" of the Branch Claim Gate job in ci.yml, and a check-only pre-commit hook branch-claim-check-workflow. The Branch Claim Gate path filter now names the caller, the test and .pre-commit-config.yaml. contracts/OMN-20708.yaml added with three acceptance criteria and their dod_evidence. 38 tests in the file pass. Not changed: scripts/branch_claim.py, the pre-push hook, required-checks.yaml, branch protection.

* fix(OMN-20708): add paths and trust boundary comment

The CI check "Tests (Split 2/2)" failed on head 822752b at tests/workflows/test_blocking_validation_workflows.py::test_filter_is_the_original_trigger[branch-claim-gate]. The PR widened the Branch Claim Gate path filter in ci.yml with three paths: tests/scripts/test_branch_claim_check_workflow.py, .github/workflows/branch-claim-check.yml, and .pre-commit-config.yaml. This commit adds those three paths to the test's FOLDED_JOBS entry for branch-claim-gate, in the filter's order.

The Hostile Reviewer gate blocked on one finding agreed by two voters: that the lab check outcome is spoofable via the check-runs API. This commit adds a comment above the "Read the lab branch-claim check for this head" step in branch-claim-check-reusable.yml stating the trust boundary: GitHub sets a check run's app from the creating token, so only the named App's installation token can publish a run the step accepts; GITHUB_TOKEN publishes as github-actions. The comment also notes jq @uri escapes & = # in the check name. No behaviour change in the workflow.

Focused tests: 71 passed in test_blocking_validation_workflows.py (branch-claim/folded selection), 38 passed in test_branch_claim_check_workflow.py.
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.

1 participant