fix(images): backport historical OpenClaw security patches - #7276
fix(images): backport historical OpenClaw security patches#7276ericksoa wants to merge 142 commits into
Conversation
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
ericksoa
added
the
area: security
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a historical OpenClaw security-revision pipeline with pinned dependency remediation, transactional rollback, deterministic archive generation, Trivy validation, immutable OCI publication, provenance, and extensive tests. ChangesHistorical security revision pipeline
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Workflow
participant Buildx
participant Trivy
participant ORAS
participant GHCR
Workflow->>Buildx: Build pinned multi-arch OCI candidate
Workflow->>Trivy: Scan candidate and historical images
Trivy-->>Workflow: Return CVE and database-hash evidence
Workflow->>ORAS: Copy validated OCI index
ORAS->>GHCR: Publish immutable revision tag
GHCR-->>Workflow: Return published manifest
Workflow->>Workflow: Verify digests and upload evidence
Possibly related issues
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage in commit 84a4ea8 in the TypeScript / code-coverage/cliThe overall coverage in commit 84a4ea8 in the Show a code coverage summary of the most impacted files.
Updated |
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
PR Review Advisor — InformationalAdvisor assessment: Informational / medium confidence Model lanes
Nemotron output stays in workflow artifacts and does not change the assessment above. E2E guidanceAdvisory only. E2E / PR Gate selects and runs jobs independently. Recommended E2E: This automated review informs maintainers. Warnings and suggestions do not require a response. A maintainer decides whether to merge. |
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
ericksoa
changed the title
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
apurvvkumaria
left a comment
apurvvkumaria
left a comment
There was a problem hiding this comment.
Requesting changes on exact head 36e8c5f014607add6dd9f81c06f8ca8fa918bf03.
There are three correctness/security blockers in addition to the existing automated findings:
-
The plugin-install wrapper is bypassable and can patch the wrong state directory.
scripts/openclaw-security-revision-wrapper.sh:11recognizes the remediation path only whenplugins installoccupy argv positions 1–2, and lines 25–32 always scan$HOME/.openclaw. OpenClaw accepts root options such as--profile/--devand anOPENCLAW_STATE_DIRoverride. Thereforeopenclaw --profile qa plugins install ...bypasses remediation entirely;openclaw plugins install ... --profile qainstalls under the profile state directory but then scans the default state directory. In both cases a reviewed plugin can remain on the vulnerable Axios graph. Parse OpenClaw's global options consistently, derive the effective state directory used by the original invocation, and add wrapper-level regression tests for prefix/suffix--profile,--dev, and customOPENCLAW_STATE_DIR. Please also ensureplugins install --helpdoes not mutate existing plugins. -
The new publishing workflow bypasses the repository's production build-argument guard. Current CI fails
test/openclaw-dependency-review.test.ts:469because.github/workflows/openclaw-tar-security-revision.yamlinvokes the production build with unguardedBASE_IMAGEandEXPECTED_OPENCLAW_VERSIONarguments. Add the canonicalValidate production Docker build argsguard before the build/push step. -
The reviewed dependency audit is red on the exact head.
reviewed-npm-auditfinds a high-severity Axios graph and a critical tar graph in the pristine historical archives. The Dockerfile targets tar 7.5.19 and the wrapper intends to transform reviewed plugin installs, but the required gate currently has no evidence for the post-remediation installed graph. Please make the audit verify the final transformed graph (while retaining explicit review of the vulnerable source inputs) rather than waiving the failure. The final image/plugin graph must be the artifact that satisfies the high/critical threshold.
I also agree with the advisor's existing PRA-1 blocker: the live dependency trees and metadata are replaced non-transactionally, so an injected rename/write failure can leave an installation partially remediated. The current unresolved CodeQL filesystem-race finding should be addressed as part of that recovery boundary.
Process gate: commits 545426aae0e8, 0f130c9d35e9, 0345ff239129, and 74c0146c9534 do not contain a Signed-off-by trailer, despite the repository requirement that every PR commit carry DCO sign-off and the PR description stating that all commits do. Please correct the branch history before marking the PR ready.
The shard-7 gateway-recovery.test.ts timeout appears unrelated/flaky; I am not treating that as a code blocker for this diff. The other failures above are directly attributable to this PR and need resolution.
There was a problem hiding this comment.
This review supersedes my earlier review and narrows the requested changes to the items that must be resolved before this security backport is ready. It applies to exact head 36e8c5f014607add6dd9f81c06f8ca8fa918bf03.
Blocking security behavior
-
Resolve the profile/state-directory remediation bypass. The wrapper recognizes only positional
plugins installand always scans$HOME/.openclaw. OpenClaw global--profile/--devoptions andOPENCLAW_STATE_DIRcan therefore bypass remediation or make it scan a different state directory from the one that received the plugin. Normalize the supported global options, use the effective OpenClaw state directory, and add focused wrapper tests for prefix/suffix--profile,--dev, and customOPENCLAW_STATE_DIR. -
Make runtime plugin remediation fail closed. After the original install succeeds, a staging, swap, metadata, or verification failure can leave the newly installed vulnerable plugin present. The minimum acceptable behavior is to remove the failed fresh installation or restore the prior plugin state before returning failure. Full transactional recovery for the Docker image overlay is not required because a failed image build is discarded and cannot be published.
Required pre-merge gates
-
Add the canonical production build-argument guard.
test/openclaw-dependency-review.test.ts:469correctly rejects the new production workflow because itsBASE_IMAGEandEXPECTED_OPENCLAW_VERSIONbuild arguments are not covered by the repository guard. -
Clear the reviewed dependency audit with bounded evidence. The audit is examining intentionally vulnerable, exact-SRI historical source archives; its failure does not by itself prove the final revised image is vulnerable. A narrowly scoped exception tied to the exact reviewed inputs is acceptable if the workflow also verifies the post-remediation installed graph and fails when high/critical vulnerable versions remain. The required aggregate CI check must be green before merge.
Explicitly non-blocking / fix-forward
A new follow-up issue, #7288, has been created to track these non-blocking fix-forward items and is linked to the main issue, #7272.
- Full rollback support for the Docker build-time tar overlay.
- The same-user CodeQL filesystem race, absent evidence of a privilege-boundary crossing.
plugins install --helpside effects.- Broader fault-injection and wrapper integration coverage beyond the focused blocker regressions above.
- The unrelated
gateway-recovery.test.tstimeout.
I withdraw the earlier DCO objection. NemoClaw requires a valid PR-body Signed-off-by declaration and GitHub-Verified commits; this PR has the declaration, all eight commits are Verified, and the required DCO check passes.
Superseded by corrected review #7276 (review). The earlier review overstated the blocker set and incorrectly treated per-commit Signed-off-by trailers as required.
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
| writeJson(packageJsonPath, packageJson); | ||
| } | ||
|
|
||
| function patchOtelSdkNodePackageGraph(packageDirectory: string): void { |
| writeJson(packageJsonPath, packageJson); | ||
| } | ||
|
|
||
| function patchOtelSdkNodePackageGraph(packageDirectory: string): void { |
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Summary
This adds a manual, main-only path to publish security-revised historical OpenClaw base images without upgrading OpenClaw or changing any existing tag or digest. It deterministically remediates the reviewed production dependency graph and keeps runtime plugin installation transactional and fail-closed.
This PR owns historical image revisions for NemoClaw
v0.0.50throughv0.0.89. Merge alone publishes no image and mutates no existing tag.Related work and ownership
2ad613d6e9abc26859b59e5a876ddd0ee40c7ca1and owns the current OpenClaw2026.6.10release/image lane plus the sharedreviewed-npm-auditmachinery. This PR is rebased on that result; its historical raw-input verification is reconciled on top of the shared audit implementation.Changes
tar@7.5.19, and require a zero-finding production audit for the remediated OpenClaw core graph.tarand verify that npm, npx, pack, and install remain usable.2026.5.22,2026.5.27, and2026.6.10versions of Slack, Teams, Discord, diagnostics, and WhatsApp.axios@1.18.0and the reviewed plugin-core replacements needed for a zero high/critical plugin audit./sandbox, protect credentials outside rollback snapshots, and restore state transactionally across injected install/remediation/rollback failures.O_NOFOLLOWplus descriptor-basedfstatvalidation, and reject symlinked intermediate state/project parents before candidate discovery.Type of Change
Quality Gates
DGX Station Hardware Evidence
Verification
Tested head:
91d7d83f3b05df63458f93f29e3ba9014236d110npm run check:diffpasses, including repository, formatting, lint, shell, Dockerfile, TypeScript, commit, and pre-push gates..test.ts; its decision tests pass 3/3 and the changed-test conditional guard reports no added branch.\n- [x] Plugin archive snapshots open leaf files withO_NOFOLLOW, validate the opened descriptor withfstat, and hash content plus mode from that same handle, closing the CodeQL filesystem-race finding.\n- [x] Historical core verification now requires each exact reviewed post-remediationnpm lsproblem set and preserves the reviewed GNU/musl clipboard platform trees.2026.6.10, reports zero core production audit findings, and reports zero high/critical findings for every reviewed remediated plugin archive.2026.5.18, reports the same clean audits, and preserves the historical no-shrinkwrap contract./sandboxSlack install resolvesaxios@1.18.0,body-parser@2.3.0, andws@8.21.1; its production audit reports zero high/critical findings./sandboxWhatsApp install resolvesprotobufjs@8.7.1andws@8.21.1; its production audit reports zero high/critical findings.scripts/audit-reviewed-npm-graph.mtsbehavior is reconciled and covered by the exact-head audit suite.\n- [x] Current advisory audit under Node 22.22.2 proves the pinned raw archive graph exactly matches the reviewed input (low=1 moderate=6 high=4 critical=1), while the remediated shipping graph is clean at the release threshold (low=1 moderate=1 high=0 critical=0) and the locked mcporter graph has zero findings at every severity.91d7d83f3b05df63458f93f29e3ba9014236d110.npm run docsbuilds without warnings (doc changes only)No historical image publication workflow has been dispatched, and no image or existing tag has been published or mutated by this PR.
Signed-off-by: Aaron Erickson aerickson@nvidia.com
Signed-off-by: Charan Jagwani cjagwani@nvidia.com
Summary by CodeRabbit
tarto 7.5.19 and strengthened historical OpenClaw security revision checks for Axios, Hono node-server, and OpenTelemetry/Jaeger.@hono/node-serverto 2.0.10 and tightened reviewed dependency verification criteria.