Skip to content

fix(e2e): restore image regression coverage#7355

Merged
apurvvkumaria merged 4 commits into
mainfrom
codex/fix-e2e-image-regressions
Jul 22, 2026
Merged

fix(e2e): restore image regression coverage#7355
apurvvkumaria merged 4 commits into
mainfrom
codex/fix-e2e-image-regressions

Conversation

@prekshivyas

@prekshivyas prekshivyas commented Jul 22, 2026

Copy link
Copy Markdown
Collaborator

Summary

Restore the deterministic image and upgrade coverage exposed by E2E main run 29887082757. Deep Agents Code now installs the verified archive downloader before node-tar remediation, legacy OpenClaw fixture images remediate their affected tar dependency before the completed-image scan, and frozen gateway-upgrade fixtures no longer fail only because the current advisory database changed.

Changes

  • Move the Deep Agents Code npm-private node-tar remediation after the layer that installs curl, and extend the Dockerfile contract to enforce that prerequisite ordering.
  • Add an exact, E2E-only openclaw@2026.3.11 remediation from tar@7.5.11 to reviewed tar@7.5.19. The rebuild-openclaw and upgrade-stale-sandbox fixtures require this compatibility path; relaxing the completed-image scanner would weaken the production security boundary. The OpenClaw remediation and integrity contract tests protect the archive identity, dependency shape, metadata hash, install path, and scanned tree.
  • Extract the existing frozen-installer adapter and skip only the current advisory audit for an immutable historical mcporter lock while retaining npm audit signatures. The historical source cannot be changed without invalidating the upgrade fixture; the new E2E-support tests prove the exact replacement and ambiguous-boundary rejection.
  • Update the existing OpenClaw dependency review note with the fifth reviewed remediation identity and fixture-only audit boundary.

Type of Change

  • Code change (feature, bug fix, or refactor)
  • Code change with doc updates
  • Doc only (prose changes, no code sample modifications)
  • Doc only (includes code sample changes)

Quality Gates

  • Tests added or updated for changed behavior
  • Existing tests cover changed behavior — justification:
  • Tests not applicable — justification:
  • Docs updated for user-facing behavior changes
  • Docs not applicable — justification: No supported user-facing behavior changes; the existing security review note is updated only to keep reviewed fixture identities and boundaries aligned.
  • Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging)
  • Sensitive-path review completed or maintainer-approved waiver recorded — reviewer/approval link/justification: Maintainer security review is pending on this PR.
  • Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue:

DGX Station Hardware Evidence

  • Tested on DGX Station
  • Tested commit: not applicable
  • Station profile/scenario: not applicable
  • Result: not applicable
  • Supporting evidence: not applicable

Verification

  • PR description includes a Signed-off-by: line and every commit appears as Verified in GitHub
  • Normal pre-commit, commit-msg, and pre-push hooks passed, or npm run check:diff passed when hooks were skipped or unavailable
  • Targeted behavior tests pass for the current change set, or tests are marked not applicable above — npx vitest run --project integration test/node-tar-dockerfile-contract.test.ts test/openclaw-npm-remediation.test.ts test/openclaw-integrity-pin-contract.test.ts (23 passed); npx vitest run --project e2e-support test/e2e/support/openshell-gateway-upgrade-old-installer.test.ts test/e2e/support/rebuild-openclaw-old-base-context.test.ts (6 passed); npm run test:changed (3 passed); npm run test:projects:check and npm run source-shape:check passed.
  • Applicable broad gate passed — focused image and fixture changes use the targeted evidence above; required CI is pending.
  • Quality Gates section completed with required justifications or waivers — sensitive-path review is pending.
  • No secrets, API keys, or credentials committed
  • npm run docs builds without warnings (doc changes only) — the build passed with two pre-existing Fern warnings.
  • Doc pages follow the style guide (doc changes only)
  • New doc pages include SPDX header and frontmatter (new pages only)

Signed-off-by: Prekshi Vyas prekshiv@nvidia.com

Summary by CodeRabbit

  • Bug Fixes
    • Added support for installing and upgrading OpenClaw 2026.3.11 with the correct legacy remediation behavior.
    • Improved npm archive remediation integrity checking and expanded post-install global package verification across supported OpenClaw versions.
    • Improved determinism and reliability of historical gateway upgrade flows while preserving archive signature verification and enforcing stricter audit boundaries.
  • Documentation
    • Updated security/dependency review guidance for the adjusted remediation rules and expected integrity artifacts.
  • Tests
    • Expanded e2e and contract tests for legacy upgrades, installer patching, archive integrity pinning, and step ordering verification.

