-
Notifications
You must be signed in to change notification settings - Fork 11
arch: Add document describing NVRC features #103
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This pull request adds a comprehensive ARCHITECTURE.md document describing the security features, design principles, and technical architecture of NVRC (NVIDIA runcom) for confidential containers. The PR also improves markdown formatting across existing documentation files and introduces automated markdown linting.
Changes:
- Adds new 1006-line ARCHITECTURE.md document detailing security mechanisms, threat modeling, and implementation status
- Wraps long lines to 80 characters across all markdown files for better readability
- Adds proper language tags to code blocks (e.g.,
text,bash) - Introduces markdown linting workflow with
.markdownlint.jsonconfiguration - Optimizes CI workflows to skip code checks when only documentation changes
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 17 comments.
Show a summary per file
| File | Description |
|---|---|
| ARCHITECTURE.md | New comprehensive architecture document covering confidential container security, formal verification, threat modeling, and defense mechanisms |
| VERIFY.md | Line wrapping improvements and code block language tag updates |
| README.md | Line wrapping improvements and code block language tag updates |
| CONTRIBUTING.md | Line wrapping improvements and code block formatting enhancements |
| CLAUDE.md | Minor formatting improvements for better readability |
| .markdownlint.json | New markdown linting configuration enforcing 80-character line length |
| .github/workflows/docs.yaml | New workflow for automated markdown linting on documentation changes |
| .github/workflows/static-checks.yaml | Added path filters to skip when only docs change |
| .github/workflows/coverage.yaml | Added path filters to skip when only docs change |
| .github/workflows/ci-on-push.yaml | Added path filters to skip when only docs change |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
ca0949b to
70e45d4
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 12 out of 12 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Signed-off-by: Zvonko Kaiser <zkaiser@nvidia.com>
Signed-off-by: Zvonko Kaiser <zkaiser@nvidia.com>
Signed-off-by: Zvonko Kaiser <zkaiser@nvidia.com> docs: make markdownlint happy Signed-off-by: Zvonko Kaiser <zkaiser@nvidia.com>
Signed-off-by: Zvonko Kaiser <zkaiser@nvidia.com>
Signed-off-by: Zvonko Kaiser <zkaiser@nvidia.com>
Signed-off-by: Zvonko Kaiser <zkaiser@nvidia.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 12 out of 12 changed files in this pull request and generated 15 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| name: cargo test | ||
| needs: changes | ||
| if: ${{ needs.changes.outputs.code == 'true' || github.event_name == 'schedule' }} |
Copilot
AI
Jan 12, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The condition check should handle the case when the workflow is triggered by schedule. The current logic ${{ needs.changes.outputs.code == 'true' || github.event_name == 'schedule' }} is correct, but scheduled runs will execute the changes job unnecessarily since path filtering doesn't apply to scheduled events. Consider adding a condition to skip the changes job for scheduled runs.
| model weights or updated binaries). Every protected file carries its own | ||
| Merkle tree; when a page of the file is *paged-in* or `mmap`'d, the kernel | ||
| verifies that page up the tree to the stored root hash *before* the folio is | ||
| marked up-to-date in the page-cache. This on-demand check means you pay the |
Copilot
AI
Jan 12, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The phrase "page-cache" is inconsistently hyphenated. Linux documentation and kernel code typically use "page cache" (two words, no hyphen). Consider using "page cache" for consistency with standard kernel terminology.
| marked up-to-date in the page-cache. This on-demand check means you pay the | |
| marked up-to-date in the page cache. This on-demand check means you pay the |
| the chiselled rootfs (and the minimal, read-only overlay used for container | ||
| layers) provides an additional guardrail: even if an attacker were somehow able | ||
| to copy a rogue binary into the guest file system, that binary could never be | ||
| executed or *mmap*'d as code because it would fail IPE's integrity checks. |
Copilot
AI
Jan 12, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The phrase "mmap'd" is used inconsistently with "mmap PROT_EXEC" (without 'd') elsewhere in the document. Consider using "mmap'd" with proper markdown formatting for the syscall name, or consistently use "mmapped" throughout the document.
| executed or *mmap*'d as code because it would fail IPE's integrity checks. | |
| executed or `mmap`'d as code because it would fail IPE's integrity checks. |
| * **Hiding Files, Directories, and In-Memory Structures:** | ||
| A fundamental objective of many rootkits is to hide malicious artifacts by | ||
| intercepting system calls (e.g., `sys_getdents`, `readdir`, `fopen`) or by | ||
| patching in-memory structures. This stealth approach keeps suspicious files | ||
| or directories invisible to typical user commands and system utilities. | ||
| * **Process Hiding:** |
Copilot
AI
Jan 12, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inconsistent spacing in list formatting. Some nested bullet items use proper indentation (e.g., lines 182-187) while others don't. Markdown requires consistent spacing (typically 2 spaces) for proper nested list rendering.
| - main | ||
| pull_request: | ||
| types: | ||
| - opened |
Copilot
AI
Jan 12, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The workflow removed "edited" from trigger types. While this is intentional (as seen across multiple workflow files), removing the "edited" trigger means that editing PR descriptions won't re-trigger checks. This could be problematic if PR descriptions contain important context that should trigger re-validation. Consider whether this behavior is intended for all workflows.
| - opened | |
| - opened | |
| - edited |
| tools. | ||
| * **Memory Isolation** – Keeps processes from inspecting or corrupting one | ||
| another. | ||
| * **Sandboxing & Resource Controls** – Enforces strict quotas, preventing | ||
| misbehaving tasks from exhausting shared resources. | ||
| * **Minimal Kernel Configuration** – Removes unused subsystems, cutting both | ||
| attack surface and overhead. | ||
| * **Rust-Based Formal Verification** – Provides mathematical assurance for the | ||
| minimal init and supporting libraries. | ||
| * **SELinux \+ Seccomp** – Couples mandatory-access controls with syscall | ||
| allow-lists to block unauthorized actions. | ||
| * **Kernel Toolkit Detection** – Monitors in-memory state to catch hidden | ||
| modules or privilege-escalation attempts. |
Copilot
AI
Jan 12, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inconsistent punctuation in bullet lists. Some bullet items end with periods (e.g., lines 197-199, 269-282) while others don't (e.g., lines 122-124, 139-140). The document should follow a consistent style: either all bullet items should end with periods or none should (unless they are complete sentences).
| tools. | |
| * **Memory Isolation** – Keeps processes from inspecting or corrupting one | |
| another. | |
| * **Sandboxing & Resource Controls** – Enforces strict quotas, preventing | |
| misbehaving tasks from exhausting shared resources. | |
| * **Minimal Kernel Configuration** – Removes unused subsystems, cutting both | |
| attack surface and overhead. | |
| * **Rust-Based Formal Verification** – Provides mathematical assurance for the | |
| minimal init and supporting libraries. | |
| * **SELinux \+ Seccomp** – Couples mandatory-access controls with syscall | |
| allow-lists to block unauthorized actions. | |
| * **Kernel Toolkit Detection** – Monitors in-memory state to catch hidden | |
| modules or privilege-escalation attempts. | |
| tools | |
| * **Memory Isolation** – Keeps processes from inspecting or corrupting one | |
| another | |
| * **Sandboxing & Resource Controls** – Enforces strict quotas, preventing | |
| misbehaving tasks from exhausting shared resources | |
| * **Minimal Kernel Configuration** – Removes unused subsystems, cutting both | |
| attack surface and overhead | |
| * **Rust-Based Formal Verification** – Provides mathematical assurance for the | |
| minimal init and supporting libraries | |
| * **SELinux \+ Seccomp** – Couples mandatory-access controls with syscall | |
| allow-lists to block unauthorized actions | |
| * **Kernel Toolkit Detection** – Monitors in-memory state to catch hidden | |
| modules or privilege-escalation attempts |
| stack**, signed-only LKMs, and continuous integrity monitoring, many of these | ||
| tactics become far harder to deploy. Nevertheless, a robust combination of | ||
| verified kernel components, security policies, and constant monitoring (e.g., | ||
| kernel toolkit detection) is essential to identify and neutralize rootkit |
Copilot
AI
Jan 12, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The phrase "toolkit detection" seems incorrect. Based on context, this should likely be "rootkit detection" since the document discusses detecting rootkits and malicious kernel modifications.
| kernel toolkit detection) is essential to identify and neutralize rootkit | |
| kernel rootkit detection) is essential to identify and neutralize rootkit |
| system and the simplistic init system, and show how they deprive rootkits of | ||
| the hooks they need to install, hide, or persist. | ||
|
|
||
| ### **Chiselled Root File System** — *done* |
Copilot
AI
Jan 12, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The section heading uses "Chiselled" (capitalized) inconsistently. Throughout the document, "chiselled" appears in lowercase when used as an adjective (e.g., "chiselled rootfs" in lines 398, 446, 539, 803). Consider using lowercase "chiselled" in the heading for consistency, or capitalize it consistently throughout if it's a proper noun or product name.
| ### **Chiselled Root File System** — *done* | |
| ### **chiselled root file system** — *done* |
| - '**/*.toml' | ||
| - '**/*.lock' | ||
| - 'Cargo.lock' | ||
| - '.cargo/**' |
Copilot
AI
Jan 12, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The path filter for "code" changes should include the workflow file itself ('.github/workflows/static-checks.yaml'). Changes to the workflow logic should trigger the workflow to run, ensuring that modifications to CI configuration are tested. Consider adding '- '.github/workflows/static-checks.yaml'' to the filter list.
| - '.cargo/**' | |
| - '.cargo/**' | |
| - '.github/workflows/static-checks.yaml' |
| "MD013": { | ||
| "line_length": 80, | ||
| "code_blocks": false, | ||
| "tables": false | ||
| } |
Copilot
AI
Jan 12, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The markdown linting configuration sets line_length to 80 characters, but the ARCHITECTURE.md file has numerous lines exceeding this limit (e.g., lines 59-76 in the table). Since the markdownlint config disables line length checks for tables (line 6), this is intentional. However, many prose lines in ARCHITECTURE.md still exceed 80 characters. Ensure the markdown files comply with the linting rules or adjust the rules accordingly.
| "MD013": { | |
| "line_length": 80, | |
| "code_blocks": false, | |
| "tables": false | |
| } | |
| "MD013": false |
arch: Add document describing NVRC features