fix(queue-health): enforce canonical repository identity - #2268
seonghobae wants to merge 6 commits into
Conversation
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: trueThanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Exact-head RED authority for |
seonghobae
left a comment
There was a problem hiding this comment.
Exact-head review: this is a valid source-level RED. scripts/ci/actions_queue_health_core.py still admits ContextualWisdomLab/repository. and ContextualWisdomLab/repo..name because REPOSITORY_PATTERN allows dots anywhere inside the repository component and _repository_name() only rejects components exactly equal to . or ... The new production-path regression through load_allowlist() therefore exercises a real admission defect rather than a replacement-regex assertion. Minimum causal repair remains the repository-component predicate only; preserve the queue-health read-only boundary, allowlist semantics, pagination/identity fail-closed behavior, and existing security/quality gates. Current Security Scan, SAST, CodeQL PR, and Python Security runs are still queued, so no hosted GREEN is inferred from this review.
seonghobae
left a comment
There was a problem hiding this comment.
Follow-up finding: the new RED covers non-canonical repository-name components but not the owner component, while _repository_name() validates the generic owner/repository identifier and the same permissive character pattern applies on both sides of /. Current production therefore also admits ContextualWisdomLab./repository and Contextual..WisdomLab/repository. Add those hostile witnesses before the production repair so the fix cannot harden only the right-hand component. This remains test-scope RED; no gate weakening or production broadening is needed.
|
Cross-owner contract alignment: |
seonghobae
left a comment
There was a problem hiding this comment.
Exact-head source review: 142e5b2617778e79f665693be1e6f8c04d7533aa is the minimum causal production repair for the established RED. The candidate was constructed against c754daf..., inspected before ref movement, and compares as ahead 1 / behind 0 with exactly one production file and one changed line (+1/-1). _repository_name() now applies the component invariant symmetrically after the existing full-name grammar: any owner/repository component containing .. or ending in . is rejected, while .github and ordinary interior single dots remain admissible. No queue-health collection, pagination, exact-head identity, or read-only behavior changed. This review does not transfer predecessor evidence or claim hosted GREEN: the naturally-created Security Scan, SAST, Python Security, and CodeQL PR runs for this exact head are still queued, and qualifying independent review remains required before merge.
|
Exact-head hosted RCA for |
|
Fresh RCA: SAST run |
seonghobae
left a comment
There was a problem hiding this comment.
Fresh consumer-side prerequisite audit: this owner lane remains source-stable, but the PR body’s hosted-evidence paragraph is stale. Exact-head Security Scan 35352222783 is terminal success; Python Security 35352222876, SAST 35352222809, and CodeQL PR 35352222726 are terminal failures, not queued. Keep Draft and do not transfer Security Scan success across the failed gates or any future reconciliation. Shared #2279 foundation has also advanced ordinarily to a newer exact head and is still pending its own hosted acceptance. Any later adoption into scheduler owner #2040 must remain path-wise and preserve this repaired queue-health identity slice without copying mutable heads wholesale.
seonghobae
left a comment
There was a problem hiding this comment.
Fresh owner-lane evidence update on exact 142e5b2617778e79f665693be1e6f8c04d7533aa: the PR body still describes the hosted security lanes as queued, but they are terminal. Security Scan 35352222783 is SUCCESS; Python Security 35352222876, SAST Semgrep 35352222809, and CodeQL PR 35352222726 are FAILURE. Python Security reached real gates rather than failing at setup: pip-audit fails at Run pip-audit (hard gate on any known vulnerability), while Bandit successfully produces/uploads SARIF and then fails Enforce bandit gate (fail on MEDIUM+ findings). Treat these as RCA/fix inputs, not rerun noise; do not call #2268 accepted or transfer this source slice into #2040 as GREEN until the owner failures are causally repaired and exact-head evidence is reacquired.
seonghobae
left a comment
There was a problem hiding this comment.
P1 dependency/single-writer finding on exact head 142e5b2617778e79f665693be1e6f8c04d7533aa.
The terminal security failures are not caused by this PR's two queue-health files. SAST run 35352222809, job 105622840267 reports exactly two unsuppressed dynamic-urllib-use-detected findings at scripts/ci/codeql_ghas_configuration_identity.py:158 and scripts/ci/strix_evidence_binding.py:264. Python Security run 35352222876, Bandit job 105746160782 independently reports two MEDIUM/HIGH-confidence B310 findings at the same dynamic URL-open boundary.
Canonical owner #2279 exact d1e4380c15e948aaf104d46aa134fa614058782a changes those exact production files and adds the production-opener authority/redirect fixtures plus gap-baseline doctoring. This head and #2279 are currently diverged (ahead 4 / behind 32, merge-base 64aa08d7fa487deacd41c761c36277ca68cab6c9), so #2268 cannot inherit that repair or any hosted evidence yet.
Acceptance order: settle #2279 normally first; then non-force restack this unique queue-health identity delta onto the accepted foundation, preserving only scripts/ci/actions_queue_health_core.py and tests/test_actions_queue_health_repository_identity.py; update the PR/ADR-gap dependency record; and reacquire exact-head SAST/Bandit/CodeQL/quality evidence. Do not copy the shared URL clients, transfer #2279 receipts, or rerun this unchanged divergent head. The pip-audit failure is a separate terminal owner input and is not claimed repaired by this dependency finding.
seonghobae
left a comment
There was a problem hiding this comment.
Exact-head owner-integration review for 7822a7f077d2ebf07c579c855da9d6837a10b1ed (COMMENT, not approval).
The prior exact-head SAST/Python Security failures were independently reproduced as the two inherited dynamic urllib boundaries in the CodeQL GHAS identity and Strix evidence clients. Canonical owner PR #2279 repairs those boundaries with fail-closed HTTPS authority validation, no-redirect production openers, hostile fixtures, and published-lineage evidence. This branch now preserves the complete current owner head d1e4380c15e948aaf104d46aa134fa614058782a as the second parent of an ordinary merge; it is retargeted to fix/github-api-url-boundary, so the effective consumer delta remains only:
scripts/ci/actions_queue_health_core.pytests/test_actions_queue_health_repository_identity.py
Exact tree 4c75b6fc733b20acc42f3082afbcb20aa089b7e8 verification:
- focused queue-health + CodeQL/Strix URL-authority suites: 174 passed
- same focused suite with
GITHUB_ACTIONS=true: 174 passed - full
GITHUB_ACTIONS=truewarnings-as-errors suite: 3379 passed, 28 skipped, 40 subtests - compileall and diff check: PASS
- protected
maincomparison at verification: 37 ahead / 0 behind - unresolved review threads: 0
The hosted exact-head CodeQL, Runtime Quality, Security Scan, SAST, and Python Security runs are newly queued. No qualifying current-head approval exists, so this evidence does not authorize Ready or merge.
Preserve the reviewed queue-health tree byte-for-byte while recording the protected #2279 merge commit as current ancestry.
seonghobae
left a comment
There was a problem hiding this comment.
Exact-head protected-base reconciliation review at 7d8a2a823f8dff523d749eb56965b001f4be53b5 (COMMENT, not approval).
The prior head 7822a7f... preserved canonical owner d1e4380c..., but after #2279 merged the live protected base became main@e6334e229581a918e2f22de18733b76fa65d7e71. Fresh compare showed the prior head was ahead 5 / behind 1 with merge base d1e4380c...; therefore its queued runs could not prove current protected-base ancestry.
This ordinary two-parent successor records the prior head and protected merge commit as parents while preserving tree 4c75b6fc733b20acc42f3082afbcb20aa089b7e8 byte-for-byte. Pre-ref checks established:
- predecessor → successor: no file changes;
- protected main → successor: ahead 6 / behind 0, merge base exactly protected main;
- effective diff remains exactly
scripts/ci/actions_queue_health_core.pyandtests/test_actions_queue_health_repository_identity.py.
Fresh CodeQL 35453297707, Python Security 35453297725, SAST 35453297864, and Security 35453297869 are queued/nonterminal. Keep Draft until terminal exact-head hosted evidence and a qualifying independent approval; no predecessor result transfers.
Current authority — 2026-09-20 KST
7d8a2a823f8dff523d749eb56965b001f4be53b5; tree is byte-identical to the reviewed predecessor at4c75b6fc733b20acc42f3082afbcb20aa089b7e8.main@e6334e229581a918e2f22de18733b76fa65d7e71.7822a7f077d2ebf07c579c855da9d6837a10b1edand the protected fix(security): prove GitHub API URL authority #2279 merge commite6334e229581a918e2f22de18733b76fa65d7e71.Superseded authority record
Problem
Queue-health repository admission was looser than the canonical component invariant. The protected-main parser accepted owner/repository components containing embedded
..or ending in.becauseREPOSITORY_PATTERNallowed those characters and_repository_name()rejected only components exactly equal to.or...Tracks #2263. This lane owns only queue-health admission; scheduler reconciliation remains owned by #2040.
RED contract
The production
load_allowlist()regression requires rejection of:ContextualWisdomLab/repository.ContextualWisdomLab/repo..nameContextualWisdomLab./repositoryContextual..WisdomLab/repositoryContextualWisdomLab/..ContextualWisdomLab/.while preserving
.github, interior single dots, underscores, and hyphens.Reviews
5245968191and5245980104record the repository-side RED and symmetric owner-side gap.Production repair
Current exact head:
7822a7f077d2ebf07c579c855da9d6837a10b1ed.Exact tree:
4c75b6fc733b20acc42f3082afbcb20aa089b7e8.The queue-health delta changes one production predicate in
scripts/ci/actions_queue_health_core.pyand addstests/test_actions_queue_health_repository_identity.py. After the existing one-slash/allowed-character grammar succeeds, each owner/repository component is rejected when it contains..or ends in.. Valid leading single-dot names such as.githubremain admissible.Canonical security-owner integration
The earlier exact SAST and Python Security failures were not caused by the queue-health delta. They identified two inherited dynamic
urllibboundaries in:scripts/ci/codeql_ghas_configuration_identity.pyscripts/ci/strix_evidence_binding.pyPR #2279 owns that repeated GitHub REST authority responsibility. Ordinary merge commit
7822a7f077d2ebf07c579c855da9d6837a10b1edpreserves both complete histories with parents:142e5b2617778e79f665693be1e6f8c04d7533aa;d1e4380c15e948aaf104d46aa134fa614058782a.This PR is stacked on
fix/github-api-url-boundaryso its effective delta remains the two queue-health files. It does not copy or fork the owner implementation.Exact-tree verification
GITHUB_ACTIONS=true: 174 passedGITHUB_ACTIONS=truesuite with warnings-as-errors: 3379 passed, 28 skipped, 40 subtestscompileall: PASSgit diff --check: PASSmaincomparison at verification: 37 ahead / 0 behindAcceptance
No force-push, destructive rebase, unchanged-head rerun, source-neutral wake commit, scanner suppression, self-approval, or gate weakening.