docs(security): address documentation gaps for auth, credentials, and policies#1487
Open
dknos wants to merge 3 commits intoNVIDIA:mainfrom
Open
docs(security): address documentation gaps for auth, credentials, and policies#1487dknos wants to merge 3 commits intoNVIDIA:mainfrom
dknos wants to merge 3 commits intoNVIDIA:mainfrom
Conversation
Contributor
|
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 two published docs (Authentication, Credential Storage), revises network-policy docs and baseline endpoint descriptions (GitHub, npm, Telegram, Discord), and updates docs index and architecture reference to surface credential permission details and new links. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (6)
docs/network-policy/customize-network-policy.md (2)
80-80: Avoid bold emphasis on routine prose.
**cannot**reads like emphasis styling rather than a UI label/parameter; please keep this plain text unless this is intended as a formal warning style. LLM pattern detected.
As per coding guidelines, "Unnecessary bold on routine instructions... Bold is reserved for UI labels, parameter names, and genuine warnings."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/network-policy/customize-network-policy.md` at line 80, The sentence in the docs uses bold styling for routine prose ("**cannot**") which violates the guideline that bold is reserved for UI labels/parameters/warnings; update the sentence to remove the bold emphasis so it reads: "This means method and path restrictions cannot be enforced on api.github.com — the agent has full API access." Ensure "api.github.com" remains plain text and no other emphasis is introduced.
60-61: Split table cell prose to one sentence per source line.These rows currently place multiple sentences on the same source line, which hurts diff readability under the docs style rules.
As per coding guidelines, "One sentence per line in source (makes diffs readable). Flag paragraphs where multiple sentences appear on the same line."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/network-policy/customize-network-policy.md` around lines 60 - 61, The two table cell descriptions containing multiple sentences should be split so each sentence sits on its own source line to improve diff readability; update the row for the `protocol`/`rest` cell (the sentence about TLS termination and the sentence about forwarding only matching requests) and the row for the `access`/`full` cell (the sentence about creating a raw CONNECT tunnel and the sentence about when to use it) so that each sentence is on a separate line in the markdown source.docs/reference/network-policies.md (1)
99-99: Rewrite this row to avoid clause colons and multi-sentence single-line formatting.This line uses colons as general punctuation and packs multiple sentences on one source line.
As per coding guidelines, "Colons should only introduce a list. Flag colons used as general punctuation between clauses." and "One sentence per line in source (makes diffs readable)."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/reference/network-policies.md` at line 99, The row "REST API: GET, POST. WebSocket gateway: `access: full`. CDN: GET only." uses colons as general punctuation and packs multiple sentences on one line; rewrite it to remove clause colons and put one sentence per source line by replacing colons with verbs/phrases (e.g., "REST API supports GET and POST", "WebSocket gateway access is full", "CDN allows GET only") and place each sentence on its own line so the three statements are separate and colon-free.docs/deployment/authentication.md (2)
73-80: Split multi-sentence lines into one sentence per source line.The description lines here contain multiple sentences on the same source line.
As per coding guidelines, "One sentence per line in source (makes diffs readable)."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/deployment/authentication.md` around lines 73 - 80, The documentation lines for `allowInsecureAuth`, `auth.token`, and `trustedProxies` contain multiple sentences on a single source line; split each description so that each sentence is its own source line. For example, break the `allowInsecureAuth` line into one line stating how it is derived from `CHAT_UI_URL` scheme and a separate line describing behavior for `http://` vs `https://`; do the same for `auth.token` (one line for generation method `secrets.token_hex(32)`, another for uniqueness per build) and `trustedProxies` (one line for purpose, another for default values `127.0.0.1` and `::1`) so each sentence occupies its own line.
27-28: End these sentences with periods.Both label-style sentences are missing terminal periods.
As per coding guidelines, "Every sentence must end with a period."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/deployment/authentication.md` around lines 27 - 28, The two label-style sentences describing the build arg—"**Location**: Dockerfile build argument (line 59), propagated to `openclaw.json` as `dangerouslyDisableDeviceAuth`" and "**Default**: `0` (device auth enabled — secure by default)`"—need terminal periods; update those two lines in the authentication.md content so each sentence ends with a period (i.e., add a period after the backtick-quoted `dangerouslyDisableDeviceAuth` line and after the closing parenthesis on the Default line).docs/security/credential-storage.md (1)
37-37: Remove non-essential bold emphasis (LLM pattern detected).
**plaintext JSON**reads like emphasis styling rather than UI label/parameter/warning markup.As per coding guidelines, "Unnecessary bold on routine instructions ... Bold is reserved for UI labels, parameter names, and genuine warnings."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/security/credential-storage.md` at line 37, Remove the unnecessary bold markup around the phrase "plaintext JSON" in the sentence "Credentials are stored as **plaintext JSON**" by deleting the surrounding ** markers so it reads either as plain text "plaintext JSON" or use inline code-style formatting (`plaintext JSON`) if you intend to mark it as a technical term; update the sentence where the phrase "plaintext JSON" appears to avoid bold emphasis.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/deployment/authentication.md`:
- Around line 23-25: Add a 1–2 sentence introductory paragraph immediately after
the H1 "Authentication Configuration" that briefly describes what the page
covers (e.g., purpose of authentication configuration and which environment
flags like `NEMOCLAW_DISABLE_DEVICE_AUTH` are documented), so the page no longer
jumps straight from the H1 to the section heading; place the intro before the
`NEMOCLAW_DISABLE_DEVICE_AUTH` section.
- Around line 82-86: Rename the bottom section header from "Related Topics" to
"Next Steps" and keep the existing links intact; update the header string in the
markdown so the section reads "## Next Steps" (the list under the header with
links to "Security Best Practices", "Sandbox Hardening", and "Deploy to a Remote
GPU Instance" should remain unchanged).
In `@docs/security/credential-storage.md`:
- Around line 147-150: Rename the bottom section header from "Related Topics" to
"Next Steps" and keep the existing bullet links unchanged; update the Markdown
heading text "Related Topics" to "Next Steps" so the section follows the coding
guideline requiring a Next Steps section that links to related pages (the two
list items referencing Architecture and Security Best Practices should remain
as-is).
- Line 3: Update the document's top-level H1 to exactly match the frontmatter
value page: "Credential Storage Security" so the H1 text equals "Credential
Storage Security"; locate the H1 in this file (the markdown heading at the top)
and replace its current text with the frontmatter page value to ensure the H1
heading matches title.page.
- Around line 23-25: The page "Credential Storage" starts the H2 "Location"
immediately after the H1; add a one- or two-sentence introduction paragraph
immediately below the H1 "# Credential Storage" that briefly summarizes the
purpose and scope of the document (e.g., what credential storage covers and any
audience or high-level goal) so the page conforms to the guideline requiring a
short intro before subsequent sections like the "Location" heading.
---
Nitpick comments:
In `@docs/deployment/authentication.md`:
- Around line 73-80: The documentation lines for `allowInsecureAuth`,
`auth.token`, and `trustedProxies` contain multiple sentences on a single source
line; split each description so that each sentence is its own source line. For
example, break the `allowInsecureAuth` line into one line stating how it is
derived from `CHAT_UI_URL` scheme and a separate line describing behavior for
`http://` vs `https://`; do the same for `auth.token` (one line for generation
method `secrets.token_hex(32)`, another for uniqueness per build) and
`trustedProxies` (one line for purpose, another for default values `127.0.0.1`
and `::1`) so each sentence occupies its own line.
- Around line 27-28: The two label-style sentences describing the build
arg—"**Location**: Dockerfile build argument (line 59), propagated to
`openclaw.json` as `dangerouslyDisableDeviceAuth`" and "**Default**: `0` (device
auth enabled — secure by default)`"—need terminal periods; update those two
lines in the authentication.md content so each sentence ends with a period
(i.e., add a period after the backtick-quoted `dangerouslyDisableDeviceAuth`
line and after the closing parenthesis on the Default line).
In `@docs/network-policy/customize-network-policy.md`:
- Line 80: The sentence in the docs uses bold styling for routine prose
("**cannot**") which violates the guideline that bold is reserved for UI
labels/parameters/warnings; update the sentence to remove the bold emphasis so
it reads: "This means method and path restrictions cannot be enforced on
api.github.com — the agent has full API access." Ensure "api.github.com" remains
plain text and no other emphasis is introduced.
- Around line 60-61: The two table cell descriptions containing multiple
sentences should be split so each sentence sits on its own source line to
improve diff readability; update the row for the `protocol`/`rest` cell (the
sentence about TLS termination and the sentence about forwarding only matching
requests) and the row for the `access`/`full` cell (the sentence about creating
a raw CONNECT tunnel and the sentence about when to use it) so that each
sentence is on a separate line in the markdown source.
In `@docs/reference/network-policies.md`:
- Line 99: The row "REST API: GET, POST. WebSocket gateway: `access: full`. CDN:
GET only." uses colons as general punctuation and packs multiple sentences on
one line; rewrite it to remove clause colons and put one sentence per source
line by replacing colons with verbs/phrases (e.g., "REST API supports GET and
POST", "WebSocket gateway access is full", "CDN allows GET only") and place each
sentence on its own line so the three statements are separate and colon-free.
In `@docs/security/credential-storage.md`:
- Line 37: Remove the unnecessary bold markup around the phrase "plaintext JSON"
in the sentence "Credentials are stored as **plaintext JSON**" by deleting the
surrounding ** markers so it reads either as plain text "plaintext JSON" or use
inline code-style formatting (`plaintext JSON`) if you intend to mark it as a
technical term; update the sentence where the phrase "plaintext JSON" appears to
avoid bold emphasis.
🪄 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: Pro
Run ID: 9dd99901-fa91-45eb-9c0b-280e2278c445
📒 Files selected for processing (6)
docs/deployment/authentication.mddocs/index.mddocs/network-policy/customize-network-policy.mddocs/reference/architecture.mddocs/reference/network-policies.mddocs/security/credential-storage.md
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (7)
docs/network-policy/customize-network-policy.md (4)
73-74: Split table cell content onto separate lines.The table cell contains two sentences on the same line.
📝 Proposed fix
- `full` - - OpenShell creates a raw CONNECT tunnel. - No HTTP inspection — all traffic to the host:port is allowed. + - OpenShell creates a raw CONNECT tunnel. + No HTTP inspection — all traffic to the host:port is allowed. Use only when protocol-level inspection is not possible, such as `git` SSH-over-HTTPS or WebSocket upgrades.As per coding guidelines: "One sentence per line in source (makes diffs readable)."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/network-policy/customize-network-policy.md` around lines 73 - 74, The table cell currently has two sentences on one line ("OpenShell creates a raw CONNECT tunnel." and "No HTTP inspection — all traffic to the host:port is allowed."); split them so each sentence is on its own source line inside the same table cell (i.e., break the line between the sentences) to follow the "one sentence per line" guideline and make diffs readable.
68-69: Split table cell content onto separate lines.The table cell contains two sentences on the same line.
📝 Proposed fix
- `rest` - - OpenShell terminates TLS and inspects HTTP method/path against `rules`. - Only matching requests are forwarded. + - OpenShell terminates TLS and inspects HTTP method/path against `rules`. + Only matching requests are forwarded.As per coding guidelines: "One sentence per line in source (makes diffs readable)."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/network-policy/customize-network-policy.md` around lines 68 - 69, The table cell currently contains two sentences on one line: "OpenShell terminates TLS and inspects HTTP method/path against `rules`. Only matching requests are forwarded."; split them so each sentence is on its own line within the same table cell (i.e., break the line between the period after `rules` and the start of "Only matching...") to follow the one-sentence-per-line guideline.
27-28: Split sentences onto separate lines.This paragraph contains two sentences on the same line, which makes diffs harder to read.
📝 Proposed fix
-The sandbox policy is defined in a declarative YAML file in the NemoClaw repository and enforced at runtime by [NVIDIA OpenShell](https://github.com/NVIDIA/OpenShell). -NemoClaw supports both static policy changes that persist across restarts and dynamic updates applied to a running sandbox through the OpenShell CLI. +The sandbox policy is defined in a declarative YAML file in the NemoClaw repository and enforced at runtime by [NVIDIA OpenShell](https://github.com/NVIDIA/OpenShell). +NemoClaw supports both static policy changes that persist across restarts and dynamic updates applied to a running sandbox through the OpenShell CLI.As per coding guidelines: "One sentence per line in source (makes diffs readable). Flag paragraphs where multiple sentences appear on the same line."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/network-policy/customize-network-policy.md` around lines 27 - 28, The two-sentence paragraph containing "The sandbox policy is defined in a declarative YAML file in the NemoClaw repository and enforced at runtime by [NVIDIA OpenShell](https://github.com/NVIDIA/OpenShell)." and "NemoClaw supports both static policy changes that persist across restarts and dynamic updates applied to a running sandbox through the OpenShell CLI." should be split so each sentence is on its own line (one sentence per line) to make diffs readable—update the block in customize-network-policy.md accordingly.
94-96: Split sentences onto separate lines.The note contains three sentences, with two on the same line.
📝 Proposed fix
:::{note} `access: full` bypasses all HTTP-layer rules. -The `github` policy uses `access: full` because `git` requires CONNECT tunneling. -This means method and path restrictions cannot be enforced on `api.github.com` — the agent has full API access. +The `github` policy uses `access: full` because `git` requires CONNECT tunneling. +This means method and path restrictions cannot be enforced on `api.github.com` — the agent has full API access. See [Security Best Practices](../security/best-practices.md) for hardening options. :::As per coding guidelines: "One sentence per line in source (makes diffs readable)."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/network-policy/customize-network-policy.md` around lines 94 - 96, The three-sentence note about `access: full` should be split so each sentence is on its own line; find the paragraph referencing `access: full`, the `github` policy, and `api.github.com` and break it into three separate lines (one sentence per line) so "`access: full` bypasses all HTTP-layer rules." is a line, "The `github` policy uses `access: full` because `git` requires CONNECT tunneling." is a line, and "This means method and path restrictions cannot be enforced on `api.github.com` — the agent has full API access." is a line.docs/reference/network-policies.md (3)
99-101: Split table cell sentences onto separate lines.The discord rules cell contains three sentences with multiple sentences on the same line.
📝 Proposed fix
- - REST API allows GET and POST. - WebSocket gateway uses `access: full`. - CDN allows GET only. + - REST API allows GET and POST. + WebSocket gateway uses `access: full`. + CDN allows GET only.As per coding guidelines: "One sentence per line in source (makes diffs readable)."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/reference/network-policies.md` around lines 99 - 101, The table cell currently containing "REST API allows GET and POST. WebSocket gateway uses `access: full`. CDN allows GET only." should be split so each sentence is on its own source line; locate the discord rules table cell (the cell text above) and break it into three separate lines, one for "REST API allows GET and POST.", one for "WebSocket gateway uses `access: full`.", and one for "CDN allows GET only." to follow the "one sentence per line" guideline.
25-27: Split sentences onto separate lines.This introduction paragraph contains three sentences with some on the same line.
📝 Proposed fix
NemoClaw runs with a deny-by-default network policy. The sandbox can only reach endpoints that are explicitly allowed. -Any request to an unlisted destination is intercepted by OpenShell, and the operator is prompted to approve or deny it in real time through the TUI. +Any request to an unlisted destination is intercepted by OpenShell, and the operator is prompted to approve or deny it in real time through the TUI.As per coding guidelines: "One sentence per line in source (makes diffs readable)."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/reference/network-policies.md` around lines 25 - 27, The introductory paragraph containing the sentences starting "NemoClaw runs with a deny-by-default network policy.", "The sandbox can only reach endpoints that are explicitly allowed.", and "Any request to an unlisted destination is intercepted by OpenShell, and the operator is prompted to approve or deny it in real time through the TUI." should be updated so each sentence is on its own line (one-sentence-per-line) to follow the style guideline and improve diff readability; edit that paragraph in docs/reference/network-policies.md so those three sentences are split into three separate lines exactly as written.
105-110: Split note sentences onto separate lines.The note contains four sentences with some on the same line.
📝 Proposed fix
:::{note} -Endpoints with `access: full` use a raw CONNECT tunnel with no HTTP-layer inspection. -Method and path restrictions cannot be enforced on these endpoints. -The `github` policy uses `access: full` because `git` requires CONNECT tunneling. -See [Security Best Practices](../security/best-practices.md) for details on the L4-only vs L7 inspection trade-off. +Endpoints with `access: full` use a raw CONNECT tunnel with no HTTP-layer inspection. +Method and path restrictions cannot be enforced on these endpoints. +The `github` policy uses `access: full` because `git` requires CONNECT tunneling. +See [Security Best Practices](../security/best-practices.md) for details on the L4-only vs L7 inspection trade-off. :::As per coding guidelines: "One sentence per line in source (makes diffs readable)."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/reference/network-policies.md` around lines 105 - 110, The note block describing endpoints with `access: full` currently has multiple sentences on the same lines; split each sentence into its own line in that note (the block starting with "Endpoints with `access: full` use a raw CONNECT tunnel..." and including the sentence about the `github` policy and the Security Best Practices link) so that each sentence is on a separate source line for better diffs and readability.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@docs/network-policy/customize-network-policy.md`:
- Around line 73-74: The table cell currently has two sentences on one line
("OpenShell creates a raw CONNECT tunnel." and "No HTTP inspection — all traffic
to the host:port is allowed."); split them so each sentence is on its own source
line inside the same table cell (i.e., break the line between the sentences) to
follow the "one sentence per line" guideline and make diffs readable.
- Around line 68-69: The table cell currently contains two sentences on one
line: "OpenShell terminates TLS and inspects HTTP method/path against `rules`.
Only matching requests are forwarded."; split them so each sentence is on its
own line within the same table cell (i.e., break the line between the period
after `rules` and the start of "Only matching...") to follow the
one-sentence-per-line guideline.
- Around line 27-28: The two-sentence paragraph containing "The sandbox policy
is defined in a declarative YAML file in the NemoClaw repository and enforced at
runtime by [NVIDIA OpenShell](https://github.com/NVIDIA/OpenShell)." and
"NemoClaw supports both static policy changes that persist across restarts and
dynamic updates applied to a running sandbox through the OpenShell CLI." should
be split so each sentence is on its own line (one sentence per line) to make
diffs readable—update the block in customize-network-policy.md accordingly.
- Around line 94-96: The three-sentence note about `access: full` should be
split so each sentence is on its own line; find the paragraph referencing
`access: full`, the `github` policy, and `api.github.com` and break it into
three separate lines (one sentence per line) so "`access: full` bypasses all
HTTP-layer rules." is a line, "The `github` policy uses `access: full` because
`git` requires CONNECT tunneling." is a line, and "This means method and path
restrictions cannot be enforced on `api.github.com` — the agent has full API
access." is a line.
In `@docs/reference/network-policies.md`:
- Around line 99-101: The table cell currently containing "REST API allows GET
and POST. WebSocket gateway uses `access: full`. CDN allows GET only." should be
split so each sentence is on its own source line; locate the discord rules table
cell (the cell text above) and break it into three separate lines, one for "REST
API allows GET and POST.", one for "WebSocket gateway uses `access: full`.", and
one for "CDN allows GET only." to follow the "one sentence per line" guideline.
- Around line 25-27: The introductory paragraph containing the sentences
starting "NemoClaw runs with a deny-by-default network policy.", "The sandbox
can only reach endpoints that are explicitly allowed.", and "Any request to an
unlisted destination is intercepted by OpenShell, and the operator is prompted
to approve or deny it in real time through the TUI." should be updated so each
sentence is on its own line (one-sentence-per-line) to follow the style
guideline and improve diff readability; edit that paragraph in
docs/reference/network-policies.md so those three sentences are split into three
separate lines exactly as written.
- Around line 105-110: The note block describing endpoints with `access: full`
currently has multiple sentences on the same lines; split each sentence into its
own line in that note (the block starting with "Endpoints with `access: full`
use a raw CONNECT tunnel..." and including the sentence about the `github`
policy and the Security Best Practices link) so that each sentence is on a
separate source line for better diffs and readability.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 3c7fe703-d636-4673-8a0d-f1cca59dc704
📒 Files selected for processing (4)
docs/deployment/authentication.mddocs/network-policy/customize-network-policy.mddocs/reference/network-policies.mddocs/security/credential-storage.md
✅ Files skipped from review due to trivial changes (2)
- docs/security/credential-storage.md
- docs/deployment/authentication.md
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (4)
docs/network-policy/customize-network-policy.md (4)
73-75: Convert to active voice.Line 74 uses passive voice: "all traffic to the host:port is allowed."
Suggested revision for lines 73-74:
"OpenShell creates a raw CONNECT tunnel.
OpenShell allows all traffic to the host:port without HTTP inspection."As per coding guidelines: "Active voice required. Flag passive constructions."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/network-policy/customize-network-policy.md` around lines 73 - 75, Update the passive sentence following "OpenShell creates a raw CONNECT tunnel." to active voice by replacing "all traffic to the host:port is allowed." with an active construction; for example, change the second line to "OpenShell allows all traffic to the host:port without HTTP inspection." so the two lines read together as an active-voice description of the CONNECT tunnel behavior.
68-69: Convert to active voice."Only matching requests are forwarded" uses passive voice.
Suggested revision:
"OpenShell forwards only matching requests."As per coding guidelines: "Active voice required. Flag passive constructions."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/network-policy/customize-network-policy.md` around lines 68 - 69, Replace the passive sentence "Only matching requests are forwarded." with an active-voice construction; update the text following "OpenShell terminates TLS and inspects HTTP method/path against `rules`." to read "OpenShell forwards only matching requests." so the subject "OpenShell" performs the action (refer to the existing phrase "OpenShell terminates TLS and inspects HTTP method/path against `rules`" and replace the passive sentence immediately after it).
27-27: Convert to active voice.The sentence uses passive constructions ("is defined" and "enforced").
Suggested revision:
"A declarative YAML file in the NemoClaw repository defines the sandbox policy, and NVIDIA OpenShell enforces it at runtime."As per coding guidelines: "Active voice required. Flag passive constructions."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/network-policy/customize-network-policy.md` at line 27, The sentence uses passive voice ("is defined" and "enforced"); rewrite it in active voice by making the subject perform the actions — e.g., change "The sandbox policy is defined in a declarative YAML file in the NemoClaw repository and enforced at runtime by NVIDIA OpenShell." to an active construction such as "A declarative YAML file in the NemoClaw repository defines the sandbox policy, and NVIDIA OpenShell enforces it at runtime," replacing the passive phrases "is defined" and "enforced" accordingly.
96-96: Convert to active voice."method and path restrictions cannot be enforced" uses passive voice.
Suggested revision:
"This means OpenShell cannot enforce method and path restrictions onapi.github.com— the agent has full API access."As per coding guidelines: "Active voice required. Flag passive constructions."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/network-policy/customize-network-policy.md` at line 96, Replace the passive-voice sentence "method and path restrictions cannot be enforced on `api.github.com` — the agent has full API access." with an active-voice version that names the system doing the enforcing, e.g., change it to "OpenShell cannot enforce method and path restrictions on `api.github.com` — the agent has full API access." Update the sentence in the docs network-policy text (the line containing the quoted sentence) to use "OpenShell cannot enforce..." so it conforms to the active-voice guideline.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@docs/network-policy/customize-network-policy.md`:
- Around line 73-75: Update the passive sentence following "OpenShell creates a
raw CONNECT tunnel." to active voice by replacing "all traffic to the host:port
is allowed." with an active construction; for example, change the second line to
"OpenShell allows all traffic to the host:port without HTTP inspection." so the
two lines read together as an active-voice description of the CONNECT tunnel
behavior.
- Around line 68-69: Replace the passive sentence "Only matching requests are
forwarded." with an active-voice construction; update the text following
"OpenShell terminates TLS and inspects HTTP method/path against `rules`." to
read "OpenShell forwards only matching requests." so the subject "OpenShell"
performs the action (refer to the existing phrase "OpenShell terminates TLS and
inspects HTTP method/path against `rules`" and replace the passive sentence
immediately after it).
- Line 27: The sentence uses passive voice ("is defined" and "enforced");
rewrite it in active voice by making the subject perform the actions — e.g.,
change "The sandbox policy is defined in a declarative YAML file in the NemoClaw
repository and enforced at runtime by NVIDIA OpenShell." to an active
construction such as "A declarative YAML file in the NemoClaw repository defines
the sandbox policy, and NVIDIA OpenShell enforces it at runtime," replacing the
passive phrases "is defined" and "enforced" accordingly.
- Line 96: Replace the passive-voice sentence "method and path restrictions
cannot be enforced on `api.github.com` — the agent has full API access." with an
active-voice version that names the system doing the enforcing, e.g., change it
to "OpenShell cannot enforce method and path restrictions on `api.github.com` —
the agent has full API access." Update the sentence in the docs network-policy
text (the line containing the quoted sentence) to use "OpenShell cannot
enforce..." so it conforms to the active-voice guideline.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: af288416-e316-450e-bce4-54b25ed70ba1
📒 Files selected for processing (1)
docs/network-policy/customize-network-policy.md
wscurran
added
documentation
Contributor
|
✨ Thanks for submitting this pull request, which proposes a way to improve security documentation around authentication, credential storage, and policy behavior. Possibly related open issues: |
dknos
commented
Apr 6, 2026
Contributor
Author
dknos
left a comment
dknos
left a comment
There was a problem hiding this comment.
Addressed all CodeRabbit style feedback on docs/network-policy/customize-network-policy.md: rewrote passive-voice sentences to active voice (lines 27, 69, 74) and removed unnecessary bold emphasis from 'cannot'. The two doc pages originally flagged (docs/deployment/authentication.md, docs/security/credential-storage.md) were already fixed in earlier commits on this branch.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/network-policy/customize-network-policy.md (1)
18-21:⚠️ Potential issue | 🟠 MajorUse the exact SPDX header text required by the repo.
Line 19 currently uses a
2025-2026range, but the project rule requires the exact 2026 copyright line.
Please update the Markdown SPDX comment to match the required text verbatim.Suggested fix
<!-- - SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. + SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. SPDX-License-Identifier: Apache-2.0 -->As per coding guidelines, "
**/*.{js,ts,tsx,sh,md}: Every source file must include an SPDX license header ... (use HTML comments for Markdown)".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/network-policy/customize-network-policy.md` around lines 18 - 21, Replace the SPDX HTML comment header block at the top of the Markdown file so it uses the exact single-year copyright text "Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved." (i.e., change the existing "2025-2026" range to "2026") and keep the SPDX-License-Identifier line "SPDX-License-Identifier: Apache-2.0" intact; this change should be applied to the existing SPDX comment block shown in the diff.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/network-policy/customize-network-policy.md`:
- Line 27: Replace the GitHub repository link for "NVIDIA OpenShell" in the
sentence that reads "A declarative YAML file in the NemoClaw repository defines
the sandbox policy, and NVIDIA OpenShell enforces it at runtime" with the
official NVIDIA OpenShell documentation URL (use the OpenShell docs page instead
of the GitHub repo) so the Markdown conforms to the rule that links must point
to official tool documentation rather than third-party code repositories.
---
Outside diff comments:
In `@docs/network-policy/customize-network-policy.md`:
- Around line 18-21: Replace the SPDX HTML comment header block at the top of
the Markdown file so it uses the exact single-year copyright text "Copyright (c)
2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved." (i.e., change the
existing "2025-2026" range to "2026") and keep the SPDX-License-Identifier line
"SPDX-License-Identifier: Apache-2.0" intact; this change should be applied to
the existing SPDX comment block shown in the diff.
🪄 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: Pro
Run ID: f02d5b3e-dae0-4c80-b57a-27b8e96c6022
📒 Files selected for processing (1)
docs/network-policy/customize-network-policy.md
Contributor
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/network-policy/customize-network-policy.md (1)
18-21:⚠️ Potential issue | 🟠 MajorAlign SPDX header text with the required canonical form.
The Markdown SPDX header is present, but the copyright text does not match the required canonical value (
2026only).
Please update it to the exact required wording.As per coding guidelines, "
**/*.{js,ts,tsx,sh,md}: Every source file must include an SPDX license header: '... Copyright (c) 2026 ...' and '... Apache-2.0' (use HTML comments for Markdown)".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/network-policy/customize-network-policy.md` around lines 18 - 21, Update the Markdown SPDX header comment to the canonical form by changing the copyright year text to exactly "Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved." and ensure the SPDX identifier line reads "SPDX-License-Identifier: Apache-2.0" (keep the header as an HTML comment at the top of the file); replace the existing multi-year or differing text with this exact wording so the file matches the required SPDX canonical header.
🧹 Nitpick comments (1)
docs/network-policy/customize-network-policy.md (1)
170-175: Add a one-line intro underNext Steps.This H2 section currently jumps straight into bullets.
Add a short introductory sentence before the list to match docs page-structure rules.As per coding guidelines, "
docs/**: Sections use H2 and H3, each starting with an introductory sentence."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/network-policy/customize-network-policy.md` around lines 170 - 175, Add a one-line introductory sentence immediately under the "Next Steps" H2 (before the bullet list) that briefly explains the purpose of the list (e.g., "Below are recommended next actions to configure and manage and learn more about network policies."), so the section follows docs style rules; update the content right under the "Next Steps" header in the customize-network-policy.md file to include this short intro sentence before the existing bullets.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@docs/network-policy/customize-network-policy.md`:
- Around line 18-21: Update the Markdown SPDX header comment to the canonical
form by changing the copyright year text to exactly "Copyright (c) 2026 NVIDIA
CORPORATION & AFFILIATES. All rights reserved." and ensure the SPDX identifier
line reads "SPDX-License-Identifier: Apache-2.0" (keep the header as an HTML
comment at the top of the file); replace the existing multi-year or differing
text with this exact wording so the file matches the required SPDX canonical
header.
---
Nitpick comments:
In `@docs/network-policy/customize-network-policy.md`:
- Around line 170-175: Add a one-line introductory sentence immediately under
the "Next Steps" H2 (before the bullet list) that briefly explains the purpose
of the list (e.g., "Below are recommended next actions to configure and manage
and learn more about network policies."), so the section follows docs style
rules; update the content right under the "Next Steps" header in the
customize-network-policy.md file to include this short intro sentence before the
existing bullets.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 8c0b7866-306e-4c32-a4e6-ae54bd22c30d
📒 Files selected for processing (1)
docs/network-policy/customize-network-policy.md
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/network-policy/customize-network-policy.md (1)
18-21:⚠️ Potential issue | 🟠 MajorUpdate SPDX header to the required canonical text.
The Markdown SPDX block exists, but the copyright line does not match the required string (Line 19 uses
2025-2026instead of the required2026text).Proposed fix
<!-- - SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. + SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. SPDX-License-Identifier: Apache-2.0 -->As per coding guidelines, "
**/*.{js,ts,tsx,sh,md}: Every source file must include an SPDX license header: ...Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved...."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/network-policy/customize-network-policy.md` around lines 18 - 21, Update the SPDX license header block so the copyright line exactly matches the canonical required text; replace the existing "Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved." string in the SPDX header with "Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved." (i.e., edit the SPDX header block at the top of the Markdown file to use the single year 2026).
🧹 Nitpick comments (1)
docs/network-policy/customize-network-policy.md (1)
96-96: Prefer active voice in the warning sentence.Line 96 uses passive voice (“cannot be enforced”). Rephrase to active voice and address the reader directly (for example, “With
access: full, you cannot enforce method/path restrictions onapi.github.com.”).As per coding guidelines, "
docs/**: Active voice required. Flag passive constructions." and "docs/**: Second person ('you') when addressing the reader."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/network-policy/customize-network-policy.md` at line 96, The warning uses passive voice; change it to active voice addressing the reader and reference the config and host explicitly — for example, replace the sentence about method/path restrictions on api.github.com with an active sentence such as: "With access: full, you cannot enforce method and path restrictions on api.github.com." Update the sentence in the docs/customize-network-policy.md content (the line mentioning api.github.com and access: full) to this active, second-person phrasing.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@docs/network-policy/customize-network-policy.md`:
- Around line 18-21: Update the SPDX license header block so the copyright line
exactly matches the canonical required text; replace the existing "Copyright (c)
2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved." string in the
SPDX header with "Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights
reserved." (i.e., edit the SPDX header block at the top of the Markdown file to
use the single year 2026).
---
Nitpick comments:
In `@docs/network-policy/customize-network-policy.md`:
- Line 96: The warning uses passive voice; change it to active voice addressing
the reader and reference the config and host explicitly — for example, replace
the sentence about method/path restrictions on api.github.com with an active
sentence such as: "With access: full, you cannot enforce method and path
restrictions on api.github.com." Update the sentence in the
docs/customize-network-policy.md content (the line mentioning api.github.com and
access: full) to this active, second-person phrasing.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b1c30ba1-57ed-4d36-ad28-48b11e939d98
📒 Files selected for processing (1)
docs/network-policy/customize-network-policy.md
dknos
force-pushed
the
docs/security-documentation-gaps
branch
from
6b2c7ec to
b667bb4
Compare
… policies - Fix GitHub API method restriction claims to match actual policy (NVIDIA#1441) - Document NEMOCLAW_DISABLE_DEVICE_AUTH build arg and implications (NVIDIA#1443) - Add security guidance for credential storage (NVIDIA#1444) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: dknos <rneebo@gmail.com>
Add intro paragraphs, rename sections, fix formatting per review. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: dknos <rneebo@gmail.com>
Remove unnecessary bold emphasis, split multi-sentence table cells and definition list items to one sentence per line, add terminal periods to label-style sentences, and convert pipe table to list-table for better multi-line support. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: dknos <rneebo@gmail.com>
dknos
force-pushed
the
docs/security-documentation-gaps
branch
from
b667bb4 to
000e92d
Compare
cv
requested changes
Apr 9, 2026
Contributor
cv
left a comment
cv
left a comment
There was a problem hiding this comment.
Security Review — WARNING (factual error in security docs)
The PR is largely accurate and improves documentation, but one error needs fixing.
Required fix
npm_registry policy is wrong: docs/reference/network-policies.md documents npm_registry as access: full (no HTTP inspection, CONNECT tunnel). The actual default sandbox policy uses protocol: rest with GET-only rules:
- host: registry.npmjs.org
protocol: rest
tls: terminate
rules:
- allow: { method: GET, path: "/**" }The npm preset uses access: full, but the default policy does not. Documenting it as access: full overstates the agent's permissions in a security-labeled PR.
Minor
- Dockerfile line reference says "line 59", actually line 74 — consider removing line numbers (they drift)
Fixes #1444in PR body but that issue is already closed by #1592
What's good
- Authentication doc is accurate (verified against Dockerfile)
- GitHub, Telegram, Discord policies all match actual YAML
- Access modes section correctly explains
protocol: restvsaccess: full - Security warning about disabling device auth is well-placed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
access: fullpolicy (Documentation Claims GitHub API Method Restrictions That Don't Exist in Policy YAML - IssueFinder - SN 17 #1441)NEMOCLAW_DISABLE_DEVICE_AUTHbuild arg and security implications (NEMOCLAW_DISABLE_DEVICE_AUTH Build Arg Undocumented — Disables Authentication - IssueFinder - SN 19 #1443)~/.nemoclaw/credentials.jsonstorage (Credential Storage at ~/.nemoclaw/credentials.json Lacks Security Documentation - IssueFinder - SN 20 #1444)Test plan
Fixes #1441, #1443, #1444
🤖 Generated with Claude Code
Summary by CodeRabbit
Signed-off-by: dknos rneebo@gmail.com