Repository navigation
fix: prevent source overrides from poisoning release caches - #144
Conversation
|
🦞👀 Pull request received. I will update this pull request when review starts. ClawSweeper review completeClawSweeper finished reviewing this revision. The review result is being finalized. |
|
Codex review: needs maintainer review before merge. What this changesThis PR restricts cache writes to validated main/stable builds and routes source overrides through read-only cache credentials. Example: Dispatch an unsigned build from main with openclaw_ref=383c124cc32ffee3f8e56f549b88da94d269a4af.
Review scores
ProductKind: Bug fix · Worth it: Yes · Fix scope: Complete Merge readiness✅ Ready for maintainer review The completed cache window clears the previous operational blocker, and no actionable defect remains in the reviewed head; required architecture and upgrade CI is still pending. Priority: P1 Before mergeNone. FindingsNone. Tests
Agent review detailsHow this fits togetherThe packaging workflow captures upstream source identity, selects cache authority, builds x64 and ARM64 packages through one reusable workflow, and passes validated artifacts to separate signing and publication jobs. flowchart LR
A[Workflow event and source selector] --> B[Immutable source preflight]
B --> C[Trusted write caller]
B --> D[Override read caller]
C --> E[Shared package and MSIX build]
D --> E
E --> F[Selected build validation]
F --> G[Upgrade and release gates]
Technical reviewBest possible solution: Land the reviewed cache boundary through required CI, then complete the accepted hardened-main activation checks before any official release. Do we have a high-confidence way to reproduce the issue? Main checks out and executes arbitrary selected upstream code in a cache-writing job; suppressing explicit saves cannot constrain that code's direct or nested cache-service access. Is this the best way to solve the issue? The repair places authority in caller-scoped service credentials, preserves one reusable build owner, and validates the selected route and captured source before downstream consumers. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning medium; reviewed against 9eb1eac47ffe. Provenance checked
TestingProof path: shipped entry point. SecurityNone. EvidenceWhat I checked:
Likely related people:
LabelsLabel changes:
Label justifications:
Rating scale6/6 🦀 challenger crab · 5/6 🦞 diamond lobster · 4/6 🐚 platinum hermit · 3/6 🦐 gold shrimp · 2/6 🦪 silver shellfish · 1/6 🧂 unranked krab. Overall follows the weaker of proof and patch quality; ✨ marks media proof (a screenshot, video, or linked artifact) that directly shows the changed behavior. WorkflowClawSweeper edits this one comment on every review. Comment HistoryReview history (9 earlier review cycles; latest 8 shown)
Reviewed October 16, 2026, 6:21 PM ET / 22:21 UTC (Revision 10). |
371b3d6 to
8d924f0
Compare
Capture source once and route trusted main/stable builds and arbitrary-source builds through explicit write/read cache caps into one shared build workflow. Preserve source validation, retries, dispatch behavior, both architectures, and release gates, with behavioral regression coverage and migration guidance. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 025783bc-ac15-42c0-b753-ed6407a65d72
Keep downstream job gates cancellation-aware and require successful prerequisites even when one build route is intentionally skipped. Fail the aggregate gate when eligible upgrade tests do not run. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 025783bc-ac15-42c0-b753-ed6407a65d72
Keep the aggregate validator available even after a prerequisite fails, without persisting checkout credentials. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 025783bc-ac15-42c0-b753-ed6407a65d72
8d924f0 to
b7abfdf
Compare
|
@clawsweeper re-review Current head c16ded6 restores BOTH versioning/non-PR bundle-upload conditions identified in your last review. The packaging relevance, selected-build, and job-result suites pass again. Current CI: https://github.com/openclaw/openclaw-windows-packaging/actions/runs/38972723480 . The only code delta from reviewed8f is those two conditions. The maintainer has now explicitly authorized the GitHub Actions cache reset and scoped alert disposition. Fresh8f CodeQL instances173–175 were reconciled with current service proof and confirmed dismissed as false positives at21:47–21:48UTC; scanning remains enabled. Exact service evidence: run38969891188 read job117206112599 logged service denial after forged client mode; actual reusable job117206177150 passed its direct denial assertion. No cache policy or source trust check changed in c16. The updated body records the controlled activation order. We will let legacy main writers settle, prevent racing legacy runs, persist/enumerate/purge authorized main cache IDs and verify their absence, then install this reviewed boundary. Main cold/warm and arbitrary-source execution are mandatory before release activation, and can only run after the hardened workflow is on main; the merge window exposes no legacy cache. No official release has started. Please reassess the concrete upload repair, authorized security decision, and explicit staged activation rather than treating the older pending-approval note as current. |
|
🦞👀 Re-review progress:
|
|
@clawsweeper re-review The sole before-merge action in the current c16 review is now complete. Authorized cache-window receipt follows. Existing PR CI continues while new packaging triggers are paused; no source/permission/scanning rule has changed. The task-owned obsolete unsigned main run38969067147 settled before deletion, all other active-main run queries were empty, all22 freshly enumerated approved main-scoped cache IDs were deleted, a fresh read confirmed0, and main was independently read back unchanged. The body records the completed pre-merge window and exact resume owner/condition. After exact-head review/CI admit the merge, this task will re-enable only the hardened main workflow and require cold/warm/explicit-source read-only service proof before official release. The earlier three CodeQL false-positive dispositions are already authorized and confirmed; scans remain enabled. Please reassess the completed operational action on unchanged head c16ded6. |
|
🦞👀 Re-review progress:
|
What Problem This Solves
An arbitrary upstream source dispatched from the default branch can write caches that later trusted builds restore. Suppressing explicit cache saves alone does not constrain subprocesses or nested actions.
User Impact
Only validated main/stable payload builds receive write-capable cache tokens. Source overrides and PR builds retain read-only cache access; signing/publication jobs receive none. Existing hotfix payload overrides, rollback selectors, both architectures, and package identities remain supported.
Reviewed boundary landed; release activation remains gated: all22 approved legacy main-scoped entries were invalidated, main was freshly verified empty before admission, and this reviewed head landed on main16b7f9eb2779a1c2a554bc21af1a880b9a9eec65. The hardened packaging workflow is active again. Unsigned cold-main run38979137940 is in progress; cold/warm/override proof is not yet complete. No official signing or publication has started.
Why This Change Was Made
One source-preflight owner captures and binds the immutable selection to workflow/run/ref/event context. Mutually exclusive callers give one reusable build owner a service-enforced write or read cap. The selected-build validator requires exactly the expected successful route and matching payload/release-identity outputs; eligible versioning upgrades cannot silently skip.
The conflict repair preserves current main's hotfix/rollback contract, centralized path classification, optional Store/bundle composition, payload artifacts, and all newer runtime changes. Package/payload cache format 2 excludes pre-boundary explicit caches; implicit NuGet/upstream/CodeQL caches still require the coordinated reset.
Versioning CI now records synthetic trusted write/independent restore and a save attempt with
ACTIONS_CACHE_MODE=writeunder a read-scoped token. The actual reusable build job also sends a direct cache-service request and requires a policy denial, not a timeout or unrelated failure. These are PR-ref service-capability tests, not a claim that main cold/warm cutover has already run.Evidence
Head:
c16ded692ebe4a1b845c03ef130bb3942fb25b76; integrated base:9eb1eac47ffeea36f514d61784e0ff649d34f7bf. All three original contributor commits remain ancestors. Residual diff: 20 files, +1232/-736; most workflow growth moves the existing build into one shared owner. The cache-key owner/regression and one-line patch revision from #180 join the original scope. #139 is integrated with original ancestry, so signed-tag admission and cache authority are qualified together.Local PowerShell 7 checks passed, using process-local
TEMPon macOS:Test-OpenClawSource.Tests.ps1: 68 passed, including trusted/read-only routing, main-dispatched SHA overrides, tags, workflow context drift, immutable replay, hotfixes, rollback, and fail-closed missing snapshots.Test-WorkflowSigningConfiguration.ps1: selected-build contracts passed.Test-WorkflowJobResults.Tests.ps1: eligible skipped/failed/cancelled upgrades rejected; intentional skips retained.Test-PackagingRelevance.Tests.ps1,Test-OpenClawCacheKey.Tests.ps1,Test-OpenClawPackage.Tests.ps1,Test-MSIXReleaseIdentity.Tests.ps1,Test-WorkflowPackageVersion.Tests.ps1,Test-MSIXUpgradeMatrix.Tests.ps1: passed.Test-SigningInputs.Tests.ps1: synthetic package/bundle and captured-source authorization passed.Test-DocReferences.ps1: zero findings.git diff --check: passed.After integrating #139 and #180, the 68 source cases, signing-input suite, and cache-key suite passed again. Identity/workflow-version suites also passed after the revision integration. Other local results are reused only where their inputs are unchanged. Current-head CI is 38972723480. The previous run passed both architectures and 12 upgrade cases but correctly rejected candidate2026.10.400.0 equal to the installed Store baseline. The requested revision1 now derives2026.10.401.0. No guard, immutable baseline, upstream/runtime pin, or signing policy was relaxed. Naturally triggered current-head CI is authoritative for workflow parsing, service capability, Windows host/NativeAOT, x64/ARM64 package/bundle construction, all 13 upgrade scenarios, and CodeQL. Earlier-head green checks are not being used for merge admission.
Scoped service and snapshot evidence (prior head; cache/snapshot contracts unchanged)
CI run 38962691564 ran prior head 9c657cf. Both synthetic cache jobs passed. The independent read job restored the exact bytes written by the write-capable job; after
ACTIONS_CACHE_MODE=writewas forged, the service reported:cache write denied: token has no writable scopes. The directCreateCacheEntryrequest in the actual reusable build job also passed its required service-policy-denial assertion. No cache-scanning rule or token policy was relaxed.Downloaded that run's real immutable source artifact and replayed it with the current
Get-WorkflowSource.ps1whileInvoke-RestMethodwas replaced with a throwing network guard. That replay used the prior-head resolver; final-head CI must confirm the integrated signed-tag/captured-context contract. The captured packaging commit is3de3ec103b37a5c4af5a16ad98000db316556b59(the CI merge snapshot). Results:383c124cc32ffee3f8e56f549b88da94d269a4af, recomputed routeread-only, original bytes unchangedworkflowRef; start a new workflow runworkflowRef, bytes unchanged; no re-resolutionCurrent source-snapshot SHA-256:
bdf1b9333d566c1715b395a7f5f629566aa93a000514fbbd9b9621370e962b05. Legacy snapshot SHA-256:01612b5613b9dab2df750435e764ac3c79e417e78944722b2c68cc02bce65e4e.The maintainer authorized the GitHub Actions-only cache cutover and the three scoped false-positive dispositions. Alerts 173, 174, and 175 were confirmed dismissed as false positives at21:47–21:48UTC on2026-10-16, after reconciling fresh8f head instances and actual service denial. That run38969891188 independently passed write persistence/read restoration/forged-client denial; job117206112599 logged the explicit no-writable-scopes policy error, and actual reusable job117206177150 passed direct reservation denial. The c16 follow-up changes only two bundle-upload conditions, not cache authority. Scanning remains enabled; current-head CodeQL and CI must still be green before landing. The approved activation window has now cleared22 main-scoped legacy cache entries and verified0 remain; the operational receipt below records the completed boundary.
Not run locally: Windows managed/NativeAOT or installed package tests, main cache reset/cold/warm builds, signing, deployment, or publication. Live service enforcement ran on GitHub, not this host. Current-head hosted architecture/upgrade gates passed. Hardened-main cold/warm/override activation is now qualified in the completed record below.
Review repair and controlled activation
Current-head follow-up restores both selective bundle-upload conditions from #146: a non-versioning PR may compose bundles for packaging proof without transferring two artifacts no later job consumes. This repairs the dropped integration conditions and preserves their measured performance intent. Packaging-relevance, selected-build, and job-result suites passed again. Versioning PRs and non-PR runs retain both uploads; no consuming upgrade/release lane is weakened.
The approved cache activation uses an explicit GitHub Actions window: let every legacy main writer settle, prevent new legacy runs during the window, enumerate and record main-scoped IDs, invalidate the approved legacy entries (including implicit/CodeQL caches), and verify no legacy entries remain. Land the reviewed boundary with no legacy cache available, then re-enable the hardened main workflow and require cold trusted, warm trusted, and arbitrary-source read-only service proof before any official release. Cold/warm main execution necessarily follows installation of the hardened workflow on main; it remains a release activation gate, not a claim made by PR-ref probes. Unexpected writers, changed inputs, denied invalidation, or missing service enforcement stop activation.
The final reviewer must assess this restored upload intent and the authorized/dispositioned security gate. All six user-listed prerequisite outcomes and final assembled-source qualification remain mandatory before official signing/publication.
Completed pre-merge cache window
At2026-10-16T22:13:32Z, main remained9eb1eac47ffeea36f514d61784e0ff649d34f7bf. The task-owned obsolete unsigned main run38969067147 was retired and confirmed terminal; no other active main workflow remained. Gateway workflow343344141 was paused and independently confirmed disabled_manually to prevent new legacy runs. A fresh main-only inventory captured22 entries, including NuGet, CodeQL, explicit package/payload, and signing-tool download caches; all22 approved IDs were deleted. A fresh API read confirmed0 entries, and main was read back unchanged. PR-ref caches, release assets, secrets, scanning rules, and user Windows state were not touched.
The activation owner verified this exact head actually merged, confirmed the resulting main tree equals the reviewed tree, and immediately re-enabled the hardened workflow. The pause is closed. At re-enablement, cold trusted, warm trusted, and explicit-source/read-only service proof were still pending. All three are now qualified below; no official release was dispatched. The only cache present before cold dispatch was a newly created CodeQL key bound to hardened main16b7f9e, not a retired legacy entry.
Final exact-head admission evidence
Head c16ded6: CI38972723480 completed SUCCESS on2026-10-16. Windows managed and NativeAOT gates, both architecture packages, both channel bundles, the selected-build validator, and all13 upgrade scenarios passed. Both retained Release/Debug TRX files report1312/1312 tests and0 failures. Current-head CodeQL is green. The complete37-context rollup is SUCCESS; GitHub reports CLEAN/MERGEABLE, with no unresolved review threads or Before-merge actions in the current-head ClawSweeper verdict.
Downloaded all13 transition receipts. Store product9NV70LV3D6XC installed baseline2026.10.400.0 and upgraded in place to2026.10.401.0: reserved family OpenClawFoundation.OpenClawGateway_rfcbke2p71se2, statusOk, localStateRetained=true. These are isolated hosted transition receipts, not the still-unrun MXC session/recovery/sign-in proof required by #171.
The current-head read-scoped service job actually restored its allowed cache, then received "cache write denied: token has no writable scopes" after the client mode was forged. The actual reusable package job independently required rejection of CreateCacheEntry by its inherited read-scoped token. The current immutable snapshot binds PR merge58c393ef6f1d678199659398e07014102cc65986/run38972723480/refs/pull/144/merge to upstream383c124cc32ffee3f8e56f549b88da94d269a4af/version2026.10.4.
Immediately before admission, main remained9eb1eac47ffeea36f514d61784e0ff649d34f7bf, the campaign-owned workflow pause remained effective, there were no active main runs, and a fresh main-cache inventory was empty. Landing uses a merge commit to preserve the integrated reviewed #139/#180 ancestry. Immediately after actual landing, the activation owner enabled hardened main. The post-merge activation runs are now qualified below. #171's installed-session proof, the remaining prerequisite outcomes, independent review of #183, and exact final assembled-source qualification still gate official signing/publication.
Completed hardened-main activation
The cold trusted build and warm trusted build both completed successfully at main
16b7f9eb2779a1c2a554bc21af1a880b9a9eec65. Actual package/x64/ARM64 cache miss/save and subsequent restore receipts identify the same cold-created entries:9073888904,9074067878, and9074183685.After #182/#183, the corrected unsigned main SHA override completed successfully at main
085043be6367b95ae7de787c32503a396a3dfc2f. It selected the read-only route, restored those same three entries, and actually executed the inherited-token cache-service denial despite a forgedACTIONS_CACHE_MODE=write. The exact denied key inventory is zero. Both architecture builds, both bundles, NativeAOT execution, and the selected-build validator passed; both retained host TRX records report 1,312/1,312 passed and zero failed.The upstream/cache tuple is unchanged:
383c124cc32ffee3f8e56f549b88da94d269a4af, OpenClaw2026.10.4, Node.js24.21.0, package SHA-256347d94798ce90d10c34b2c884a2742760bfa7eed9da5561ac9a3f1844229c6d3. The main override's immutable source snapshot replays read-only without byte changes, SHA-25653ac1f9183034f2ea0c62d1d161139969b2f8f34f8c732cd78d30f01a8ae93fe. These unsigned manual runs intentionally skip the installed upgrade matrix, signing, and publication. They do not prove MXC recovery/sign-in or the final assembled release after #171/#153.