@prekshivyas prekshivyas added area: e2e End-to-end tests, nightly failures, or validation infrastructure area: security Security controls, permissions, secrets, or hardening labels Jul 22, 2026
@prekshivyas prekshivyas self-assigned this Jul 22, 2026
@coderabbitai

coderabbitai Bot commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: e1123041-9678-4f12-8f5b-42e9d727894b

📥 Commits

Reviewing files that changed from the base of the PR and between 26a32b9 and 6097357.

📒 Files selected for processing (4)
  • docs/security/openclaw-2026.6.10-dependency-review.md
  • test/e2e/live/openshell-gateway-upgrade-old-installer.ts
  • test/e2e/live/openshell-gateway-upgrade.test.ts
  • test/e2e/support/openshell-gateway-upgrade-old-installer.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
  • docs/security/openclaw-2026.6.10-dependency-review.md
  • test/e2e/live/openshell-gateway-upgrade.test.ts

📝 Walkthrough

Walkthrough

Adds OpenClaw 2026.3.11 legacy-core remediation, version-specific Docker install verification, historical installer fixture tests, and node-tar remediation ordering checks.

Changes

OpenClaw remediation and packaging

Layer / File(s) Summary
Legacy core remediation
scripts/lib/openclaw-npm-remediation.mts, docs/security/..., test/openclaw-npm-remediation.test.ts
Adds the legacy-core remediation path for openclaw@2026.3.11, conditionally hashes shrinkwrap metadata, documents the reviewed archive boundary, and tests rejection and rebuild behavior.
Version-specific image remediation
Dockerfile, Dockerfile.base, test/openclaw-integrity-pin-suite.ts
Applies the reviewed lifecycle recipe and remediation to OpenClaw 2026.3.11, with separate global package verification and remediated archive assertions.
Historical installer adapter validation
test/e2e/live/*, test/e2e/support/*, docs/security/...
Moves the historical installer patch helper into its own module and adds E2E coverage for OpenClaw pinning, signature verification, advisory isolation, and ambiguous audit rejection.
Node-tar remediation layer ordering
agents/langchain-deepagents-code/Dockerfile.base, test/node-tar-dockerfile-contract.test.ts
Runs the bundled npm node-tar patch after installing the verified downloader and validates the expected Dockerfile step ordering.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Installer
  participant ArchiveVerifier
  participant OpenClawRemediation
  participant DockerBuild
  Installer->>ArchiveVerifier: package reviewed OpenClaw archive
  ArchiveVerifier->>Installer: provide verified archive path
  DockerBuild->>OpenClawRemediation: build remediated OpenClaw archive
  OpenClawRemediation->>DockerBuild: return patched archive and metadata
  DockerBuild->>Installer: run historical installer with pinned archive
Loading

Possibly related issues

Possibly related PRs

Suggested labels: bug-fix, integration: openclaw, dependencies, area: e2e, area: security

Suggested reviewers: cv

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately reflects the main change: restoring E2E/image regression coverage.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/fix-e2e-image-regressions

Comment @coderabbitai help to get the list of available commands.

@github-actions

Copy link
Copy Markdown
Contributor

@github-code-quality

github-code-quality Bot commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

Code Coverage Overview

Languages: TypeScript

TypeScript / code-coverage/plugin

The overall coverage in commit 6097357 in the codex/fix-e2e-image-... branch remains at 96%, unchanged from commit 7be1bad in the main branch.


Updated July 22, 2026 03:53 UTC

@prekshivyas prekshivyas added the v0.0.91 Release target label Jul 22, 2026
@github-actions

github-actions Bot commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

PR Review Advisor — Informational

Advisor assessment: Informational / low confidence
Next action: Review the warnings below.
Findings: 0 blockers · 1 warning · 0 suggestions
Status: Partial review preserved 1 canonical finding(s) before the advisor stopped.

Model lanes

  • GPT-5.6 Terra (primary): Failed after a partial review · low confidence · 0 blockers · 1 warning · 0 suggestions
  • Nemotron 3 Ultra (second opinion): Completed · high confidence · 0 blockers · 0 warnings · 0 suggestions

Nemotron output stays in workflow artifacts and does not change the assessment above.

E2E guidance

Advisory only. E2E / PR Gate selects and runs jobs independently.

Recommended E2E: cloud-onboard, credential-sanitization, security-posture, openshell-gateway-upgrade

1 warning · 0 suggestions

Warnings

Warnings do not block.

PRA-1 Warning — Bind the legacy fixture's resolved dependency graph

  • Location: scripts/lib/openclaw-npm-remediation.mts:409
  • Category: security
  • Problem: The new legacy-core remediation changes only openclaw@2026.3.11's direct tar dependency to 7.5.19. That published archive has no npm shrinkwrap, and the fixture installs it with npm, so its remaining dependency ranges can resolve differently across builds despite the top-level archive SRI and direct tar pin.
  • Impact: The gateway-upgrade fixture can assemble different transitive dependency graphs over time, weakening the deterministic and reviewed dependency boundary for an E2E path that builds and runs a historical image.
  • Recommendation: Use a reviewed lock-derived install input for this fixture, or explicitly materialize and verify the complete resolved legacy graph before installation; document and audit the resulting fixture graph together with its archive pin.
  • Verification: Inspect the 2026.3.11 package manifest and the legacy-core branch: it rejects a shrinkwrap and changes only packageJson.dependencies.tar before npm install.
  • Test coverage: Add a fixture build regression that proves the complete installed legacy dependency graph matches committed identities (or that a checked-in lock-derived installation is used) and fails on transitive resolution drift.
  • Evidence: scripts/lib/openclaw-npm-remediation.mts:400-410 rejects an npm-shrinkwrap and only assigns packageJson.dependencies.tar. Dockerfile.base installs the rebuilt legacy archive with npm install -g, which resolves dependency metadata from that archive. test/openclaw-npm-remediation.test.ts verifies tar replacement and absence of a shrinkwrap, but does not assert a complete installed legacy graph.

Workflow run details

This automated review informs maintainers. Warnings and suggestions do not require a response. A maintainer decides whether to merge.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🧹 Nitpick comments (2)
test/e2e/live/openshell-gateway-upgrade-old-installer.ts (1)

10-63: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Exact-match marker strings are duplicated between the adapter and its test fixture with no shared source. needle, the embedded clone-line needle, and advisory_audit are hard-coded literals in the adapter and are independently hand-copied in the test's fixture generator; if either changes, the other can silently drift out of sync instead of failing at the point of change.

  • test/e2e/live/openshell-gateway-upgrade-old-installer.ts#L10-L63: export the top-level needle (and, where feasible, the other marker strings) as named constants from this module so consumers import rather than re-type them.
  • test/e2e/support/openshell-gateway-upgrade-old-installer.test.ts#L25-L60: import the exported constants from the adapter module instead of re-declaring the literal needle/clone-line/advisory_audit strings in writeHistoricalFixture.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test/e2e/live/openshell-gateway-upgrade-old-installer.ts` around lines 10 -
63, Export named constants for the top-level needle, embedded clone-line needle,
and advisory_audit marker from patchOldInstallerFixture’s module in
test/e2e/live/openshell-gateway-upgrade-old-installer.ts. In
test/e2e/support/openshell-gateway-upgrade-old-installer.test.ts, import and
reuse those constants in writeHistoricalFixture instead of redeclaring the
marker literals; update both affected sites to keep the adapter and fixture
generator synchronized.
test/e2e/support/openshell-gateway-upgrade-old-installer.test.ts (1)

95-105: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Strengthen the rejection test to prove no partial write occurred.

This test proves the process exits non-zero with the expected message, but doesn't assert that fixture.dockerfile was left unmodified. The underlying Python script mutates text in memory for the ARG-marker injection before the ambiguous-audit check raises (see the extracted helper, lines 29-51 vs 53-58) — capturing the original Dockerfile content and asserting it's unchanged after the failed run would directly verify the "no partial write on failure" contract this fixture exists to protect.

✅ Suggested strengthening
   it("rejects an ambiguous historical advisory boundary", () => {
     const fixture = writeHistoricalFixture(2);
+    const originalDockerfile = fs.readFileSync(fixture.dockerfile, "utf8");
     patchOldInstallerFixture(fixture.installer);

     const result = spawnSync("bash", [fixture.installer], {
       encoding: "utf8",
       env: { ...process.env, NEMOCLAW_OLD_OPENCLAW_VERSION: "2026.5.27" },
     });
     expect(result.status).not.toBe(0);
     expect(result.stderr).toContain("historical mcporter advisory audits; expected at most one");
+    expect(fs.readFileSync(fixture.dockerfile, "utf8")).toBe(originalDockerfile);
   });
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test/e2e/support/openshell-gateway-upgrade-old-installer.test.ts` around
lines 95 - 105, Strengthen the rejection test in “rejects an ambiguous
historical advisory boundary” by capturing the original contents of
fixture.dockerfile before spawning the installer, then assert the file contents
remain identical after the non-zero result and expected stderr. Use the existing
fixture.dockerfile symbol and preserve the current rejection assertions.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@docs/security/openclaw-2026.6.10-dependency-review.md`:
- Around line 92-93: The documentation overstates the helper’s pre-rebuild
source-shape validation. Update the text describing the E2E-only
openclaw@2026.3.11 handling to state only the checks actually
enforced—tar@7.5.11, absence of bundledDependencies, and absence of an npm
shrinkwrap—while noting that other manifest drift is detected later by the
metadata hash.

In `@test/node-tar-dockerfile-contract.test.ts`:
- Around line 74-77: Update the curl assertion in the Dockerfile contract test
to locate the downloader command specifically, such as the actual RUN curl line,
rather than any curl= package-pin text. Preserve the ordering check against
patchCopy and patchRun, and ensure the assertion confirms the downloader block
is absent when installsPatchDownloader is false.

In `@test/openclaw-npm-remediation.test.ts`:
- Around line 443-481: The test should validate the committed
expectedPatchedMetadataIntegrity rather than replace it with a hash extracted
from the intentional mismatch. First invoke buildRemediatedOpenClawArchive with
the default remediation metadata and assert success, then perform the
deliberate-mismatch call separately to verify the reported hash and retain the
archive-content assertions using the committed expectation.

---

Nitpick comments:
In `@test/e2e/live/openshell-gateway-upgrade-old-installer.ts`:
- Around line 10-63: Export named constants for the top-level needle, embedded
clone-line needle, and advisory_audit marker from patchOldInstallerFixture’s
module in test/e2e/live/openshell-gateway-upgrade-old-installer.ts. In
test/e2e/support/openshell-gateway-upgrade-old-installer.test.ts, import and
reuse those constants in writeHistoricalFixture instead of redeclaring the
marker literals; update both affected sites to keep the adapter and fixture
generator synchronized.

In `@test/e2e/support/openshell-gateway-upgrade-old-installer.test.ts`:
- Around line 95-105: Strengthen the rejection test in “rejects an ambiguous
historical advisory boundary” by capturing the original contents of
fixture.dockerfile before spawning the installer, then assert the file contents
remain identical after the non-zero result and expected stderr. Use the existing
fixture.dockerfile symbol and preserve the current rejection assertions.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: b46bd415-cae6-416e-9420-03ec81f6f62d

📥 Commits

Reviewing files that changed from the base of the PR and between 7be1bad and 26a32b9.

📒 Files selected for processing (11)
  • Dockerfile
  • Dockerfile.base
  • agents/langchain-deepagents-code/Dockerfile.base
  • docs/security/openclaw-2026.6.10-dependency-review.md
  • scripts/lib/openclaw-npm-remediation.mts
  • test/e2e/live/openshell-gateway-upgrade-old-installer.ts
  • test/e2e/live/openshell-gateway-upgrade.test.ts
  • test/e2e/support/openshell-gateway-upgrade-old-installer.test.ts
  • test/node-tar-dockerfile-contract.test.ts
  • test/openclaw-integrity-pin-suite.ts
  • test/openclaw-npm-remediation.test.ts

Comment on lines +92 to +93
For the E2E-only `openclaw@2026.3.11` identity, the helper replaces the exact `tar@7.5.11` declaration with reviewed `tar@7.5.19`.
It rejects a source archive that has different dependency metadata or an unexpected npm shrinkwrap.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Narrow the claimed source-shape validation.

The helper does not reject arbitrary dependency-metadata changes before rebuilding; it validates only tar@7.5.11, no bundledDependencies, and no shrinkwrap. Other manifest drift fails later through the metadata hash. Update this wording or enforce an exact dependency shape. As per coding guidelines, “Verify documented behavior against source code, tests, scripts, or existing documentation before publishing it.”

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/security/openclaw-2026.6.10-dependency-review.md` around lines 92 - 93,
The documentation overstates the helper’s pre-rebuild source-shape validation.
Update the text describing the E2E-only openclaw@2026.3.11 handling to state
only the checks actually enforced—tar@7.5.11, absence of bundledDependencies,
and absence of an npm shrinkwrap—while noting that other manifest drift is
detected later by the metadata hash.

Source: Coding guidelines

Comment on lines +74 to +77
const patchDownloader = source.indexOf("curl=");
expect(patchDownloader > patchCopy && patchDownloader < patchRun, file).toBe(
installsPatchDownloader,
);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

sed -n '1,220p' test/node-tar-dockerfile-contract.test.ts | cat -n

Repository: NVIDIA/NemoClaw

Length of output: 4510


🏁 Script executed:

rg -n --fixed-strings "curl=" Dockerfile* agents scripts test

Repository: NVIDIA/NemoClaw

Length of output: 567


🏁 Script executed:

sed -n '1,140p' Dockerfile.base | cat -n
printf '\n---\n'
sed -n '1,120p' agents/hermes/Dockerfile.base | cat -n
printf '\n---\n'
sed -n '1,120p' agents/langchain-deepagents-code/Dockerfile.base | cat -n
printf '\n---\n'
sed -n '1,120p' agents/langchain-deepagents-code/Dockerfile | cat -n

Repository: NVIDIA/NemoClaw

Length of output: 31649


🏁 Script executed:

python3 - <<'PY'
from pathlib import Path

files = [
    "Dockerfile.base",
    "Dockerfile",
    "agents/hermes/Dockerfile.base",
    "agents/hermes/Dockerfile",
    "agents/langchain-deepagents-code/Dockerfile.base",
    "agents/langchain-deepagents-code/Dockerfile",
]

def completed_stage(source: str) -> str:
    import re
    matches = list(re.finditer(r"^FROM\b", source, re.M | re.I))
    if not matches:
        raise ValueError("no FROM")
    return source[matches[-1].start():]

for file in files:
    src = Path(file).read_text()
    stage = completed_stage(src)
    patch_copy = stage.find("COPY scripts/patch-bundled-npm-tar.mts /scripts/patch-bundled-npm-tar.mts")
    patch_run = stage.find("RUN node --experimental-strip-types /scripts/patch-bundled-npm-tar.mts")
    patch_downloader = stage.find("curl=")
    print(file)
    print("  patch_copy", patch_copy)
    print("  patch_run ", patch_run)
    print("  curl=     ", patch_downloader)
    print("  expr      ", patch_downloader > patch_copy and patch_downloader < patch_run)
PY

Repository: NVIDIA/NemoClaw

Length of output: 748


Scope the curl check to the downloader command.
source.indexOf("curl=") is matching the apt package pin in agents/langchain-deepagents-code/Dockerfile.base, so this can pass without finding the downloader block. Match the actual RUN curl ... line instead, or assert that no downloader text appears when installsPatchDownloader is false.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test/node-tar-dockerfile-contract.test.ts` around lines 74 - 77, Update the
curl assertion in the Dockerfile contract test to locate the downloader command
specifically, such as the actual RUN curl line, rather than any curl=
package-pin text. Preserve the ordering check against patchCopy and patchRun,
and ensure the assertion confirms the downloader block is absent when
installsPatchDownloader is false.

Source: Path instructions

Comment on lines +443 to +481
it("rebuilds the legacy fixture archive without adding mutable lock metadata", () => {
const directory = writeLegacyCoreFixture();
const root = mkdtempSync(path.join(tmpdir(), "nemoclaw-legacy-openclaw-build-remediation-"));
temporaryDirectories.push(root);
const archivePath = path.join(root, "openclaw-2026.3.11.tgz");
packFixture(directory, archivePath);
const request = {
archivePath,
packageSpec: "openclaw@2026.3.11",
workingDirectory: path.join(root, "work"),
};
let metadataIntegrity = "";
try {
buildRemediatedOpenClawArchive({
...request,
expectedPatchedMetadataIntegrity: "sha512-deliberate-mismatch",
});
} catch (error) {
metadataIntegrity = String(error).match(/got (sha512-\S+)/u)?.[1] ?? "";
}
expect(metadataIntegrity).toMatch(/^sha512-/u);

const remediated = buildRemediatedOpenClawArchive({
...request,
expectedPatchedMetadataIntegrity: metadataIntegrity,
});
const extracted = path.join(root, "asserted");
mkdirSync(extracted, { recursive: true });
const extraction = spawnSync("tar", ["-xzf", remediated.archivePath, "-C", extracted], {
encoding: "utf8",
});
expect(extraction.status, extraction.stderr).toBe(0);
expect(existsSync(path.join(extracted, "package", "npm-shrinkwrap.json"))).toBe(false);
expect(
readJson<{ dependencies?: Record<string, string> }>(
path.join(extracted, "package", "package.json"),
).dependencies?.tar,
).toBe("7.5.19");
});

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Assert the committed metadata pin without overriding it.

The successful call supplies a hash extracted from the intentional mismatch, so this test passes even when REMEDIATIONS["openclaw@2026.3.11"].expectedPatchedMetadataIntegrity is wrong. First build with the default expected value, then keep the deliberate-mismatch assertion separately; otherwise the image remediation can fail despite this suite passing.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test/openclaw-npm-remediation.test.ts` around lines 443 - 481, The test
should validate the committed expectedPatchedMetadataIntegrity rather than
replace it with a hash extracted from the intentional mismatch. First invoke
buildRemediatedOpenClawArchive with the default remediation metadata and assert
success, then perform the deliberate-mismatch call separately to verify the
reported hash and retain the archive-content assertions using the committed
expectation.

Source: Coding guidelines

@apurvvkumaria apurvvkumaria left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I found two blocking issues, both security-boundary violations in the historical E2E fixture path:

  1. test/e2e/live/openshell-gateway-upgrade-old-installer.ts injects npm install -g "openclaw@${version}" into a Docker build without a committed integrity pin/local reviewed archive and without --ignore-scripts. That lets mutable registry resolution and package lifecycle scripts execute as root in the fixture build. Please route this through the reviewed-archive/SRI boundary (or an equivalently verified local fixture archive), keep the install local-archive-only, and disable lifecycle scripts.

  2. patchLegacyOpenClawCorePackageGraph() changes the legacy package manifest from tar@7.5.11 to tar@7.5.19, but the legacy archive has no shrinkwrap and the replacement is not bundled. The later local-archive install therefore resolves tar@7.5.19 from the registry without the committed TAR_INTEGRITY, while the provenance marker claims a transitive remediation. Please fetch/verify the replacement through packReviewedNpmArchive and bundle it, or add an equivalent locked resolved + integrity graph that is validated before install.

These are blocking because they weaken the supply-chain controls this security regression suite is meant to enforce. The existing documentation wording, curl-contract specificity, committed-metadata-pin test, zero-advisory-boundary test, and maintainability/test-hardening findings are valid but can be handled as fix-forward; I do not consider them merge blockers for this PR.

@apurvvkumaria apurvvkumaria left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-reviewed exact head 609735706b4e9317e8deca3e14ada0aefd663f5e.

The blocking privileged-install finding is addressed: the historical versions are now an explicit allowlist with committed SRI and tarball URLs, packReviewedNpmArchive verifies registry metadata and downloaded bytes, the Docker build consumes only the verified local archive with --ignore-scripts, the reviewed lifecycle step is invoked explicitly, and the installed version is checked. I independently verified all three committed integrity/tarball pairs against current npm metadata. The adapter also now fails closed unless exactly one historical advisory-audit boundary is present.

On reconsideration, the legacy openclaw@2026.3.11 transitive tar@7.5.19 pin gap should be fix-forward rather than blocking: that branch is explicitly E2E-only, production builds reject the fixture selector, the dependency version is exact, lifecycle scripts are disabled, and the resulting legacy image is not shipped as a production artifact. Bundling or lock-pinning the already-committed TAR_INTEGRITY would improve registry-compromise resistance and should still be tracked, but it does not create a direct production exposure or critical correctness failure in this PR.

I found no remaining blocking security or critical-bug finding. Current-head CI is still running and remains a separate merge gate.

@apurvvkumaria
apurvvkumaria merged commit 33f210e into main Jul 22, 2026
73 of 76 checks passed
@apurvvkumaria
apurvvkumaria deleted the codex/fix-e2e-image-regressions branch July 22, 2026 04:01
@yanyunl1991

Copy link
Copy Markdown
Contributor

This should also cover the upgrade-stale-sandbox live E2E, which currently fails on main for the same reason (I filed #7357 for it, now closed as superseded by this PR).

The failure is in the old-base build (test/e2e/live/upgrade-stale-sandbox.test.ts:87, buildOldBase.exitCode === 0): building the legacy OpenClaw 2026.3.11 fixture trips node-tar-image-scan because that pin still bundles tar@7.5.11:

ERROR: /usr/local/lib/node_modules/openclaw/node_modules/tar contains tar@7.5.11

Since this PR extends the legacy-fixture tar remediation to 2026.3.11 (Dockerfile.base / openclaw-npm-remediation.mts), the stale fixture should now be remediated before the scan. Could you confirm the upgrade-stale-sandbox target is in scope here (the summary names openshell-gateway-upgrade explicitly)? Happy to validate on hardware once it lands.

@prekshivyas

Copy link
Copy Markdown
Collaborator Author

Follow-up #7360 carries the review fixes that landed after this PR merged. It verifies and bundles reviewed tar@7.5.19 for the E2E-only OpenClaw 2026.3.11 archive (resolving Apurv’s requested security change), makes the committed metadata pin part of the successful test path, narrows the dependency-review wording, strengthens the apt/curl ordering contract, shares the historical installer markers, and proves audit-boundary failures leave the Dockerfile unchanged. The follow-up commit is GitHub-verified and its focused local gates pass.

@prekshivyas

Copy link
Copy Markdown
Collaborator Author

@yanyunl1991 Confirmed: upgrade-stale-sandbox is in scope and uses the merged remediation. Its buildOldOpenClawBase() path builds Dockerfile.base with OPENCLAW_VERSION=2026.3.11 plus the explicit legacy-fixture selector; #7355 routes that exact version through openclaw-npm-remediation.mts before node-tar-image-scan. Follow-up #7360 hardens the same path by verifying and bundling tar@7.5.19, so the later local archive install cannot re-resolve the patched tar package from mutable registry state.

cv pushed a commit that referenced this pull request Jul 22, 2026
<!-- markdownlint-disable MD041 -->
## Summary

Follow up on #7355 so the E2E-only `openclaw@2026.3.11` remediation
cannot resolve its patched tar package from mutable registry state. The
remediation now verifies and bundles the reviewed `tar@7.5.19` archive
before the historical image install.

## Related Issue

Follow-up to #7355 and Apurv's requested security fix on that PR.

## Changes

- Fetch `tar@7.5.19` through `packReviewedNpmArchive`, verify its exact
SRI and tarball URL, reject a pre-existing bundled tar directory, and
copy the verified package into the remediated OpenClaw archive.
- Declare tar as a bundled dependency and bind the OpenClaw identity,
replacement declaration, bundle marker, and tar identity to the
committed patched-metadata hash.
- Make the legacy archive test succeed with the committed hash before
exercising the mismatch path, and prove the extracted archive contains
the reviewed bundled tar package.
- Address the remaining #7355 review findings by strengthening the
apt/curl ordering contract, sharing historical installer markers,
proving audit-boundary failures do not partially rewrite the Dockerfile,
and correcting the dependency review note.

## Type of Change

- [ ] Code change (feature, bug fix, or refactor)
- [x] Code change with doc updates
- [ ] Doc only (prose changes, no code sample modifications)
- [ ] Doc only (includes code sample changes)

## Quality Gates

- [x] Tests added or updated for changed behavior
- [ ] Existing tests cover changed behavior — justification:
- [ ] Tests not applicable — justification:
- [ ] Docs updated for user-facing behavior changes
- [x] Docs not applicable — justification: This changes only legacy E2E
fixture construction; the existing security dependency review note is
updated to keep the reviewed boundary accurate.
- [x] Sensitive paths changed (security, policy, credentials, preflight,
onboarding, inference, runner, sandbox, or messaging)
- [ ] Sensitive-path review completed or maintainer-approved waiver
recorded — reviewer/approval link/justification: Maintainer security
review is pending on this PR.
- [ ] Non-success, skipped, or missing CI check accepted by maintainer —
check name, approval link, and follow-up issue:

## DGX Station Hardware Evidence

- [ ] Tested on DGX Station
- Tested commit: not applicable
- Station profile/scenario: not applicable
- Result: not applicable
- Supporting evidence: not applicable

## Verification

- [x] PR description includes a `Signed-off-by:` line and every commit
appears as `Verified` in GitHub
- [x] Normal `pre-commit`, `commit-msg`, and `pre-push` hooks passed, or
`npm run check:diff` passed when hooks were skipped or unavailable
- [x] Targeted behavior tests pass for the current change set, or tests
are marked not applicable above — integration tests: 22 passed; E2E
support tests: 7 passed; `npm run test:changed`: 8 passed; `npm run
typecheck:cli`, `npm run source-shape:check`, `npm run
test:projects:check`, and the test conditional scan passed. A real
reviewed npm archive proof produced an OpenClaw archive with
`bundledDependencies: ["tar"]` and `tar@7.5.19` under
`node_modules/tar`.
- [ ] Applicable broad gate passed — focused legacy fixture and contract
changes use the targeted evidence above; required CI is pending.
- [ ] Quality Gates section completed with required justifications or
waivers — sensitive-path review is pending.
- [x] No secrets, API keys, or credentials committed
- [ ] `npm run docs` builds without warnings (doc changes only) — the
build passed with two pre-existing Fern warnings.
- [x] Doc pages follow the [style
guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md)
(doc changes only)
- [ ] New doc pages include SPDX header and frontmatter (new pages only)

---
Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

* **Security**
* Strengthened validation and remediation of legacy OpenClaw packages,
including verified bundled `tar` package metadata and integrity checks.
* Updated the expected security integrity value for the affected
OpenClaw release.

* **Bug Fixes**
* Improved installer patching for legacy upgrade paths and preserved
Dockerfile contents when boundary validation fails.

* **Tests**
* Expanded coverage for bundled dependency contents, metadata integrity
mismatches, package installation ordering, and installer failure
scenarios.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->

Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
cv pushed a commit that referenced this pull request Jul 22, 2026
<!-- markdownlint-disable MD041 -->
## Summary

Add the canonical `v0.0.91` changelog entry that was missed before the
release tag was cut. Correct the custom-image compatibility guidance
because the tagged code still accepts the legacy inference route
selector instead of removing it in v0.0.91.

## Changes

- Add `docs/changelog/2026-07-22.mdx` with the release summary and
detailed security, rebuild, Hermes, DGX Station, and
historical-validation changes from the published v0.0.91 announcement.
- Link shipped behavior to the most specific published OpenClaw and
Hermes documentation routes.
- Correct the v0.0.90 changelog and command reference so they match the
compatibility fallback present in the tagged v0.0.91 code without
inventing a new removal version.
- Keep the immutable v0.0.91 release tag unchanged; this is the
documented post-release recovery path.

Source summary:

- [#7332](#7332),
[#7289](#7289), and
[#7294](#7294) ->
`docs/changelog/2026-07-22.mdx`: Summarize completed-image `node-tar`
remediation and current and historical container verification.
- [#7213](#7213),
[#7363](#7363),
[#7366](#7366), and
[#7369](#7369) ->
`docs/changelog/2026-07-22.mdx`: Summarize trusted base preparation,
backup reuse, deletion convergence, and rebuild confidence.
- [#7212](#7212) ->
`docs/changelog/2026-07-22.mdx`: Summarize the Hermes API bearer-token
lifecycle and supported retrieval command.
- [#7327](#7327) and
[#7328](#7328) ->
`docs/changelog/2026-07-22.mdx`: Summarize qualified DGX Station
guidance and reproducible coding-agent installation instructions.
- [#7355](#7355),
[#7360](#7360),
[#7362](#7362), and
[#7364](#7364) ->
`docs/changelog/2026-07-22.mdx`: Summarize restored historical OpenClaw
upgrade and Hermes rebuild validation.
- [#7189](#7189) ->
`docs/changelog/2026-07-20.mdx`, `docs/reference/commands.mdx`: Correct
its forward-looking removal deadline after v0.0.91 shipped with the
documented legacy fallback still present.
- [#7282](#7282),
[#7306](#7306), and
[#7341](#7341) need no additional
user-guide update because they already update their owned contributor or
user-facing text directly.

## Type of Change

- [ ] Code change (feature, bug fix, or refactor)
- [ ] Code change with doc updates
- [x] Doc only (prose changes, no code sample modifications)
- [ ] Doc only (includes code sample changes)

## Quality Gates

- [ ] Tests added or updated for changed behavior
- [x] Existing tests cover changed behavior — justification:
`test/changelog-docs.test.ts` validates dated changelog structure, SPDX
syntax, version ordering, and published routes.
- [ ] Tests not applicable — justification:
- [x] Docs updated for user-facing behavior changes
- [ ] Docs not applicable — justification:
- [ ] Sensitive paths changed (security, policy, credentials, preflight,
onboarding, inference, runner, sandbox, or messaging)
- [ ] Sensitive-path review completed or maintainer-approved waiver
recorded — reviewer/approval link/justification:
- [ ] Non-success, skipped, or missing CI check accepted by maintainer —
check name, approval link, and follow-up issue:

## DGX Station Hardware Evidence

- [ ] Tested on DGX Station
- Tested commit: Not applicable; this documentation-only PR does not
change Station preparation or runtime behavior.
- Station profile/scenario: Not applicable.
- Result: Not applicable.
- Supporting evidence: Not applicable.

## Verification

- [x] PR description includes a `Signed-off-by:` line and every commit
appears as `Verified` in GitHub
- [x] Normal `pre-commit`, `commit-msg`, and `pre-push` hooks passed, or
`npm run check:diff` passed when hooks were skipped or unavailable
- [x] Targeted behavior tests pass for the current change set, or tests
are marked not applicable above — `npx vitest run
test/changelog-docs.test.ts` (6 passed).
- [ ] Applicable broad gate passed — `npm test` for broad
runtime/test-harness changes; `npm run check` for repo-wide
validation/coverage changes — not run; this is a focused
documentation-only change.
- [x] Quality Gates section completed with required justifications or
waivers
- [x] No secrets, API keys, or credentials committed
- [ ] `npm run docs` builds without warnings (doc changes only) —
completed with 0 errors and 2 existing site-wide warnings.
- [x] Doc pages follow the [style
guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md)
(doc changes only)
- [ ] New doc pages include SPDX header and frontmatter (new pages only)
— the native changelog entry uses the required parser-safe MDX SPDX
comment and intentionally has no frontmatter.

---
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

* **Documentation**
* Updated v0.0.91 guidance for custom Dockerfiles, including continued
legacy compatibility and recommended migration to
`NEMOCLAW_INFERENCE_PROVIDER_ID`.
* Added release notes covering image security scanning, safer rebuild
behavior, token management, DGX Station guidance, and deterministic
release validation.
* Clarified that existing custom images may continue using the legacy
selector temporarily, with fallback removal planned for a future
release.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->

Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
Co-authored-by: cjagwani <cjagwani@nvidia.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: e2e End-to-end tests, nightly failures, or validation infrastructure area: security Security controls, permissions, secrets, or hardening v0.0.91 Release target

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants