Skip to content

fix(ci): validate installed managed plugins - #8227

Merged
apurvvkumaria merged 4 commits into
mainfrom
codex/fix-managed-plugin-validation
Aug 4, 2026
Merged

fix(ci): validate installed managed plugins#8227
apurvvkumaria merged 4 commits into
mainfrom
codex/fix-managed-plugin-validation

Conversation

@senthilr-nv

@senthilr-nv senthilr-nv commented Aug 4, 2026

Copy link
Copy Markdown
Collaborator

Summary

Managed-image publication now validates OpenClaw plugin manifests at their installed package paths. The previous guard read each wrapper project's manifest and rejected a valid image after the build completed.

Changes

  • Resolve each expected plugin under its managed npm project's node_modules directory.
  • Preserve exact package name, version, directory, regular-file, duplicate, and disabled-state validation.
  • Extend the workflow contract test to reject the wrapper-project manifest path that caused Base Images run 30916380356 to fail.

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: This change corrects an internal image-publication validation guard. It does not change a user-facing command, configuration, or supported workflow.
  • 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: Security code review passed all nine categories. The guard validates fixed package identities inside an immutable pulled image, fails closed, and rejects canonical package paths outside the managed node_modules root.
  • Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue:

Documentation Writer Review

  • Documentation writer subagent reviewed the completed changes
  • Result: no-docs-needed
  • Evidence: The review found no user-facing documentation impact or changed-text findings. The change affects only an internal image-publication guard and its regression test.
  • Agent: Codex Desktop

DGX Station Hardware Evidence

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

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 validate:pr passed after refreshing origin/main when hooks were skipped or unavailable
  • Targeted behavior tests pass for the current change set, or tests are marked not applicable above — npx --no-install vitest run test/managed-image-publication-workflow.test.ts passed 16 tests.
  • Applicable broad gate passed — npm test for broad runtime/test-harness changes; npm run check for repo-wide validation/coverage changes — command/result:
  • Quality Gates section completed with required justifications or waivers
  • No secrets, API keys, or credentials committed
  • npm run docs builds without warnings (doc changes only)
  • Doc pages follow the style guide (doc changes only)
  • New doc pages include SPDX header and frontmatter (new doc pages only)

Signed-off-by: Senthil Ravichandran senthilr@nvidia.com

Summary by CodeRabbit

  • Bug Fixes
    • Improved validation of managed plugins by checking package manifests throughout project dependency directories.
    • Ensured plugin names and versions are validated consistently across project-specific dependencies.
    • Preserved safeguards requiring each managed plugin to match exactly once and remain disabled.
    • Added checks to prevent validation workflows from omitting required package directory or manifest verification.

@senthilr-nv senthilr-nv self-assigned this Aug 4, 2026
@coderabbitai

coderabbitai Bot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

The managed-image workflow now discovers OpenClaw plugins within each project’s node_modules tree. It validates package roots and manifests, including realpath containment. Tests cover direct, scoped, symlinked, and weakened validation paths.

Changes

OpenClaw plugin validation

Layer / File(s) Summary
Package discovery and path validation
.github/workflows/managed-images.yaml, test/managed-image-publication-workflow.test.ts
The workflow locates requested packages under project-specific node_modules trees. It validates package roots, manifest paths, package names, and versions. Tests cover direct and scoped paths, including symlink escapes.
Workflow regression coverage
test/managed-image-publication-workflow.test.ts
Mutation tests reject direct project-root manifest derivation and removal of package-root validation.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

  • NVIDIA/NemoClaw#8047: Modifies the same workflow and test file for managed-image plugin validation.
  • NVIDIA/NemoClaw#8213: Adds related manifest and project-directory validation in the same workflow and test files.

Suggested labels: area: security

Suggested reviewers: jyaunches, apurvvkumaria

🚥 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
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.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: correcting CI validation for installed managed plugins.
✨ Finishing Touches 💡 1
📝 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-managed-plugin-validation

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

@github-code-quality

github-code-quality Bot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Code Coverage Overview

Languages: TypeScript

TypeScript / code-coverage/plugin

The overall coverage in commit 7c5d202 in the codex/fix-managed-pl... branch remains at 96%, unchanged from commit bef5d0b in the main branch.

TypeScript / code-coverage/cli

