Repository navigation
Pin the reviewed Feedz publish policy - #9129
Conversation
Pin the feedz-publish environment identity and the SHA-256 of a new operational packet in REVIEWED_OPERATIONAL_POLICY. Tests that cover a missing policy now patch it to None, and a new test checks that the pinned policy passes shape validation and matches the packet bytes.
|
Important Review skippedBot user detected. To trigger a single review, invoke the
|
|
[Elsa 3 Lead] On behalf of @sfmskywalker. I sign off on the Feedz publish operational packet in this PR as written ( @elsa-workflows-bot review |
|
This PR does not match any of the 1 configured review trigger rule. |
There was a problem hiding this comment.
Elsa 3 Code Review: APPROVE + HIGH @ 742f2cd
Code Review, Round 1/6
Small, focused change: it replaces the None placeholder in REVIEWED_OPERATIONAL_POLICY with concrete values, adds the decision record those values hash, and adjusts the tests that relied on the placeholder. No blocking findings.
Verified
- Policy pins match the live environment. Read-only reads of
environments/feedz-publishand itsdeployment-branch-policiesat review time return env id25739609238,can_admins_bypass: false, required reviewers rule70613239(reviewer938393,prevent_self_review: true), branch policy rule70613240,deployment_branch_policy= custom policies only, and exactly one branch policy{"id": 64850643, "type": "branch", "name": "main"}. Every pinned value agrees, and so does each runtime condition incheck_admission(two rules, reviewer set, branch policy list equality,allowed_ref=refs/heads/main). - Packet hash matches.
sha256of the exact bytes ofdocs/integration-program/feedz-publish-operational-packet.mdat this head is416725fcd31207336a3fd9bd0d4eb18d2ff618ddcda9fb137dcc5b1d87f49d17, identical to the pinnedoperational_packet_sha256, computed the same way the new test does (hashlib.sha256(path.read_bytes())). - Admin bypass stays required off.
check_admissionstill requiresenvironment.get("can_admins_bypass") is Falsebefore any rule checks; nothing in this PR touches that path or the workflow. - Drift fails closed. The hash link is enforced by the new unit test, and the publish workflow's
verifyjob runstest_consolidated_package_executor(normal and-O) at the same SHA beforeadmitcan run. An edit to the packet without a matching hash update therefore stops a publish run before admission. - Tests. Locally at this head:
test_consolidated_package_executor53 OK (also with-O),test_consolidated*.pydiscovery 164 OK, and the fourscripts/test_*package/stage suites 90 passed. Mutation checks: appending one byte to the packet fails the new test, and making a pinned id a string fails it withoperational_policy_invalid. The two existing tests that covered the missing policy now patch it toNoneexplicitly, so they keep testing the same thing. - CI. All check runs on this head are green.
Non-blocking
- The new test name says "passes shape validation". It reaches
runtime_not_admitted, which proves the type/shape block passes, but theallowed_refandbranch_policiesconsistency checks run later incheck_admissionand are not exercised for the pinned values here. A follow-up could split those consistency rules into a small pure helper and assert them directly against the pinned policy. Not needed for this PR. - The PR description still opens with the draft notice ("awaiting sign-off ... Do not merge"). The sign-off is now recorded in the PR conversation, so refreshing the description before merge keeps the history clear.
- The packet's
Statussection reads "Pending approval". Its own wording (takes effect after sign-off and merge) stays accurate after merge, and changing it would change the pinned hash, so leaving it as is is reasonable.
No review threads are open; CodeRabbit skipped (bot author) and Greptile did not trigger.
— Code Review (Elsa 3), via Elsa Review
|
@elsa-workflows-bot[bot] please read the following Contributor License Agreement(CLA). If you agree with the CLA, please reply with the following information.
Contributor License AgreementContribution License AgreementThis Contribution License Agreement ( “Agreement” ) is agreed to by the party signing below ( “You” ), 1. Definitions. “Code” means the computer software code, whether in human-readable or machine-executable form, “Project” means any of the projects owned or managed by .NET Foundation and offered under a license “Submit” is the act of uploading, submitting, transmitting, or distributing code or other content to any “Submission” means the Code and any other copyrightable material Submitted by You, including any 2. Your Submission. You must agree to the terms of this Agreement before making a Submission to any 3. Originality of Work. You represent that each of Your Submissions is entirely Your 4. Your Employer. References to “employer” in this Agreement include Your employer or anyone else 5. Licenses. a. Copyright License. You grant .NET Foundation, and those who receive the Submission directly b. Patent License. You grant .NET Foundation, and those who receive the Submission directly or c. Other Rights Reserved. Each party reserves all rights not expressly granted in this Agreement. 6. Representations and Warranties. You represent that You are legally entitled to grant the above 7. Notice to .NET Foundation. You agree to notify .NET Foundation in writing of any facts or 8. Information about Submissions. You agree that contributions to Projects and information about 9. Governing Law/Jurisdiction. This Agreement is governed by the laws of the State of Washington, and 10. Entire Agreement/Assignment. This Agreement is the entire agreement between the parties, and .NET Foundation dedicates this Contribution License Agreement to the public domain according to the Creative Commons CC0 1. |
Signed off by the accountable owner (see comment); ready to merge.
This pins
REVIEWED_OPERATIONAL_POLICYinscripts/integration-program/consolidated_package_executor.py(it wasNone, so admission always stopped atoperational_policy_unconfigured). It also adds the decision record that the policy hashes.Pinned values
Read from the GitHub API (
environments/feedz-publishand itsdeployment-branch-policies) at 2026-10-30T02:12:00Z:environment_id25739609238reviewer_rule_id70613239branch_rule_id70613240reviewer_ids[938393](sfmskywalker)branch_policies[{"id": 64850643, "type": "branch", "name": "main"}]allowed_refrefs/heads/mainoperational_packet_sha256416725fcd31207336a3fd9bd0d4eb18d2ff618ddcda9fb137dcc5b1d87f49d17Operational packet
docs/integration-program/feedz-publish-operational-packet.mdrecords the decision: publish the pinned 3.10.0 consolidated candidate (source5cff6722, run40347931480, artifact12664519771) to Feedzelsa-3throughfeedz-publish. It also names the roles (dispatcherelsa-workflows-bot, sole approver and accountable ownersfmskywalker), the environment identity, and the status, which is "pending approval". The SHA-256 above covers the file's exact bytes, so any edit to the packet also needs this hash updated.Any change to the
feedz-publishenvironment, including deleting and recreating it, changes these ids. Admission then fails closed until a new PR pins the new values.Tests
None.python3 -m unittest discover -s scripts/integration-program -p 'test_consolidated*.py'(also with-O): 164 tests OK.scripts/test_validate_packages_workflow_gates.py,test_validate_packages_id_set.py,test_validate_packages_publication_selection.py,test_stage_consolidated_candidate_nupkgs.py: OK.