docs(security): address documentation gaps for auth, credentials, and policies#1487
docs(security): address documentation gaps for auth, credentials, and policies#1487dknos wants to merge 2 commits intoNVIDIA:mainfrom
Conversation
… 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>
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 3 minutes and 23 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis pull request adds and updates documentation files to clarify authentication, credential storage, and network policy behaviors. It includes new documentation pages for authentication configuration and credential storage security, updates network policy documentation to accurately reflect access mode behaviors, and corrects inconsistencies between documented and actual policy configurations. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 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}
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
docs/deployment/authentication.md
Outdated
| ## Related Topics | ||
|
|
||
| - [Security Best Practices](../security/best-practices.md) for the full gateway authentication controls reference. | ||
| - [Sandbox Hardening](sandbox-hardening.md) for container-level security measures. | ||
| - [Deploy to a Remote GPU Instance](deploy-to-remote-gpu.md) for remote deployment considerations. |
There was a problem hiding this comment.
Rename “Related Topics” to “Next Steps” for new-page structure compliance.
The bottom section should be a “Next Steps” section linking related docs.
As per coding guidelines, "A 'Next Steps' section at the bottom links to related pages."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/deployment/authentication.md` around lines 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).
Add intro paragraphs, rename sections, fix formatting per review. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
2 participants
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