The overall coverage in commit 7c5d202 in the codex/fix-managed-pl... branch remains at 81%, unchanged from commit bef5d0b in the main branch.

Show a code coverage summary of the most impacted files.
File main bef5d0b codex/fix-managed-pl... 7c5d202 +/-
src/lib/policy/...ne-exclusion.ts 96% 92% -4%
src/lib/private-networks.ts 93% 90% -3%
src/lib/shields/index.ts 68% 68% 0%
src/lib/sandbox...rce-identity.ts 87% 87% 0%
src/lib/state/m...-acquisition.ts 75% 75% 0%

Updated August 04, 2026 15:54 UTC

@senthilr-nv senthilr-nv added bug-fix PR fixes a bug or regression area: ci CI workflows, checks, release automation, or GitHub Actions v0.0.102 labels Aug 4, 2026
Signed-off-by: Senthil Ravichandran <senthilr@nvidia.com>
@github-actions

github-actions Bot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

PR Review Advisor — Blocking findings reported

Advisor assessment: Blockers require maintainer review
Next action: Review the blockers below.
Findings: 1 blocker · 0 warnings · 0 suggestions

Model lanes

  • GPT-5.6 Terra (primary): Completed · high confidence · 1 blocker · 0 warnings · 0 suggestions
  • Nemotron 3 Ultra (second opinion): Completed · high confidence · 0 blockers · 2 warnings · 0 suggestions
  • Model comparison: normalized findings differ; normalized terminology decisions differ; normalized E2E selections differ; Nemotron reported 1 fewer blocker, 2 more warnings, the same number of suggestions.
4 terminology differences from the second opinion

Advisory only. These are normalized differences from the primary terminology receipt.

  • projectRoot at .github/workflows/managed-images.yaml:589: selected only by the second-opinion lane as established.
  • nodeModulesRoot at .github/workflows/managed-images.yaml:595: selected only by the second-opinion lane as established.
  • packageRelative at .github/workflows/managed-images.yaml:605: selected only by the second-opinion lane as define.
  • isStrictChildPath at test/managed-image-publication-workflow.test.ts:125: selected only by the second-opinion lane as define.
2 additional E2E selections from the second opinion

Advisory only. The primary lane did not select these E2E jobs or targets.

  • ubuntu-repo-cloud-openclaw: The completed second-opinion lane identified E2E coverage that the primary lane omitted.
  • ubuntu-repo-cloud-langchain-deepagents-code: The completed second-opinion lane identified E2E coverage that the primary lane omitted.

Second-opinion terminology and E2E selections are advisory. They do not change the primary assessment or E2E / PR Gate.

E2E guidance

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

Recommended E2E: None

Blockers

PRA-1 Blocker — Reject redirected project node_modules roots

  • Location: .github/workflows/managed-images.yaml:594
  • Category: correctness
  • Problem: The validation accepts a project whose node_modules path is a symlink to an external directory. It resolves both nodeModulesRoot and packageRoot before comparing them, so a package outside the managed project remains a child of the redirected root and counts as installed.
  • Impact: A malformed managed image can pass the publication guard while sourcing an expected plugin from outside its managed npm project, weakening the image package-location contract.
  • Fix: Before resolving containment paths, require nodeModulesRoot to be a non-symlink directory. Preserve the existing resolved package-root containment check.
  • Verification: Inspect the Node validation block with project/node_modules replaced by a symlink to an external directory; the current relative-path comparison returns an in-root path.
  • Test coverage: Add a workflow-validation regression fixture where project/node_modules is a symlink to an external directory containing an otherwise valid expected package, and assert that validation rejects it.
  • Simplification (shrink): Remove Use a single explicit root-directory guard before the existing realpath containment comparison.; use Reject nodeModulesRoot unless lstatSync(nodeModulesRoot).isDirectory() and it is not a symbolic link, then retain the current package-root checks.. Net: 2 lines.
  • Keep: Keep resolved package-root containment, package identity, duplicate, and disabled-state validation intact.
  • Evidence: .github/workflows/managed-images.yaml:594-618 constructs nodeModulesRoot, then uses its resolved path as the containment root without an lstatSync(nodeModulesRoot).isDirectory() and non-symlink check. test/managed-image-publication-workflow.test.ts:77-94 tests a scoped package path redirected outside an ordinary node_modules directory, not a node_modules directory redirected outside its project.

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: 1

