feat(onboard): add portable capability provisioning contract - #8393
feat(onboard): add portable capability provisioning contract#8393apurvvkumaria wants to merge 11 commits into
Conversation
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
📝 WalkthroughWalkthroughThis PR adds a capability provisioning contract with strict manifest and catalog validation. It adds recursive, platform-aware BOM resolution with deterministic SHA-256 fingerprints. Tests cover malformed inputs, dependency handling, compatibility checks, immutability, and secret-free output. ChangesCapability provisioning
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Caller
participant Resolver
participant ContractParsers
participant Fingerprint
Caller->>Resolver: Submit manifest, catalog, and target platform
Resolver->>ContractParsers: Validate manifest and catalog
ContractParsers-->>Resolver: Parsed contracts or contract error
Resolver->>Resolver: Resolve dependencies and select artifacts
Resolver->>Fingerprint: Hash unsigned BOM
Fingerprint-->>Resolver: SHA-256 fingerprint
Resolver-->>Caller: Immutable capability BOM
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage in commit 70b32f1 in the TypeScript / code-coverage/cliThe overall coverage in commit 70b32f1 in the Show a code coverage summary of the most impacted files.
Updated |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@src/lib/onboard/capability-provisioning/README.md`:
- Around line 8-12: The README’s dormant-contract section must link the
remaining runtime-provider migration work and define observable completion
criteria. Update the paragraph describing the later runtime-provider facet to
include the relevant GitHub issue or PR link, and state concrete criteria for
Docker, Podman, and MXC support before the feature becomes user-visible.
In `@src/lib/onboard/capability-provisioning/resolver.ts`:
- Around line 78-85: Replace both capability ID localeCompare comparators in the
request traversal and resolved-capabilities sorting with a locale-independent
code-unit comparison so canonical ordering is stable across locales. Add a
regression test using punctuation-bearing capability IDs and assert the
resulting canonical order.
🪄 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: 4c792beb-b000-4a45-95ce-1c24188aeb8c
📒 Files selected for processing (5)
src/lib/onboard/capability-provisioning/README.mdsrc/lib/onboard/capability-provisioning/contract.test.tssrc/lib/onboard/capability-provisioning/contract.tssrc/lib/onboard/capability-provisioning/resolver.test.tssrc/lib/onboard/capability-provisioning/resolver.ts
PR Review Advisor — InformationalAdvisor assessment: Informational / low confidence Model lanes
Second-opinion terminology and E2E selections are advisory. Live E2E does not run automatically for pull requests. E2E guidanceAdvisory only. A maintainer can dispatch the default E2E suite against this exact revision. Recommended E2E: This automated review informs maintainers. Warnings and suggestions do not require a response. A maintainer decides whether to merge. |
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
Security reviewVerdict: PASS. The current branch revision adds a dormant data contract and resolver with no runtime caller or installation mechanism. I found no security findings.
Files reviewed: src/lib/onboard/capability-provisioning/README.md, contract.ts, contract.test.ts, resolver.ts, and resolver.test.ts. Verification: focused capability tests passed 8 of 8; Biome, repository checks, gitleaks, CLI type checking, commit lint, DCO, and normal pre-push hooks passed. |
|
PRA-1 is addressed in the current branch revision. The new regression resolves equivalent manifests and catalogs in opposite input orders, uses punctuation-bearing capability IDs, and verifies identical BOM objects and fingerprints. The focused capability suites pass 8 of 8 tests. The refreshed primary advisor now reports high confidence with 0 blockers, 0 warnings, and 0 suggestions. |
|
Protected |
Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>
cjagwani
left a comment
cjagwani
left a comment
There was a problem hiding this comment.
Security review — PASS
Exact head: 8c907d60c0dfe1f4e6a5637aba88e14f460ff82f
Exact base: db31c286129e878c3356eed49f76ab259561e47e
I reviewed the complete five-file PR diff and its production-import boundary. No security findings remain.
- Input validation and trust boundaries — PASS. Manifest and catalog inputs require plain records with exact keys, bounded arrays and strings, closed agent/platform/kind enums, unique identifiers, and validated dependency graphs. Catalog data is explicitly trusted; caller-controlled commands and provider identities are not accepted.
- Authentication and authorization — PASS / not applicable. This dormant domain contract adds no command, handler, API, credential path, privilege boundary, or production caller.
- Secrets and sensitive data — PASS. The schemas contain no credential or secret fields. The resolved BOM contains only catalog-owned metadata and digest-pinned artifact references. The repository secret scan passed.
- Injection and command execution — PASS. The contract accepts no shell, package-manager, callback, executable instruction, or arbitrary destination. No subprocess or dynamic code execution was added.
- Network and SSRF — PASS / not applicable. No network request, URL fetch, socket, or provider operation was added. OCI references are syntax-validated and immutable by SHA-256 digest; this slice does not resolve or fetch them.
- Filesystem and path safety — PASS. Install prefixes are confined to
/opt/nemoclaw/capabilities/<validated-id>.PATHentries must be bounded relative paths and reject absolute paths and./..traversal segments. This slice performs no filesystem mutation. - Dependencies and supply chain — PASS. No dependency or lockfile changes were introduced. Artifact references require lowercase immutable SHA-256 digests; mutable tags are rejected.
- Cryptography and integrity — PASS. SHA-256 is used only to produce a deterministic, non-secret BOM integrity fingerprint from canonicalized validated data. Capability and policy ordering is locale-independent.
- Errors, availability, and observability — PASS. Contract sizes and string lengths are bounded, catalog gaps/cycles fail closed with typed errors, and diagnostics contain field/capability identifiers only—not secrets. The feature remains dormant with no production import or support claim.
Validation on this exact head: 8/8 focused tests; plugin and CLI builds; CLI typecheck; exact Vitest project membership (2,179 candidates across 7 projects); and the complete npm run check:diff gate, including lint, source-shape, test-size, secret scan, commit, pre-push, and version checks.
|
Final refreshed-branch validation is complete. The contributor patch remains the same five capability-provisioning files after reconciliation with current Ordinary CI and both advisor lanes are green. The protected E2E controller selected onboard repair and onboard resume, and both journeys passed with no skips or pending tests. This PR is technically ready for independent approval. It remains unmerged under the release-tag freeze. |
|
Release status update: v0.0.103 has been cut and the merge freeze is lifted. The completed technical, documentation, security, and review-thread evidence remains current. The remaining gate is independent approval; once approved, I will reconcile the branch with current main, run the fresh repository-controlled gates, and use the normal protected merge path. |
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
|
CI classification for the current branch revision:
No PR code change is indicated by either failure. The remaining repository gate is independent approval. |
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
|
Current-main refresh and local validation are complete. The contributor patch remains the same five capability-provisioning files after the signed merge from Validation:
Independent documentation review: PASS with no changes needed. The contract remains dormant with no production importer, runtime provider, CLI, configuration, or support claim. The internal README accurately describes the validated manifest, trusted catalog, immutable artifact boundary, deterministic BOM, and later provider qualification criteria. Documentation routes and generated variants pass. Current nine-category security review: PASS with no findings. The parsers require plain records, exact keys, bounded values, constrained identifiers, digest-pinned OCI references, fixed managed install prefixes, and traversal-free relative paths. Resolution fails closed for catalog gaps, cycles, unsupported agents or platforms, and version mismatches. No dependency, credential, authorization, network, filesystem-mutation, or execution surface is added; SHA-256 is used only for the deterministic non-secret BOM fingerprint. Fresh repository checks and automated reviews are now running. The remaining merge gate is independent approval; this PR will not be merged before approval and all required checks are green. |
|
CI classification for the current branch revision
No branch-specific failure is demonstrated by the current hosted logs. Independent approval is also still required. |
|
Current-branch receipt refresh for revision
The remaining broad failures span unrelated jobs and include setup/action-download service-unavailable errors during the GitHub Actions incident. I am classifying those as infrastructure failures and will not create duplicate reruns during the outage. Independent approval and all required green checks remain mandatory before merge. |
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
|
CI follow-up: I refreshed this branch with current
Local validation on the refreshed revision is green: 123 focused tests, CLI build, CLI typecheck, diff validation, DCO, signature verification, and the documentation-writer review. Fresh required CI is now running. This PR still needs repository-routed human approval before it can merge. |
|
CI follow-up is complete for the current branch revision.
The only remaining merge gate is independent human approval. |
cv
left a comment
cv
left a comment
There was a problem hiding this comment.
Do not land this dormant abstraction without a current consumer. The PR explicitly defers onboarding and runtime-provider consumption, so its schema and resolver prove only self-consistency and can drift before use. Either include the accepted current consumer or remove the contract until that work exists. Also restrict OCI artifacts to a repository-approved registry and publisher boundary; a digest from untrusted.example is immutable but still attacker-controlled. Add accepted and rejected registry tests, refresh onto current main, and rerun required checks.
|
This PR is now deferred for maintainer architecture and supply-chain policy direction. The current scope intentionally has no accepted onboarding or runtime consumer, and the repository-approved OCI registry/publisher boundary has not been specified. Choosing a consumer, removing the contract, or selecting trusted publishers would define product architecture and supply-chain policy; I will not guess those decisions in a maintainer follow-up. The current branch revision has green required checks and no unresolved review threads, so the earlier reported test failure is resolved. Work can resume once the accepted consumer/removal direction and trusted registry/publisher policy are recorded. |
|
CI follow-up: the current branch revision 70b32f1 has all five required gates passing. The earlier failures belonged to a prior revision and cleared after the current-main refresh; no PR-specific test failure remains. The merge blocker is still cv's architecture and supply-chain request: an accepted current consumer plus a repository-approved OCI registry and publisher boundary. I am deferring this PR until maintainers decide those boundaries, with no merge or bypass. |
3 participants
Summary
Add a provider-neutral contract for declaring sandbox capabilities and resolving them into a deterministic bill of materials (BOM). This establishes the abstraction needed before Docker, Podman, or another runtime provider implements capability installation.
Changes
PATHentries, named policy presets, and catalog-owned dependencies.Type of Change
Quality Gates
Documentation Writer Review
docs-updatedsrc/lib/onboard/capability-provisioning/README.mdaccurately documents the dormant capability manifest, catalog, and BOM contract, including rejection boundaries and activation prerequisites. No public documentation update is needed because no production module imports the contract, it has no onboarding caller, CLI or configuration surface, or runtime-provider implementation, and accepted epic [Epic] Support native Podman with buildless managed onboarding #7744 requires dormant slices to remain absent from public support documentation./root/docs_review_8393)DGX Station Hardware Evidence
Verification
Signed-off-by:line and every commit appears asVerifiedin GitHubpre-commit,commit-msg, andpre-pushhooks passed, ornpm run validate:prpassed after refreshingorigin/mainwhen hooks were skipped or unavailablemain, the capability contract/resolver suites and the two base-regression suites passed 123/123 tests; the CLI build and CLI typecheck passed; the effective five-file contributor diff is unchanged andgit diff --checkis clean.npm testfor broad runtime/test-harness changes;npm run checkfor repo-wide validation/coverage changes — command/result:npm run docsbuilds without warnings (doc changes only)Signed-off-by: Apurv Kumaria akumaria@nvidia.com
Summary by CodeRabbit
New Features
Security
Documentation