Skip to content

fix(OMN-15058): split secret-shaped test fixtures to defeat GH scanner false-positives - #1932

Merged
jonahgabriel merged 1 commit into
devfrom
jonah/omn-15058-reshape-secret-scan-fixtures-omniclaude
Jul 25, 2026
Merged

jonahgabriel merged 1 commit into
devfrom
jonah/omn-15058-reshape-secret-scan-fixtures-omniclaude

Conversation

@jonahgabriel

@jonahgabriel jonahgabriel commented Jul 25, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • 3 of omniclaude's 4 remaining open GitHub secret-scanning alerts point at test fixtures in tests/hooks/test_schemas.py that exercise prompt_preview redaction (Stripe live secret key, Stripe live restricted key, MongoDB Atlas-shaped URI). All three values are sequential-alphabet / never-issued placeholders, but GitHub's partner-pattern scanner is a raw-text structural regex with no entropy check, so it matches regardless of how fake the content looks.
  • Splits each fixture literal (string concatenation) so the full secret-shaped run never appears contiguous in the source bytes, while the runtime-reconstructed value is byte-identical and still exercises the same redaction code path. Also swaps the sequential-alphabet suffix for an explicit TESTFIXTURE...NEVERISSUED marker.
  • No assertion was weakened. This PR does not touch toggle-claude-provider.sh / claude-providers.json / agents/parallel_execution/HANDOFF.md (the real, already-resolved google_api_key credential, alert Add Claude Code GitHub Workflow #2) or the still-open, still-real slack_incoming_webhook_url alert (Phase 3: Local Template System Implementation #5) — those stay untouched/open per OMN-15058.

Test plan

  • uv run pytest tests/hooks/test_schemas.py -q — 114 passed (same 114 as before; redaction assertions for all 3 touched fixtures still pass)
  • uv run ruff format --check / uv run ruff check clean on the touched file
  • uv run mypy tests/hooks/test_schemas.py — 57 pre-existing errors, unrelated to this diff, unchanged before/after (verified via git stash comparison)

Closes OMN-15058

Evidence-Ticket: OMN-15058
Evidence-Source: OCC#4752
Evidence-Commit: d1c2f1d387bf855d4e1f150f700efc0984da2a40

…r false-positives

Three test fixtures in tests/hooks/test_schemas.py structurally matched
GitHub secret-scanning provider patterns (stripe_api_key, stripe_live_restricted_key,
mongodb_atlas_db_uri_with_credentials) despite being sequential-alphabet /
never-issued placeholder values used only to exercise prompt_preview
redaction. GitHub secret scanning is a raw-text regex scanner with no
entropy/semantic check for these partner-pattern types, so any string of
the required shape (prefix + N alnum chars, or full connection-string
shape) matches regardless of how obviously fake the content is.

Fix: split each fixture literal so the full secret-shaped run never
appears contiguous in the source text (string concatenation), while the
runtime-reconstructed value is unchanged and still exercises the same
redaction code path. Also swapped the random-looking suffix for an
explicit TESTFIXTURE...NEVERISSUED marker.

No assertions were weakened -- all 114 tests in the file still pass,
ruff format/check clean, mypy delta is zero (57 pre-existing unrelated
errors, unchanged before/after this diff).

OMN-15058
@coderabbitai

coderabbitai Bot commented Jul 25, 2026

Copy link
Copy Markdown

Warning

Review limit reached

You’ve reached a temporary PR review limit under our Fair Usage Limits Policy.

Your recent review volume is higher than typical usage, so adaptive limits are currently applied.

Next review available in: 48 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 31fd583c-62a9-4cbe-90d2-2e59cd7672c5

📥 Commits

Reviewing files that changed from the base of the PR and between df0a627 and 1c46cb9.

📒 Files selected for processing (1)
  • tests/hooks/test_schemas.py
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch jonah/omn-15058-reshape-secret-scan-fixtures-omniclaude

Comment @coderabbitai help to get the list of available commands.

jonahgabriel added a commit to OmniNode-ai/onex_change_control that referenced this pull request Jul 25, 2026
@github-actions

Copy link
Copy Markdown
Contributor

⚠️ Hostile Reviewer — DEGRADED (informational)

Blocking findings (critical/error): 0
Total findings: 0
Models succeeded: none

Note: All reviewer models failed or were unavailable. Degraded results are informational during the pilot phase (OMN-8524) and do not block merge. Error: cli_review exited non-zero (all models failed or binary error)


Gate semantics (pilot phase)

Verdict Meaning Blocks merge?
passed No critical/error findings No
blocked CRITICAL or ERROR findings found Yes
degraded All models unavailable (infra) No (pilot)

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

@jonahgabriel
jonahgabriel merged commit a15351c into dev Jul 25, 2026
303 of 357 checks passed
@jonahgabriel
jonahgabriel deleted the jonah/omn-15058-reshape-secret-scan-fixtures-omniclaude branch July 25, 2026 05:47
jonahgabriel added a commit that referenced this pull request Oct 11, 2026
…_core#1932 and verifier 0.4.310 (#2678)

* ci(OMN-20074): re-pin the omniclaude repo-evidence caller to omnibase_core#1932 and verifier 0.4.310

.github/workflows/call-repo-evidence-gate.yml moves the reusable
pin from fb0c6c2117d5868a398b0920cd0048d0824415b1
(omnibase_core#1914) to 4e4f5e0404d364e296c5d37db6aa3ab9e07f8ffa
(omnibase_core#1932). This pin matches the one omnimarket's caller
uses and descends from #1914. The verifier-version changes from
0.4.305 to 0.4.310. The shadow and compare-with-occ settings stay
"true".

At #1914 with 0.4.305, the reusable hands neither a .claude-plugin
manifest version bump nor an omnibase_core checkout pin bump by the
writer app to a classifier. #1932 carries #1927 (workflow-pin) and
#1931 (plugin version bump) classification and the VERIFIER_FLOOR
0.4.303 refusal. omnimarket 0.4.310 is the first release shipping
classify_plugin_manifest_version_only and
classify_workflow_core_pin_only (omnimarket#3694). It also carries
omnimarket#3751, where node_dod_verify refuses a contract YAML with a
duplicate mapping key.

Before re-pinning, all 171 contracts/*.yaml on dev 66bd5bd loaded
with omnibase_core's load_yaml_mapping_no_duplicates (the 0.4.310
loader): 0 refused. A planted duplicate was refused (positive
control).

tests/ci/test_repo_evidence_caller_workflow.py adds
test_caller_runs_the_writer_app_classifiers_of_verifier_0_4_310,
test_every_repo_contract_loads_without_a_duplicate_key, and
test_duplicate_key_scan_names_a_planted_duplicate. The fleet pin
constant moves to 4e4f5e04.

contracts/OMN-20074.yaml: AC1 names the new pin; new AC6 is bound by
dod-omn20074-writer-app-classifiers.

Focused run: 12 passed. On the dev caller (before the workflow
change) 2 failed: the another_repo pin test and the writer_app test.

* fix(OMN-20074): sync onex_state.py with plugin copy

Tests on omniclaude PR #2678 head dadd94d failed 2 tests:
tests/unit/hooks/test_hook_emit_drainer.py::test_drainer_applies_declare
d_lane_when_env_is_empty (AttributeError: module 'onex_state' has no
attribute 'workspace_config_file') and
test_declared_lane_beats_a_disagreeing_ambient_env (assert None is not
None).
Cause: OMN-20937 (dev commit 2b12230, #2671) added
workspace_config_root, workspace_config_file and
missing_workspace_config_message to
plugins/onex/hooks/lib/onex_state.py but not to its copy
src/omniclaude/hooks/lib/onex_state.py.
tests/unit/hooks/test_test_offloader.py puts src/omniclaude/hooks/lib
on sys.path, so in that split hook_edge_lane's bare "import onex_state"
resolves to the stale src copy.
Change: src/omniclaude/hooks/lib/onex_state.py now matches the plugin
copy byte for byte (62 lines added, nothing removed).
Verification: split 1 run locally (--splits 2 --group 1, same paths as
CI) reproduced both failures before the change; after it both pass.
ruff, ruff format and mypy are clean on the file.
jonahgabriel added a commit that referenced this pull request Oct 12, 2026
… S5 bar) (#2614)

* ci(OMN-20074): re-pin the omniclaude repo-evidence caller to omnibase_core#1932 and verifier 0.4.310

.github/workflows/call-repo-evidence-gate.yml moves the reusable
pin from fb0c6c2117d5868a398b0920cd0048d0824415b1
(omnibase_core#1914) to 4e4f5e0404d364e296c5d37db6aa3ab9e07f8ffa
(omnibase_core#1932). This pin matches the one omnimarket's caller
uses and descends from #1914. The verifier-version changes from
0.4.305 to 0.4.310. The shadow and compare-with-occ settings stay
"true".

At #1914 with 0.4.305, the reusable hands neither a .claude-plugin
manifest version bump nor an omnibase_core checkout pin bump by the
writer app to a classifier. #1932 carries #1927 (workflow-pin) and
#1931 (plugin version bump) classification and the VERIFIER_FLOOR
0.4.303 refusal. omnimarket 0.4.310 is the first release shipping
classify_plugin_manifest_version_only and
classify_workflow_core_pin_only (omnimarket#3694). It also carries
omnimarket#3751, where node_dod_verify refuses a contract YAML with a
duplicate mapping key.

Before re-pinning, all 171 contracts/*.yaml on dev 66bd5bd loaded
with omnibase_core's load_yaml_mapping_no_duplicates (the 0.4.310
loader): 0 refused. A planted duplicate was refused (positive
control).

tests/ci/test_repo_evidence_caller_workflow.py adds
test_caller_runs_the_writer_app_classifiers_of_verifier_0_4_310,
test_every_repo_contract_loads_without_a_duplicate_key, and
test_duplicate_key_scan_names_a_planted_duplicate. The fleet pin
constant moves to 4e4f5e04.

contracts/OMN-20074.yaml: AC1 names the new pin; new AC6 is bound by
dod-omn20074-writer-app-classifiers.

Focused run: 12 passed. On the dev caller (before the workflow
change) 2 failed: the another_repo pin test and the writer_app test.

* ci(OMN-20074): omniclaude S6 cut-over, delete the OCC callers and enforce repo evidence

Stacked on the re-pin commit dadd94d (omnibase_core#1932 reusable,
verifier 0.4.310), which is open as omniclaude#2678.

Deleted workflows: call-occ-preflight.yml, call-occ-autobind.yml,
call-occ-companion-effect.yml, call-receipt-gate.yml,
occ-companion-merge-heal.yml (the dev-head monitor already lives in
auto-merge-stale-poller.yml). Kept call-occ-autobind-reusable.yml and
call-occ-companion-effect-reusable.yml because other repositories call
them.

Removed the nested occ-preflight job and its needs/if edges from 29
workflow files, and the step-level needs.occ-preflight reads in
ci.yml (tests-gate, security-gate), hostile-reviewer.yml and
omni-standards-compliance.yml.

ci.yml: deleted jobs dod-evidence-check (DoD Evidence Check, resolved
the OCC Evidence-Source companion), contract-compliance (Contract
Compliance Check, checked out onex_change_control) and
occ-companion-merged; deleted 7 scripts under scripts/ci used only by
them or by the heal workflow, their tests, and their 7 lines in
.onex_ratchets/canonical_file_shape_baseline.txt.

scripts/ci/ci_summary_gate.py: no OCC context in GATE_JOBS,
STRICT_SUCCESS_JOBS, SOFT_ALLOWLIST, ALL_MUST_SUCCEED_EXTERNAL_NAMES
(now empty) or the sweep exclusions; EXPECTED_EXTERNAL_CONTEXTS gains
repo-evidence / dod-verify and the check names of seven standalone
gates that were enforced only through the occ-preflight required
context in their files: check-handshake.yml,
concurrency-primitive-guard.yml, dep-provenance-gate.yml,
imperative-contract-guard.yml, stale-todo-gate.yml,
validate-pr-contracts.yml, validate-validator-requirements.yml.

.github/required-checks.yaml drops occ-preflight / eligibility, DoD
Evidence Check and verify / verify; repo-evidence / dod-verify gets
no row (a dev ruleset is to require it).

call-repo-evidence-gate.yml: shadow "false", compare-with-occ
"false".

contracts/OMN-20074.yaml gains AC7 bound to
tests/ci/test_no_occ_callers_omn20074.py (8 tests);
contracts/OMN-20073.yaml AC2 and AC4 re-pointed so no falsifier
selects zero tests; contracts/OMN-20768.yaml evidence command drops a
deleted test file.

scripts/public_skill_hygiene_ratchet.yaml host-nick budget 92 -> 87
(the deleted files carried the residue).

tests/ci/test_no_occ_callers_omn20074.py: 8 failed on dadd94d, 8
passed after. Changed test files plus
tests/scripts/test_advisory_job_gate.py: 391 passed.
advisory_job_gate.py with the projected dev required contexts (66
minus occ-preflight / eligibility and DoD Evidence Check, plus
repo-evidence / dod-verify): PASS, 0 findings.

Lands only together with the S6 dev protection change; the PR stays a
draft.

Mechanical edits drafted by Codex, reviewed and completed in the
lane.

* test(OMN-20074): the OMN-20975 precursor test reads the S6 caller shape

Tests (Split 1/2) was red at head 07982b1 because the test expected the caller to
differ from merge base 3228efa only in compare-with-occ, with shadow "true"
and the S5 comment block kept. This PR is the S6 cut-over that the precursor
comment says sets shadow "false" and replaces the S5 block. The test now
expects shadow "false" and the S6 comment block; every other caller line and
comment still must match the merge base. No workflow or product file changed.
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