🤖 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 @.github/workflows/managed-images.yaml:
- Around line 595-604: Update the package-path validation around packageRoot to
resolve both the managed node_modules root and packageRoot, then reject any
packageRoot that is not contained within that root before reading the manifest.
Preserve the existing directory, manifest-file, name, and version checks, and
add a test covering an intermediate symlink that escapes node_modules and is
rejected.
🪄 Autofix

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: 0e4d9372-56e2-42bd-bc14-4c9f75853565

📥 Commits

Reviewing files that changed from the base of the PR and between efc3499 and 1f0935b.

📒 Files selected for processing (2)
  • .github/workflows/managed-images.yaml
  • test/managed-image-publication-workflow.test.ts

Comment thread .github/workflows/managed-images.yaml Outdated
@senthilr-nv senthilr-nv added the area: packaging Packages, images, registries, installers, or distribution label Aug 4, 2026
Signed-off-by: Senthil Ravichandran <senthilr@nvidia.com>
Comment thread test/managed-image-publication-workflow.test.ts
Comment thread test/managed-image-publication-workflow.test.ts

@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.

🧹 Nitpick comments (1)
test/managed-image-publication-workflow.test.ts (1)

125-134: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy lift

Exercise the workflow validator in the symlink regression test.

isStrictChildPath duplicates the realpath containment logic in .github/workflows/managed-images.yaml. Lines 277-278 execute only this test-local copy, so the test can pass while the inline workflow validator regresses. Exercise the exact validation command with the fixture, or extract one shared helper used by both paths. Keep the source-marker assertions as supplementary contract checks.

As per path instructions, tests must provide behavioral confidence through the public boundary and must flag copied production algorithms.

Also applies to: 266-283

🤖 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/managed-image-publication-workflow.test.ts` around lines 125 - 134,
Update the symlink regression test around isStrictChildPath and the workflow
validation assertions so it exercises the exact validator command from
managed-images.yaml against the fixture, rather than only the duplicated
test-local containment logic. Prefer invoking the workflow’s public validation
boundary; otherwise extract and reuse a shared helper for both paths, while
retaining the source-marker assertions as supplementary checks.

Source: Path instructions

🤖 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.

Nitpick comments:
In `@test/managed-image-publication-workflow.test.ts`:
- Around line 125-134: Update the symlink regression test around
isStrictChildPath and the workflow validation assertions so it exercises the
exact validator command from managed-images.yaml against the fixture, rather
than only the duplicated test-local containment logic. Prefer invoking the
workflow’s public validation boundary; otherwise extract and reuse a shared
helper for both paths, while retaining the source-marker assertions as
supplementary checks.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 215056d5-b6ff-4465-b528-d874f0be9138

📥 Commits

Reviewing files that changed from the base of the PR and between 1f0935b and f69d78c.

📒 Files selected for processing (2)
  • .github/workflows/managed-images.yaml
  • test/managed-image-publication-workflow.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • .github/workflows/managed-images.yaml

Signed-off-by: Senthil Ravichandran <senthilr@nvidia.com>
@apurvvkumaria
apurvvkumaria enabled auto-merge (squash) August 4, 2026 15:35
@apurvvkumaria
apurvvkumaria merged commit 3a7899f into main Aug 4, 2026
95 of 99 checks passed
@apurvvkumaria
apurvvkumaria deleted the codex/fix-managed-plugin-validation branch August 4, 2026 15:55
@cjagwani

cjagwani commented Aug 4, 2026

Copy link
Copy Markdown
Collaborator

Release-gate status for exact head 7c5d20287853284d0e584f0b5a037cedd6bc20e4:

  • required checks are now fully green after the unrelated src/lib/tunnel/services.test.ts timeout passed on retry;
  • review decision is approved;
  • GitHub compare shows the branch is four commits behind current main.

Please hold the currency refresh until #8233 lands. Current main contains the inherited stale aiohttp==3.14.1 capability-union expectations that #8233 fixes, so merging it now would manufacture another known-red cycle. After #8233 merges, merge current main normally and let the exact-head gate rerun. No behavior change is requested here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: ci CI workflows, checks, release automation, or GitHub Actions area: packaging Packages, images, registries, installers, or distribution bug-fix PR fixes a bug or regression

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants