fix(policy): update HuggingFace endpoint and restrict Discord DELETE method - #1540
Conversation
…method Two policy preset fixes: 1. HuggingFace (fixes NVIDIA#1453): - Replace deprecated api-inference.huggingface.co (returns HTTP 410) with router.huggingface.co (current Inference Providers API endpoint) 2. Discord (fixes NVIDIA#1433): - Remove overly broad DELETE on '/**' which allows deleting channels, roles, webhooks, and other resources an inference agent shouldn't touch - Scope DELETE to message/reaction paths only: /api/v*/channels/*/messages/* and /api/v*/channels/*/messages/*/reactions/* Signed-off-by: Benedikt Schackenberg <6381261+BenediktSchackenberg@users.noreply.github.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughDiscord preset now restricts DELETE to message- and reaction-specific endpoints; HuggingFace preset switches inference hostname from Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 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 |
There was a problem hiding this comment.
Pull request overview
Updates two network policy presets to reflect upstream API changes and tighten default permissions.
Changes:
- Update Hugging Face inference endpoint from
api-inference.huggingface.cotorouter.huggingface.co. - Restrict Discord preset
DELETEpermissions from all paths to message/reaction-specific paths.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| nemoclaw-blueprint/policies/presets/huggingface.yaml | Switches Hugging Face inference host to the current router endpoint. |
| nemoclaw-blueprint/policies/presets/discord.yaml | Narrows Discord DELETE permissions to message/reaction routes instead of /**. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| - allow: { method: DELETE, path: "/**" } | ||
| # DELETE only on message and reaction endpoints — not all paths | ||
| - allow: { method: DELETE, path: "/api/v*/channels/*/messages/*" } | ||
| - allow: { method: DELETE, path: "/api/v*/channels/*/messages/*/reactions/*" } |
There was a problem hiding this comment.
The reaction DELETE path pattern looks too narrow for Discord's reaction removal endpoints, which include an additional path segment (e.g., /reactions/{emoji}/@me or /reactions/{emoji}/{user_id}). With the current /reactions/* pattern, DELETE requests to remove a reaction will likely be blocked. Consider expanding the allowed path to include that trailing segment(s) while still keeping it scoped under the message reaction subtree.
| - allow: { method: DELETE, path: "/api/v*/channels/*/messages/*/reactions/*" } | |
| - allow: { method: DELETE, path: "/api/v*/channels/*/messages/*/reactions/*/*" } |
… segments
Discord reaction removal endpoints use the form:
DELETE /api/v*/channels/*/messages/*/reactions/{emoji}/@me
DELETE /api/v*/channels/*/messages/*/reactions/{emoji}/{user_id}
The previous /reactions/* only matched one segment after /reactions/,
which would block actual reaction DELETE calls. Changed to /reactions/*/*
to cover the emoji + target (self or user_id) segments.
Per Copilot review on NVIDIA#1540.
Signed-off-by: Benedikt Schackenberg <6381261+BenediktSchackenberg@users.noreply.github.com>
|
Fixed — changed |
|
✨ Thanks for submitting this fix, which proposes a way to update the HuggingFace endpoint and tighten Discord DELETE permissions in the network policy presets. Possibly related open issues: |
## Summary - Document `nemoclaw credentials list` and `nemoclaw credentials reset` commands in commands reference (#1597) - Add `--dry-run` flag documentation for `policy-add` (#1276) - Update policy presets table: remove `docker` (#1647), add `brave` and `brew`, update HuggingFace endpoint (#1540) - Document `NEMOCLAW_LOCAL_INFERENCE_TIMEOUT` env var for local providers (#1620) - Document `NEMOCLAW_PROXY_HOST`/`NEMOCLAW_PROXY_PORT` env vars (#1563) - Add troubleshooting entries for Docker group permissions (#1614), sandbox survival after gateway restart (#1587), and proxy configuration - Regenerate `nemoclaw-user-*` skills from updated docs ## Test plan - [x] `make docs` builds without warnings - [x] All pre-commit and pre-push hooks pass - [ ] Verify rendered pages in docs site preview 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added `nemoclaw credentials list` command to display stored credential names * Added `nemoclaw credentials reset <KEY>` command with `--yes` flag to remove credentials * Added `--dry-run` flag for policy-add to preview endpoint changes * New policy presets: `brave` and `brew` * New configuration options: `NEMOCLAW_LOCAL_INFERENCE_TIMEOUT`, `NEMOCLAW_PROXY_HOST`, and `NEMOCLAW_PROXY_PORT` * **Documentation** * Expanded troubleshooting guides for Docker permissions, sandbox connectivity, local inference timeouts, and proxy configuration <!-- end of auto-generated comment: release notes by coderabbit.ai --> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…method (NVIDIA#1540) Two policy preset fixes in one PR. ## 1. HuggingFace endpoint update (fixes NVIDIA#1453) The `api-inference.huggingface.co` endpoint referenced in `huggingface.yaml` now returns HTTP 410 Gone — HuggingFace has migrated their Inference Providers API to `router.huggingface.co`. Updated the preset to use the current endpoint. ## 2. Discord DELETE method restriction (fixes NVIDIA#1433) The Discord preset allowed `DELETE` on `/**`, which is broader than needed for an inference agent. This permits deleting channels, roles, webhooks, guild members, and other resources an agent should never touch. Scoped DELETE to message and reaction paths only: - `DELETE /api/v*/channels/*/messages/*` — delete own messages - `DELETE /api/v*/channels/*/messages/*/reactions/*` — remove reactions Signed-off-by: Benedikt Schackenberg <6381261+BenediktSchackenberg@users.noreply.github.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Chores** * Restricted DELETE requests in the Discord network preset to only specific message and reaction deletion endpoints, removing a prior broad DELETE allowance. * Updated the HuggingFace network preset to route traffic to a different external hostname for secure REST connections on port 443 while keeping existing GET/POST allowances intact. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Benedikt Schackenberg <6381261+BenediktSchackenberg@users.noreply.github.com> Co-authored-by: Carlos Villela <cvillela@nvidia.com>
## Summary - Document `nemoclaw credentials list` and `nemoclaw credentials reset` commands in commands reference (NVIDIA#1597) - Add `--dry-run` flag documentation for `policy-add` (NVIDIA#1276) - Update policy presets table: remove `docker` (NVIDIA#1647), add `brave` and `brew`, update HuggingFace endpoint (NVIDIA#1540) - Document `NEMOCLAW_LOCAL_INFERENCE_TIMEOUT` env var for local providers (NVIDIA#1620) - Document `NEMOCLAW_PROXY_HOST`/`NEMOCLAW_PROXY_PORT` env vars (NVIDIA#1563) - Add troubleshooting entries for Docker group permissions (NVIDIA#1614), sandbox survival after gateway restart (NVIDIA#1587), and proxy configuration - Regenerate `nemoclaw-user-*` skills from updated docs ## Test plan - [x] `make docs` builds without warnings - [x] All pre-commit and pre-push hooks pass - [ ] Verify rendered pages in docs site preview 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added `nemoclaw credentials list` command to display stored credential names * Added `nemoclaw credentials reset <KEY>` command with `--yes` flag to remove credentials * Added `--dry-run` flag for policy-add to preview endpoint changes * New policy presets: `brave` and `brew` * New configuration options: `NEMOCLAW_LOCAL_INFERENCE_TIMEOUT`, `NEMOCLAW_PROXY_HOST`, and `NEMOCLAW_PROXY_PORT` * **Documentation** * Expanded troubleshooting guides for Docker permissions, sandbox connectivity, local inference timeouts, and proxy configuration <!-- end of auto-generated comment: release notes by coderabbit.ai --> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
… commands (#7303) <!-- markdownlint-disable MD041 --> ## Summary The OpenClaw Discord policy preset now lets the bot manage its own application slash commands, so native slash-command deployment no longer fails with `policy_denied` (403) on startup. Before, the preset scoped `DELETE` to message and reaction endpoints only, so the bot could not delete its own stale application commands and stalled at "awaiting gateway readiness". After, the preset also allows `DELETE` on the bot's own `applications/{id}/commands`, matching the Hermes preset. ## Related Issue Fixes #7298 ## Changes - Add `DELETE /api/v*/applications/*/commands/*` to the `discord.com` REST endpoint in `src/lib/messaging/channels/discord/policy/openclaw.yaml`. The rule is scoped to the bot's own application (token-scoped `appId`); the existing `DELETE` narrowing on other paths — which blocks destructive guild-resource deletes introduced in #1540 — stays intact. This reaches parity with the Hermes preset, which already carries the same allowance. - Add a real-preset-merge test in `src/lib/onboard/initial-policy-real-policy.test.ts` asserting the merged OpenClaw Discord policy permits the application-command `DELETE` while retaining the message `DELETE`. ## Type of Change - [x] Code change (feature, bug fix, or refactor) - [ ] Code change with doc updates - [ ] Doc only (prose changes, no code sample modifications) - [ ] Doc only (includes code sample changes) ## Quality Gates <!-- Check one tests line and one docs line. Check other lines when applicable. Add every requested justification or approval reference. --> - [x] Tests added or updated for changed behavior - [ ] Existing tests cover changed behavior — justification: - [ ] Tests not applicable — justification: - [ ] Docs updated for user-facing behavior changes - [x] Docs not applicable — justification: policy preset data fix that restores the documented expected behaviour; no doc surface changes. - [x] Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging) - [x] Sensitive-path review completed or maintainer-approved waiver recorded — reviewer/approval link/justification: exact-head maintainer security review PASS: #7303 (review) - [ ] Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue: ## Documentation Writer Review - [x] Documentation writer subagent reviewed the completed changes - Result: `no-docs-needed` - Evidence: Exact-head review at `370eebe0c` against base `f1161a5e6` found only the narrow Discord application-command DELETE policy rule and its regression test. The new commit is a mechanical merge of current `main`; it adds no PR product or documentation change. The fix restores the documented Discord integration without adding user configuration, commands, or workflow changes. `git diff --check f1161a5...370eebe` passed. - Agent: `Codex Desktop` <!-- docs-review-head-sha: 370eebe --> <!-- docs-review-agents-blob-sha: 9c9b36d --> ## DGX Station Hardware Evidence <!-- Required only when scripts/prepare-dgx-station-host.sh changes. Maintainers must review the linked evidence before approving or merging. This is human-reviewed evidence, not authenticated hardware provenance. Exceptional bypasses use existing repository governance and must be documented on the PR. --> - [ ] Tested on DGX Station - Tested commit: - Station profile/scenario: - Result: - Supporting evidence: ## Verification <!-- Check each applicable item only when supported by the requested evidence. Run targeted tests once per relevant change set and rerun after later edits or hook autofixes that can affect the tested behavior. Do not rerun hook-covered checks. --> - [x] PR description includes a `Signed-off-by:` line and every commit appears as `Verified` in GitHub - [x] Normal `pre-commit`, `commit-msg`, and `pre-push` hooks passed, or `npm run check:diff` passed when hooks were skipped or unavailable - [x] Targeted behavior tests pass for the current change set, or tests are marked not applicable above — command/result: `vitest run --project cli src/lib/onboard/initial-policy-real-policy.test.ts` 9/9; `--project cli` policy-channel-policy + initial-policy + initial-policy-real-policy 64/64; `--project integration test/policies.test.ts` 90/90; `test/effective-policy-contracts.test.ts` 12/12. - [ ] Applicable broad gate passed — `npm test` for broad runtime/test-harness changes; `npm run check` for repo-wide validation/coverage changes — command/result: - [x] Quality Gates section completed with required justifications or waivers - [x] No secrets, API keys, or credentials committed - [ ] `npm run docs` builds without warnings (doc changes only) - [ ] Doc pages follow the [style guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md) (doc changes only) - [ ] New doc pages include SPDX header and frontmatter (new pages only) --- <!-- DCO sign-off is required in this PR description, and every commit must appear as Verified in GitHub. Run: git config user.name && git config user.email --> Signed-off-by: Tinson Lai <tinsonl@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Updated Discord network policy rules to permit `DELETE` requests for application-command management endpoint paths. * Refreshed the related policy comment to match the expanded deletion scope. * **Tests** * Added a new automated test asserting the initial sandbox policy’s merged Discord rules include the required `DELETE` allow path patterns and exclude overly broad/wrong delete permissions. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Tinson Lai <tinsonl@nvidia.com> Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com> Co-authored-by: Prekshi Vyas <34834085+prekshivyas@users.noreply.github.com> Co-authored-by: Prekshi Vyas <prekshiv@nvidia.com>
Two policy preset fixes in one PR.
1. HuggingFace endpoint update (fixes #1453)
The
api-inference.huggingface.coendpoint referenced inhuggingface.yamlnow returns HTTP 410 Gone — HuggingFace has migrated their Inference Providers API torouter.huggingface.co.Updated the preset to use the current endpoint.
2. Discord DELETE method restriction (fixes #1433)
The Discord preset allowed
DELETEon/**, which is broader than needed for an inference agent. This permits deleting channels, roles, webhooks, guild members, and other resources an agent should never touch.Scoped DELETE to message and reaction paths only:
DELETE /api/v*/channels/*/messages/*— delete own messagesDELETE /api/v*/channels/*/messages/*/reactions/*— remove reactionsSigned-off-by: Benedikt Schackenberg 6381261+BenediktSchackenberg@users.noreply.github.com
Summary by CodeRabbit