ci: gate what a package's registry page will actually show - #2261
seonghobae wants to merge 32 commits into
Conversation
fast-mlsirm 0.11.3's published PyPI description carried the internal commercial boundary: an enterprise sales gate, a KRW 2,000,000,000 product gate, buyer and procurement evidence links, and 16 repo-relative links that are 404s on the registry page because only LICENSE and the package sources ship. Checking the live sdist finds 68 such findings; threadweave 0.1.0 and rankweave 0.1.0 carry 3 each, one of them a source module path. The gate reads the description a registry renders - PKG-INFO from an sdist, METADATA from a wheel - rather than the README on disk, because a release is built from a commit and packaging config decides what is included. A repository with no release yet can point it at the README instead. A repository README may link internal design records; that is public development history. The same text on a package page is different, because only the distribution's files exist there and the reader is installing rather than developing. So docs/adr links stay legitimate and only have to be absolute, while docs/superpowers, docs/product, docs/commercial, docs/planning and docs/doctoring are findings. Hard-coded deal values follow the rule the organization already set in fast-mlsirm's acquisition_readiness_gate doctoring record: product quality evidence must not depend on a monetary target. Product vocabulary that merely looks commercial is deliberately not a finding. appguardrail ships a real buyer-diligence subcommand and scopeweave really does check procurement packages; the rule targets internal framing, not a domain. --allow lets a repository adopt the gate before its README is fully converted instead of landing a red check it cannot fix in one PR. No repository calls this yet, so this PR cannot break one. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YHVBDaZS5NZT9aQcbRg9Av
|
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 |
seonghobae
left a comment
seonghobae
left a comment
There was a problem hiding this comment.
두 개의 exact-head acceptance defect가 있습니다.
P0 — reusable workflow가 central gate를 caller SHA로 checkout합니다. 현재 package-description-boundary.yml은 repository: ContextualWisdomLab/.github와 함께 ref: ${{ github.workflow_sha }}를 사용하고, PR 본문도 이것이 caller가 uses: ...@<commit-sha>로 pin한 central workflow revision과 같다고 설명합니다. GitHub 공식 reusable-workflow 계약은 반대입니다. called workflow 안에서도 github context는 caller workflow에 연결됩니다. 현재 GitHub는 이 구분을 위해 job.workflow_sha / job.workflow_repository를 별도로 제공합니다. 즉 타 repository caller에서 github.workflow_sha는 caller workflow file의 commit이고, 그 SHA를 ContextualWisdomLab/.github에서 checkout하면 보통 존재하지 않아 첫 adoption부터 fail합니다.
공식 근거: https://docs.github.com/en/actions/reference/workflows-and-actions/reusing-workflow-configurations#github-context (github context is always associated with the caller workflow), https://docs.github.com/en/actions/reference/workflows-and-actions/contexts (job.workflow_sha = commit SHA of the workflow file that defines the current job; reusable jobs에서 called workflow를 가리킴).
RED: 다른 CWL repository의 thin caller가 이 workflow를 exact SHA로 호출하는 integration fixture를 두고, central-gate checkout이 그 pin과 동일한 .github SHA를 가져오는지 assert해 주세요. 현재 구현은 caller SHA를 .github ref로 사용하므로 RED여야 합니다. GREEN은 central gate checkout authority를 job.workflow_sha에 묶고, 가능하면 job.workflow_repository도 expected owner와 일치하는지 fail-closed 검증한 뒤 실제 cross-repo caller에서 script/test artifact까지 실행되는 것입니다. github.sha/github.workflow_sha fallback으로 조용히 caller revision을 받으면 안 됩니다.
P1 — registry-safe absolute URL이 release-safe인지는 검사하지 않습니다. 이 gate는 absolute GitHub URL이면 clean으로 취급합니다. 바로 지금 RankWeave#65@acc6eaaef1ba57bfb33855098e29ac418e9edacd와 EgressWeave#249@803e939a51e9c666ea09c781f59a14436e0e11a3가 package README의 contract 문서를 /blob/main/·/tree/main/ 절대 URL로 바꾸고 있습니다. 상대 링크 404는 없어지지만 이미 배포된 wheel/sdist의 registry page가 이후 main 변경에 따라 다른 CLI/schema/security/release 계약을 가리키게 됩니다. immutable artifact가 mutable documentation authority를 소비하는 셈입니다.
RED: built PKG-INFO/METADATA에서 동일 repository의 mutable branch ref (main/develop/master, 최소한 blob|tree)를 release-contract URL로 넣으면 실패시키고, released version의 문서 identity가 이후 branch 이동과 무관함을 검증해 주세요. GREEN은 built artifact 단계에서 v${version} 또는 exact release commit 등 immutable ref로 pin하고 version/tag/commit 불일치를 release failure로 연결하는 것입니다. repository README 자체의 개발용 상대 링크는 유지할 수 있으므로, source README UX와 registry artifact contract를 분리하는 쪽이 안전합니다.
추가로 load_description()이 dist 디렉터리에서 sdist를 우선해 첫 candidate 하나만 검사하므로 caller가 wheel+sdist를 만들 때 두 artifact의 description parity도 현재는 증명하지 않습니다. 이 PR의 이름이 'registry page will actually show'인 만큼 최종 GREEN은 업로드 대상 모든 distribution metadata를 검사하고 description divergence도 fail closed하는 것이 맞습니다.
The first run against the three repositories that had just been corrected produced 16, 1 and 2 findings, and nearly all of them were wrong. contextual-orchestrator genuinely ships /api/v1/commercial_readiness/latest, /api/v1/saleability_decisions/latest and /api/v1/commercial_due_diligence_rooms/latest, with tests named after them; wardnet's crate genuinely computes commercial readiness snapshots; semantic-data-portal's only finding was the sentence explaining that PRD/TRD records are excluded. The exception for product domain vocabulary was stated in the prose and absent from the regex. So go-to-market-vocabulary and requirement-map now report without failing, and --strict promotes them for a repository that wants them enforced. The mechanical rules keep blocking because they need no judgement: a relative link is dead on the registry page, a quoted module path is plumbing, and a hard-coded deal value is never a product feature. The same pass showed ADRs filed under docs/planning/adrs/ being caught by the docs/planning/ prefix. An ADR is a public design record wherever a repository files it, so the path check exempts it and a test pins both directions. Verified after the change: the live fast_mlsirm-0.11.3.tar.gz still reports 22 blocking findings and 63 under --strict, the corrected fast-mlsirm README is clean, and the three corrected READMEs pass with advisory notes only. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YHVBDaZS5NZT9aQcbRg9Av
pg-llm-batch's README links docs/doctoring/bootstrap-dsn-precedence.md, cli-secret-input.md and postgres-logical-restore.md, and those are operator documentation a package user needs. That repository files operational guidance under the same directory name this one uses for incident records. A worker removed the links to satisfy the gate and restored them in the next commit because they were legitimate - the gate causing damage rather than preventing it. So internal-working-record advises. Nothing is lost: a link into such a directory that is also repo-relative is still blocked by relative-link, which is the mechanical defect, since the page cannot resolve it. An absolute URL to the same file resolves, and whether that audience wants it is judgement. That is now the line across both corrections in this branch: block only what is broken regardless of context, advise on anything that needs to know what the product is. Re-verified: the live fast_mlsirm-0.11.3.tar.gz still reports 22 blocking findings, OriginWeave's seven repo-relative links still block, pg-llm-batch's corrected README passes with 18 advisory notes, and the corrected fast-mlsirm README is clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YHVBDaZS5NZT9aQcbRg9Av
The first revision of this reusable workflow took a free-form `build-command`
string input and interpolated it straight into a `run:` block. A caller could
have passed arbitrary shell into a workflow running in its own repository
context, and it is what ADR 0023 forbids outright: reusable inputs are data and
capability flags, not shell source.
The contract test for the fast-mlsirm reusable workflow in this same branch
series asserts that no input reaches a `run:` body. I did not apply that rule
here and did not notice until the org's own Semgrep gate failed the PR with
yaml.github-actions.security.run-shell-injection. The gate was right.
The input is now `build: sdist | wheel | none`, mapped to a fixed `python -m
build` invocation and validated in-shell so an unexpected value exits 2 rather
than silently building nothing. `dist-path` is passed through env to that step
instead of being read from a scope where it was undefined. A contract test pins
the absence of `build-command` and the absence of any `${{ inputs.* }}` inside
any `run:` body.
Local Semgrep on this file after the change: 0 findings. Contract tests: 19
passed.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YHVBDaZS5NZT9aQcbRg9Av
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
![cwl-noema-review[bot]](https://avatars.githubusercontent.com/u/295022177?s=60&v=4){kind=small}
There was a problem hiding this comment.
Noema LLM review
The report/status wiring looks intentional, but the new regression test is too weak: it only asserts the status field, even though its name says it checks that every finding is recorded.
Reviewed changed lines
.github/workflows/package-description-boundary.yml:97 (RIGHT): This test only checksreport["status"] == "failed". It should also verify that each expected finding appears in the report's findings data. Otherwise the report can drop or alter findings while the test still passes and CI stays green.
Adversarial validation
.github/workflows/package-description-boundary.yml:97 (RIGHT)confirmed: The implementation can return an emptyfindingslist while still setting the top-level status to"failed". — A test that records every finding should fail when the findings list is emptied. Since this test continues to pass with no findings, it gives false confidence about finding propagation..github/workflows/package-description-boundary.yml:97 (RIGHT)confirmed: A non-blocking finding that is silently dropped from the output will not change the asserted status, so it will not be caught. — The test name promises that every finding is recorded, but the assertion only observes the aggregate status string. The untested part of the public contract is the findings list itself.- Residual risk: The documentation/comment surrounding the reusable-workflow usage should be double-checked: if it shows
uses:directly under an event trigger, that snippet will not work as written.
Findings
- [medium] .github/workflows/package-description-boundary.yml:97 (RIGHT): The test only asserts
report["status"] == "failed". The name says it verifies that every finding is returned, so please extend the test to assert the full findings list (paths, messages/levels, and count) in addition to the status. This is the test that should prevent regressions in finding propagation.
- Result: REQUEST_CHANGES
- Head SHA:
22b16e6c6881286ed0f4c289e2a5308f2c81bedb - Reviewer credential:
noema-review-github-app-refresh - Actor:
cwl-noema-review[bot]
Clearing the Semgrep rule on these two call sites left Bandit's B310 firing on them, so `main` would still have been red after this PR merged and every PR here would still have inherited a failing required check -- just a different one. The failure on #2261 is exactly this: two B310 hits, no Semgrep hits. B310 is an AST check for `urlopen` with an unproven scheme. It cannot see `_require_github_api_url`, which is what actually answers it, so the suppression goes inline on the call line while the justification and the Semgrep suppression stay on the lines above. The hardening is still the reason both are allowed; neither replaces it. `bandit -ll` on both files: no issues identified, 2 suppressed. `semgrep --config=p/default --severity=WARNING --severity=ERROR` on scripts/ci/: 0 findings. 57 tests pass. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YHVBDaZS5NZT9aQcbRg9Av
Clearing the Semgrep rule on these two call sites left Bandit's B310 firing on them, so `main` would still have been red after this PR merged and every PR here would still have inherited a failing required check -- just a different one. The failure on #2261 is exactly this: two B310 hits, no Semgrep hits. B310 is an AST check for `urlopen` with an unproven scheme. It cannot see `_require_github_api_url`, which is what actually answers it, so the suppression goes inline on the call line while the justification and the Semgrep suppression stay on the lines above. The hardening is still the reason both are allowed; neither replaces it. `bandit -ll` on both files: no issues identified, 2 suppressed. `semgrep --config=p/default --severity=WARNING --severity=ERROR` on scripts/ci/: 0 findings. 57 tests pass. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YHVBDaZS5NZT9aQcbRg9Av
seonghobae
added
enhancement
ChatGPT Codex Connector
Validate release_version before GITHUB_ENV, refuse synthesized empty API evidence, pin pure-python build via require-hashes, and document caller contents/id-token grants. Defers #2261 registry-description gate. Co-authored-by: Cursor <cursoragent@cursor.com>
![opencode-agent[bot]](https://avatars.githubusercontent.com/in/1549082?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
OpenCode reviewed the current-head product diff. Coverage is a separate gate.
Changed files
.github/workflows/package-description-boundary.yml— GitHub Actions review jobdocs/doctoring/package-description-boundary.md— operator or user guidancescripts/ci/package_description_boundary.py— review and security gate shell pathtests/test_package_description_boundary.py— regression suite
Changed behavior
flowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["Workflow: package-description-boundary.yml"]
S1 --> I1["GitHub Actions review job"]
I1 --> R1["Review risk: Workflow: package-description-boundary.yml"]
R1 --> V1["actionlint plus required checks"]
Evidence --> S2["Docs: package-description-boundary.md"]
S2 --> I2["operator or user guidance"]
I2 --> R2["Review risk: Docs: package-description-boundary.md"]
R2 --> V2["docs review"]
Evidence --> S3["CI script: package_description_boundary.py"]
S3 --> I3["review and security gate shell path"]
I3 --> R3["Review risk: CI script: package_description_boundary.py"]
R3 --> V3["bash -n plus Strix self-test"]
Evidence --> S4["Test: test_package_description_boundary.py"]
S4 --> I4["regression suite"]
I4 --> R4["Review risk: Test: test_package_description_boundary.py"]
R4 --> V4["targeted test run"]
Findings
No source-backed product finding is synthesized from the coverage gate. A coverage miss belongs in the status comment.
- Head SHA:
22b16e6c6881286ed0f4c289e2a5308f2c81bedb - Workflow run: 35381324374
- Workflow attempt: 1
- Coverage gate:
failure
Review outcome
Coverage is a gate, not the review. This body reviews the changed product files.
Changed-File Evidence Map
flowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["Workflow: package-description-boundary.yml"]
S1 --> I1["GitHub Actions review job"]
I1 --> R1["Review risk: Workflow: package-description-boundary.yml"]
R1 --> V1["actionlint plus required checks"]
Evidence --> S2["Docs: package-description-boundary.md"]
S2 --> I2["operator or user guidance"]
I2 --> R2["Review risk: Docs: package-description-boundary.md"]
R2 --> V2["docs review"]
Evidence --> S3["CI script: package_description_boundary.py"]
S3 --> I3["review and security gate shell path"]
I3 --> R3["Review risk: CI script: package_description_boundary.py"]
R3 --> V3["bash -n plus Strix self-test"]
Evidence --> S4["Test: test_package_description_boundary.py"]
S4 --> I4["regression suite"]
I4 --> R4["Review risk: Test: test_package_description_boundary.py"]
R4 --> V4["targeted test run"]
OpenCode Review Overview
Coverage evidence did not pass, so approval is blocked. The formal pull-request review is the source-backed diff review, not this status comment. |
Use the called job's workflow repository and exact workflow SHA for the central checkout, remove the unhashed runtime build install in favor of a pinned uv action and version, validate every sdist/wheel description in an upload set, and reject release-contract links to moving GitHub branches. Regression tests cover workflow identity, immutable build tooling, mutable blob/tree links, and matching/divergent artifact metadata. The operator note now matches the implemented blocking/advisory policy.
seonghobae
left a comment
seonghobae
left a comment
There was a problem hiding this comment.
Exact-head review for e7f34ea60bdc04ebb5c518c252543f9dc8acbd0d.
The reusable-workflow authority defect is repaired with called-job identity (job.workflow_repository / job.workflow_sha) and a fail-closed 40-hex check. The GHAS unhashed install is removed in favor of a commit-pinned setup-uv action plus exact uv version. Artifact-directory inspection now checks all sdist/wheel descriptions for parity, and mutable GitHub branch links are a blocking mechanical rule. Documentation now matches the implemented advisory/blocking split.
Test evidence: focused 27 passed; full GITHUB_ACTIONS=true -W error suite 3,362 passed / 28 skipped / 40 subtests; compileall and diff check PASS. Remote tree 8e3df2203e6c26421b56a0aaf762354481438e81 is byte-identical to the independently verified local tree. No additional source-backed finding remains.
This COMMENT is exact-head evidence, not self-approval. Draft remains appropriate until hosted Checks and independent approval complete.
|
Exact-head evidence receipt for
Hosted replacement evidence is pending:
The PR remains Draft/Proposed; this receipt does not substitute for independent approval or terminal hosted Checks. |
seonghobae
left a comment
seonghobae
left a comment
There was a problem hiding this comment.
Exact-head restack review for ad526e3332e6004f1babf1ec3344f833316358b2.
Protected main@e6334e229581a918e2f22de18733b76fa65d7e71 is preserved as the second parent; the prior repaired head remains the first parent. GitHub compare is 6 ahead / 0 behind and the effective diff remains exactly the four #2261 owner files.
The combined exact tree a5c6ebf779666bc534b4bb92808841739ce04c5a passed 117 focused package-boundary plus #2279 GitHub-authority contracts and the full warnings-as-errors suite: 3,396 passed / 28 skipped / 40 subtests. Remote and independently constructed local tree SHAs are identical. No new source-backed finding was introduced by the protected-main integration.
This COMMENT is exact-head evidence, not self-approval. Draft remains appropriate until replacement hosted Checks and an independent qualifying approval complete.
|
Protected-main restack receipt for exact head
Fresh hosted evidence is queued:
No Force Push, rebase, bypass, synthetic status, or predecessor-check transfer was used. Draft/Proposed remains until terminal exact-head Checks and independent approval. |
|
@coderabbitai review Please review exact head |
|
|
Exact-head authoritative-metadata repair — 2026-09-19Head: The prior parser selected sdist metadata with PyPA specifies a single top-level sdist directory containing
Exact remote source/test/workflow/docs assertions are 8/8. The exact source compiled successfully; ambiguous sdist 1/1 and ambiguous wheel 1/1 were rejected by a standard-library harness. Fresh exact-head runs are queued: Security, Semgrep, CodeQL, and Python Security. The PR remains Draft; no predecessor result is merge evidence. |
…e the consumer root Green step for a8d6261. The 24 specialized cases in test_strix_quick_gate.sh installed the trusted gate/model/binder into $repo_root_dir/scripts/ci and ran ./scripts/ci/strix_quick_gate.sh, so a consumer-root binder lookup could never fail there and masked the #2292 defect. Each case now materializes into $tmp_dir/trusted-source/scripts/ci and runs the gate from that directory with STRIX_REPO_ROOT=$repo_root_dir, which keeps the old repo-root semantics (the gate defaults REPO_ROOT to SCRIPT_DIR/../..). Evidence: - tests/test_strix_trusted_fixture_boundary.py: fails on a8d6261 (CI job 106083294309), passes here. - bash scripts/ci/test_strix_quick_gate.sh on Linux, umask 022: a8d6261 PASS (rc=0, 727s) and this commit PASS (rc=0, 726s). - strix-related pytest (8 files): 242 passed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P5o6j4zfxGPdRaH4Lug8UY
|
Admission correction — exact current head |
…tack Preserve the package-description boundary delta while adopting #2291, including the canonical AnyIO, CodeQL, and Strix owner repairs.
|
Exact-head RCA and canonical-owner restack — 2026-09-26
The PR remains Draft/Open. Fresh exact-head Checks and independent approval remain required; queued or skipped states are not passing. |
|
Concurrent-head re-audit: 새 head에도 blocker가 남아 Draft/Proposed를 유지합니다: 활성 CHANGES_REQUESTED 2건. 이전 head의 approval/Checks는 병합 근거로 승계하지 않습니다. Current head의 terminal Checks와 qualifying independent approval 전에는 merge하지 않습니다. |
Keep the registry-description gate, called-workflow checkout identity, and the Job Analysis trusted-base context. The Strix harness keeps a single trusted fixture helper that copies the report-scope module, and it runs the gate with STRIX_REPO_ROOT pointed at the consumer workspace.
This branch has not been deployed
2 participants
Finding
Published package descriptions are built-artifact metadata, not repository README state. The gate inspects the authoritative sdist
PKG-INFOand wheelMETADATA; README inspection is available only through explicit pre-releasebuild: none.Live evidence that motivated the gate:
Exact-head repair
Head:
1be0bf0e40f902f77036d2339de162b078251073job.workflow_repositoryandjob.workflow_shafor the reusable workflow's immutable checkout.persist-credentials: falsebefore project build code runs.astral-sh/setup-uvand exact uv0.11.28; no unhashed runtimepip install.{name}-{version}/PKG-INFOand{distribution}-{version}.dist-info/METADATAlocations. Exactly one authoritative entry is required; archive ordering never decides admission.main,master, ordevelopGitHub contract links while allowing exact commits and release tags.RED → GREEN
42c49398covers caller credential persistence and post-build README fallback.3b8ceba2removes both fail-open paths;fe0a908crecords the operational scenes.9715362dconstructs ambiguous sdist and wheel metadata.a5603d87replaces path-depth/ZIP-order tie-breaks with the standard metadata-root contract;1be0bf0edocuments the authoritative sources.Verification
main@e6334e22: ahead 12, behind 0, mergeable true.Authoritative references: PyPA source distribution format and PyPA wheel format.
Protected-default-branch code search previously found no callers for this workflow; adoption must pin a protected/released commit. Draft / Proposed is intentional containment. Ordinary merge remains blocked until terminal required Checks and an independent qualifying current-head approval exist.