fix(policy): disambiguate Hermes Slack credential routes - #10271
Merged
Conversation
Signed-off-by: Rebecca Sliter <571084+rsliter@users.noreply.github.com>
Signed-off-by: Rebecca Sliter <571084+rsliter@users.noreply.github.com>
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (4)
Included review availability: Your plan provides up to 12 included reviews per hour; 11 remain after this review. 📝 WalkthroughWalkthroughThe Slack app connections endpoint now has an explicit ChangesSlack endpoint policy
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to The change narrowly disambiguates Slack credential routing by assigning the specific app endpoint to the app provider while preserving the bot fallback, with targeted and broad checks passing; no actionable merge-blocking risk remains. Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 2 files. (2 skipped: 2 unsupported.)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
Contributor
Signed-off-by: Rebecca Sliter <571084+rsliter@users.noreply.github.com>
Contributor
PR Review Advisor — No blocking findings reportedAdvisor assessment: No blocking advisor findings reported E2E guidanceAdvisory only. A maintainer can dispatch the default E2E suite for the commit under review. Recommended E2E: None Manual-only E2E: This automated review informs maintainers. Warnings and suggestions do not require a response. A maintainer decides whether to merge. |
Signed-off-by: Rebecca Sliter <571084+rsliter@users.noreply.github.com>
Signed-off-by: Rebecca Sliter <571084+rsliter@users.noreply.github.com>
Signed-off-by: Rebecca Sliter <571084+rsliter@users.noreply.github.com>
Signed-off-by: Rebecca Sliter <571084+rsliter@users.noreply.github.com>
15 tasks
20 tasks
sandl99
reviewed
Aug 25, 2026
sandl99
left a comment
sandl99
left a comment
Collaborator
There was a problem hiding this comment.
Reviewed commit 8810327d9929669c9b7a10ce0f11377440900263 and the complete policy/test diff.
The trusted Manual PR Run is immutably bound to this commit, base fa34f2aca47a690a465d1a9dbd268e57b515c738, and workflow 4f82ea594a94456274e91fc44f85a954e0415310: https://github.com/NVIDIA/NemoClaw/actions/runs/32869481833 (correlation 45273bce-8113-4ae4-8565-53823de339c6). The dispatch receipt, candidate evidence manifests, and cleanup records match that binding. channels-stop-start-hermes passed all phases and cleanup. Open egress, Discord rebuild, and Slack failed, with cleanup passing for each.
The evidence supports the narrow path-selector implementation but not the PR's current four-job completion claim:
- Hermes Slack now completes sandbox creation, Hermes health, configuration isolation, and policy inspection. This proves the equal-specificity
slack.com:443conflict is removed. Its first failure is later: both API probes return HTTP 500 because the target still sends the old provider-shaped aliases instead of revision-scoped injected values. - Open egress and Discord rebuild now pass sandbox creation and Hermes health, then fail while applying the permissive policy: the Slack credential binding references
<sandbox>-slack-app, but that provider is not attached because Slack was not configured. The original ambiguity masked this independent inactive-provider lifecycle defect. - Hermes stop/start passed in 7m28s, including disable, rebuild, re-enable, rebuild, state validation, and terminal cleanup.
Required follow-up before approval: give the inactive Slack bindings an owner that does not leave the permissive policy referencing unattached providers, update the Slack live probe/runtime path to use the identity-bound revision-scoped credential, then rerun the same four selectors. Please also update the E2E claim and verification checkboxes to reflect the terminal evidence. The ordinary changes and growth-guardrail failures are GitHub installation API-rate-limit failures, while the advisor synthesis failed internally; they are not candidate findings but remain non-success checks.
Security review of this diff: Secrets/Credentials PASS; Input Validation/Data Sanitization PASS; Authentication/Authorization PASS; Dependencies PASS; Error Handling/Logging PASS; Cryptography/Data Protection PASS; Configuration/Security Headers PASS; Security Testing PASS for the selector shape, with live completion still blocked as described above; System Security PASS. The new path narrows credential selection and does not widen hosts, ports, methods, binaries, provider access, or secret visibility.
This is a non-approval review until the focused run is green; I am leaving it as a comment because the observed blockers are independent follow-on defects rather than a security flaw in the narrow path selector itself.
Collaborator
Author
|
Focused E2E validated this PR's Slack endpoint-specificity correction at The original equal-specificity Slack credential-binding ambiguity is absent from all four focused targets. Hermes channel stop/start passed, validating the correction for #10153 item 7. The other three targets advanced to two independent failures, so this is not a claim that the whole focused run passed:
#10271 is validated for the policy ambiguity root it claims. |
This was referenced Aug 25, 2026
wscurran
added
area: policy
cv
reviewed
Aug 25, 2026
cv
left a comment
cv
left a comment
Collaborator
There was a problem hiding this comment.
Reviewed commit 8810327. I found no blocking correctness, product-scope, or security issue. The endpoint path limits the Slack app credential to /api/apps.connections.open and preserves the bot-provider fallback. Both effective policy paths have regression coverage. Required checks pass, and the focused E2E run no longer reports the policy ambiguity. Local focused Vitest could not start because the isolated worktree has no installed dependencies. I did not install dependencies. This PR remains a draft, so I am recording the result as a comment rather than an approval.
rsliter
marked this pull request as ready for review
August 25, 2026 21:36
Collaborator
Author
|
Focused E2E validation passed for exact PR head
The original equal-specificity |
cv
approved these changes
Aug 25, 2026
cv
pushed a commit
that referenced
this pull request
Aug 27, 2026
<!-- markdownlint-disable MD041 --> ## Summary Hermes post-boot policy selection could retain repository-owned messaging presets when `enabledChannels` did not include those channels. The selector now removes those inactive presets, preserves same-name custom presets, and retains required presets for enabled channels without changing OpenClaw selection. ## Related Issue Part of #10153. Supersedes #10296. This PR preserves Charan Jagwani's protocol-aware Hermes Discord fixture correction as commit `3cf9274893309fa837c994ae15680ce9849ae185` and adds the production inactive-preset correction. ## E2E Root-Cause Claim E2E root cause: Hermes policy selection / post-boot preset reconciliation / an inactive repository-owned messaging preset survives the authoritative enabled-channel set and references an unattached provider. Source run: https://github.com/NVIDIA/NemoClaw/actions/runs/32869481833 Affected jobs: - Hermes open egress, job 97873593428 - Hermes Discord rebuild, job 97873593660 Stable signature: `credential_binding references provider '<sandbox>-slack-app', but that provider is not attached to the sandbox`. Scope: one root cause across two jobs. PR #10271 separately owns the earlier equal-specificity Slack route ambiguity. The Hermes Slack API HTTP 500 result is not part of this PR. Focused E2E selectors: `common-egress-agent-hermes-open-reference,hermes-discord`. ## Changes - Derive repository-owned messaging preset names from the channel manifest registry. - Exclude inactive Hermes messaging presets from fresh suggestions, recorded resume state, applied-preset preservation, and custom-list reconciliation. - Preserve same-name custom presets and existing OpenClaw behavior. - Merge required presets for enabled Hermes messaging channels after pruning inactive presets. - Persist the selected preset list only after gateway synchronization and post-sync readiness succeed, so failed removal remains retryable. - Add regression coverage for empty enabled channels, Discord-only and Google Chat selection, stale applied Slack state, custom ownership, failed removal and resume, custom lists, and OpenClaw compatibility. The shared pruner serves the fresh, resume, and custom selection consumers. Separate filters at each call site would duplicate the channel-to-preset authority. The focused suggestion, resume, selection, and handler tests protect these paths. ## Type of Change - [ ] Code change (feature, bug fix, or refactor) - [x] Code change with doc updates - [ ] Doc only (prose changes, no code sample modifications) - [ ] Doc only (includes code sample changes) ## Quality Gates - [x] Tests added or updated for changed behavior - [ ] Existing tests cover changed behavior. Justification: - [ ] Tests not applicable. Justification: - [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: Codex Desktop independently reviewed all nine security categories at commit under review `f53c448bdb2bc04a99c7ef7059821cec886a4eda`; result `PASS` with no findings. The latest commit is a signed empty child of `39aaae46641a133818622f441d63410905b4f574` with the identical reviewed tree. The selector narrows repository-owned Hermes messaging presets to enabled channels, preserves same-name custom policies, restores required presets after pruning, and persists the selection only after live synchronization and readiness succeed. The change does not expose credentials or widen hosts, providers, methods, protocols, or policy authority. - [ ] Non-success, skipped, or missing CI check accepted by maintainer. Check name, approval link, and follow-up issue: None accepted. Current exact-head checks are still in progress. ## DGX Station Hardware Evidence - [ ] Tested on DGX Station - Tested commit: Not applicable - Station profile/scenario: Not applicable - Result: Not applicable - Supporting evidence: Not applicable ## Verification - [x] PR description includes a `Signed-off-by:` line and every commit appears as `Verified` in GitHub - [x] `npm run validate:pr` passed for the unchanged effective patch at `0273a1deb451f2c106658c27d7d0482eb9f913f9`. Commit under review `f53c448bdb2bc04a99c7ef7059821cec886a4eda` is a signed empty child with the identical tree, based on current main `803a587752aef1896c659f0ed7373d501b3b4b24`. - [x] Targeted behavior tests pass for the current change set, or tests are marked not applicable above. For the unchanged effective patch, 177 focused deterministic tests and the added rebuild regression passed 1/1. The CLI build and typecheck, repository checks, `npm run validate:pr`, `npm run docs` with 0 errors and 2 existing warnings, and `git diff --check` passed. The current signed empty child has the identical tree and stable patch ID. - [ ] Applicable broad gate passed. Not run because this change affects one policy-selection failure class; the focused suite and `npm run validate:pr` passed. - [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) ## Documentation Writer Review - [x] Documentation writer subagent reviewed the completed changes - Result: `docs-updated` - Evidence: Reviewed the complete 24-file effective diff at commit under review `f53c448bdb2bc04a99c7ef7059821cec886a4eda` against base SHA `803a587752aef1896c659f0ed7373d501b3b4b24`. The latest PR commit is a signed empty child of `39aaae46641a133818622f441d63410905b4f574` with the same tree, and the stable patch ID matches the previously corrected diff. The four changed documentation pages accurately limit inactive built-in messaging preset removal and exact custom-policy preservation to Hermes. Shared onboarding guidance accurately states that skip mode applies required presets for enabled channels. The diagnostic, comments, and behavior-oriented test titles match the implementation. The supplied CLI build and typecheck, repository checks, `npm run validate:pr`, docs build with 0 errors and 2 existing warnings, focused deterministic tests, rebuild regression, and fresh `git diff --check` passed. - Agent: Codex documentation writer subagent (`/root/docs_review_10270`) <!-- docs-review-head-sha: f53c448 --> <!-- docs-review-agents-blob-sha: 281e79f --> --- Signed-off-by: Rebecca Sliter <571084+rsliter@users.noreply.github.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved Hermes onboarding and sandbox rebuilds by removing inactive built-in messaging presets. * Preserves enabled-channel and custom presets, including custom presets sharing names with inactive channels. * Added Google Chat support to channel-based preset selection. * Improved interrupted onboarding recovery, persistence failures, and retry behavior. * Preserves required presets in custom and skip modes for enabled messaging channels. * **Documentation** * Clarified messaging-channel behavior during onboarding and sandbox recovery. * **Tests** * Expanded coverage for onboarding, rebuilds, resume selection, custom ownership, retries, and supported channels. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Charan Jagwani <cjagwani@nvidia.com> Signed-off-by: Aaron Erickson <aerickson@nvidia.com> Signed-off-by: Rebecca Sliter <571084+rsliter@users.noreply.github.com> Co-authored-by: Rebecca Sliter <571084+rsliter@users.noreply.github.com> Co-authored-by: Charan Jagwani <cjagwani@nvidia.com> Co-authored-by: Aaron Erickson <aerickson@nvidia.com>
sandl99
pushed a commit
that referenced
this pull request
Aug 27, 2026
… 0.0.106 (#10273) ## Summary A sandbox reads its provider environment once, at boot, and the agent process inherits that read for the life of the container. Any channel credential that only becomes injectable after boot therefore never reaches the running agent, and no restart recovers it — only recreating the sandbox does. This change makes every messaging credential injectable before the agent starts, and stops the agent config from shadowing the injected value once it arrives. ## Related Issue Part of #10079. It does not close that issue: WeChat and Teams on Hermes are untouched here and are described below. ## Changes - **Bind the credential in the policy preset and apply that preset at boot.** The provider profiles are endpointless, so the binding is the only thing that makes the token injectable, and `requiredAtCreate` is what puts the preset in the boot policy rather than a post-boot apply. Without both, OpenShell withholds the credential entirely (`withholding static provider credential handle from endpointless profile`). Bindings this PR adds: - Telegram — both agents. - Teams — OpenClaw. - Slack — OpenClaw; the Hermes side landed on `main` as #10271. Discord already carried the binding on both agents before this branch. - **Pass the sandbox name through both policy preflights.** Channel presets bind `{sandboxName}-<channel>-bridge`, so composing one without a sandbox name throws. Two paths dropped the name after resolving it: - `preflightPolicyRequirements` resolves it for the sandbox inspection. - `prepareSandboxCreatePolicy` has it on the create intent, and is the path the external-authority onboarding flow takes. #10314 fixed the sibling site inside `materializeSandboxCreatePlan`; these two were still uncovered. Four tests composed presets directly and mirrored the old shape, which let the composition error escape the test body and kill a whole vitest shard. - **Stop persisting the canonical placeholder in agent config.** OpenShell 0.0.106 refuses the canonical form once a credential is identity-bound, so the shape that used to work is now the one shape the credential endpoint rejects. Removed: - OpenClaw config — `botToken` for Telegram, `botToken` and `appToken` for Slack, `appPassword` for Teams. - Hermes `~/.hermes/.env` — the Telegram, Slack, and Discord token lines. - The Slack manifest's legacy `slackRuntimeEnvAliases` normalization, which existed only to rewrite those placeholders. Each agent now reads the key from its process environment, which OpenShell fills with the revision-scoped placeholder at boot. - **Prune stale credential keys from the Hermes env file.** Hermes loads `~/.hermes/.env` with `override=True`, so a leftover canonical placeholder from an earlier onboarding shadows the injected process value and the channel stays unauthenticated. Four gaps kept that line alive: - Cleanup lived only in `applyAgentConfigAtOpenShell`, whose sole production caller returns early for any non-OpenClaw plan. The Hermes runtime applier merged env lines and never removed any. - `readEnvLineKey` read `export KEY` as the key, so an export-prefixed assignment matched nothing. - Deletion keys came from the persisted plan, so a binding naming an unrelated key could remove an operator-owned line. - A plan encoded before the credential moved to a policy binding still carries the token in `agentRender`, and rebuild refreshes only host forwards and runtime setup, so the render reintroduced the line the cleanup had just removed. The rules now live in one module both appliers use: read the key from either assignment form, take deletion authority from the channel manifest rather than persisted state, treat a rendered key as wanted only while the manifests still assign a credential to it, and visit an owned target even when the plan renders nothing into it. WeChat and Teams render their Hermes credential under a different key than the provider env key, so the assignment metadata, not the provider key, decides what survives. Each rule was checked by removing it and confirming the new tests fail. - **Wait for the first gateway mint before creating the sandbox.** `provider refresh configure` returns while the credential is still the create-time sentinel and the refresh worker mints on its own sweep, so the sandbox was booting inside that window and pinning a revision whose value is the sentinel. The poll itself accepted any status table it could parse and counted attempts only, so two failure modes also passed through: - A nonzero `provider refresh status` can still print a stale `refreshed` row, which was read as success. - Attempts do not bound the wait; one probe with no timeout can hang and the loop never reaches its cap. It now requires exit status 0 before trusting a row, gives each probe a command timeout, and stops at an overall deadline. Current requirement and consumer: Google Chat, the only channel with a gateway-minted credential. Failing closed stays correct: creating the sandbox before the first mint pins the create-time sentinel for the life of the container. The `configureMessagingBridgeRefreshes` tests cover the success and the never-minted path, and the optional `sleep` dependency is a test injection point, not a configuration surface. - **Make the Google Chat outbound preload forward the injected placeholder verbatim.** Rewriting it to the canonical form produced `credential_unavailable` on every send. - **Keep preserved Hermes env lines anchored to an enabled channel.** They were dropped whenever no enabled channel happened to render a `~/.hermes/.env` entry — which is now the common case, since the token lines are gone. - **Add two drift guards over the real policy files.** A preset that declares `credential_binding` must be `requiredAtCreate`, and a host and port declared twice must carry distinct path selectors. Each guard was checked by reintroducing the defect and confirming it fails. - **Align the Discord render assertion added by #10277.** That PR fixed the OpenClaw half; the Hermes Discord policy already bound every endpoint to `{sandboxName}-discord-bridge`, so rendering the canonical placeholder into `~/.hermes/.env` wrote the one shape the credential endpoint refuses. - **Refresh the reviewed managed-startup bundle.** `managed-startup-image-runtime.bundle` embeds the channel manifests, so the manifest changes above made `bundle:reviewed:check` fail in `static-checks`. Regenerated from the merged tree; the delta is 8 blocks, all of them the credential renders removed above plus the two `requiredAtCreate` flags. Three overlapping fixes landed on `main` while this PR was open and are merged in here: #10271 (the Hermes Slack `path` selector), #10277 (the OpenClaw half of Discord), and #10314 (binding the Discord create-path providers). This branch keeps only an explanatory comment on `slack/policy/hermes.yaml`; the behavior there is main's. #10314 fixed the `materializeSandboxCreatePlan` call site; the two preflight call sites it left uncovered are fixed here. ## Channel coverage after this change | Channel | OpenClaw | Hermes | Status | |---|---|---|---| | Slack | fixed | fixed | live, bot replied — Hermes policy selector landed separately as #10271 | | Discord | fixed | fixed | live, bot replied — OpenClaw half landed separately as #10277 | | Google Chat | fixed | fixed | live, bot replied | | Telegram | fixed | fixed | live, bot replied on both | | Teams | fixed | not covered | withholding log observed, no live run | | WeChat | not covered | not covered | not measured | | WhatsApp | unaffected | unaffected | injects no provider credential (QR pairing) | Every `fixed` row except Teams was confirmed by an actual bot reply on a freshly wiped host, not by test output alone. For Telegram, both agents were run against OpenShell 0.0.106: each sandbox booted with the revision-scoped placeholder in its agent process, the policy matched the redacted `/bot[CREDENTIAL]/` path, and the bot answered — with no denial and no credential error across five hours of OpenClaw polling and twenty minutes of Hermes polling. Out of scope here: - **WeChat** — injects a provider credential with no endpoints on the profile and no `credential_binding`. Telegram's shape, so the same withholding is expected, but it was not measured, so it is not claimed. - **Teams on Hermes** — Hermes reads `TEAMS_CLIENT_SECRET`, the provider injects `MSTEAMS_APP_PASSWORD`. A name mismatch, not the ordering defect. ## Known gaps, deliberately out of scope - **Ready-sandbox reuse does not migrate messaging config.** Both reuse branches in `sandbox-create/orchestration.ts` revalidate policy, seed presets, upsert providers, restore the dashboard, and return. A sandbox that booted without the injected provider environment cannot be repaired by pruning `~/.hermes/.env` — it needs a recreate decision in the existing drift guard beside `credentialRotation.changed`, which is a new drift signal rather than a cleanup change. Nearest coverage: the create and rebuild paths this PR fixes. - **`remove-channel` on a legacy plan leaves that channel's placeholder line behind.** `removePlanChannel()` drops the credential binding and the render together, so cleanup has no ownership evidence for the key. The residue is a placeholder rather than a credential, is inert once the provider is removed, and is pruned if the channel is added again. ## 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 - [x] Tests added or updated for changed behavior - [ ] Existing tests cover changed behavior — justification: - [ ] Tests not applicable — justification: - [x] Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging) - [ ] Sensitive-path review completed or maintainer-approved waiver recorded — reviewer/approval link/justification: outstanding; this change touches messaging credentials, network policy presets, and the onboarding provider path, so it needs a maintainer sensitive-path review before merge. - [ ] Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue: four checks are red on this branch and none of them is reachable from it. `CLI` fails its coverage gate on `src/lib/policy/commands.ts` at 88.88% against the 100% threshold that #9511 declares for `src/lib/policy/{commands,merge}.ts`, and `Required Checks` fails only because `CLI` does. `PR / Agent runtimes / Test activation` and both `PR / OpenClaw / MCP Discovery` runs fail on the same assertion, `Sandbox policy authority validation failed after creation`, in `managed-image-activation-e2e.test.ts` and `mcp-bridge.test.ts`. All four were red on #10332's own PR run before it merged, with a byte-identical coverage error, and #10332 both rewrote `src/lib/policy/commands.ts` and added its `commands.test.ts`. Bucketing open PRs by base confirms the boundary: `ac3ebe9aa` (#10384, the direct parent of #10332) passes those checks, while `1293457d3` (#10332 itself, #10392), `1effafb3f` (#10391), and `6062006e6` (this PR, #10397) all fail. This branch changes nothing under `src/lib/policy/`, and the failing image runs configure no messaging channel, so no preset from this PR is composed on that path. ## DGX Station Hardware Evidence Not applicable — `scripts/prepare-dgx-station-host.sh` is unchanged. - [ ] Tested on DGX Station - Tested commit: - Station profile/scenario: - Result: - Supporting evidence: ## Verification - [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 validate:pr` passed after refreshing `origin/main` 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 or justification: `npx vitest run --project cli src/lib/messaging src/lib/onboard/sandbox-create-plan.test.ts src/lib/onboard/messaging-bridge-provider.test.ts src/lib/onboard/policy-authority/preflight.test.ts src/lib/actions/sandbox/policy-channel-remove-flow.test.ts` — 69 files, 785 pass; `npx vitest run --project integration test/runtime/messaging test/runtime/policy test/generation test/channels/channels-add-bridge-lifecycle.test.ts test/onboard-external-policy-authority-composition.test.ts` — 77 files, 1359 pass, and 6 failures in `whatsapp-qr-compact.test.ts` that come from `qrcode` not being installed on this host; `npm run typecheck:cli`, `npm --prefix nemoclaw run typecheck`, `npm run checks:repository`, and `npm --prefix tools/mcp-tool-discovery-runtime run bundle:reviewed:check` all pass. CI confirms the branch itself: all 12 `CLI / Shard` jobs, `Static Checks`, `Build and type-check`, `Installer Integration`, and `Plugin` pass on the merged head. - [ ] Applicable broad gate passed — `npm test` for broad runtime/test-harness changes; `npm run check` for repo-wide validation/coverage changes — command/result: not applicable; this changes messaging manifests, policy presets, and one onboarding step, not the runtime, the test harness, or repo-wide validation. - [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) --- Signed-off-by: Hung Le <hple@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Security & Reliability** * Messaging credentials are injected at runtime instead of written to configuration files. * Stale credential entries are removed while unrelated environment settings are preserved. * Google Chat authentication supports revision-scoped credentials and dynamic refresh. * **Messaging Channels** * Updated Telegram, Teams, Slack, Discord, and Google Chat credential handling. * Slack access distinguishes Socket Mode from Web API traffic. * Added credential-bound network policies for Telegram and Teams. * **Onboarding** * Credential setup now waits for successful token issuance and reports clear failures. * Channel policies support sandbox-specific credential providers. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Rebecca Sliter <571084+rsliter@users.noreply.github.com> Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com> Co-authored-by: Rebecca Sliter <571084+rsliter@users.noreply.github.com> Co-authored-by: Prekshi Vyas <34834085+prekshivyas@users.noreply.github.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Prekshi Vyas <prekshiv@nvidia.com>
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
4 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
Fix the Hermes Slack policy ambiguity reported by the E2E rerun. The Slack app provider now owns the specific
/api/apps.connections.openendpoint path, while the bot provider remains the less-specific fallback for other Slack REST routes.OpenShell v0.0.106 selects request-pipeline metadata from the matching endpoint with the most-specific path. This removes the equal-specificity credential-provider conflict without combining or widening the app and bot credentials.
Related Issue
Part of #10153 and #10155.
E2E Root-Cause Claim
E2E root cause: Hermes Slack policy / sandbox creation / equal-specificity
slack.com:443routes use different credential providersSource run: https://github.com/NVIDIA/NemoClaw/actions/runs/32841422570 (run 32841422570, attempt 2)
Affected jobs:
Signature:
network policies 'slack' endpoint[0] and endpoint[1] overlap with conflicting credential_binding.provider metadataScope: one root cause across four jobs. The separate provider-profile reconciliation failure from item 7 is not part of this PR and is already present in the base branch through #10159.
Changes
Type of Change
Quality Gates
DGX Station Hardware Evidence
Verification
Signed-off-by:line and every commit appears asVerifiedin GitHubpre-commit,commit-msg, andpre-pushhooks passed, ornpm run validate:prpassed after refreshingorigin/mainwhen hooks were skipped or unavailablenpm run validate:prpassed at exact head77604a11c500223c300f908a767f059e9b9198e4. Focused E2E run 32908363585 selected onlychannels-stop-start-hermes; all six lifecycle phases and cleanup passed.npm run docsbuilds without warnings (doc changes only)Documentation Writer Review
no-docs-needed77604a11c500223c300f908a767f059e9b9198e4; focused source tests passed 131/131,npm run validate:prpassed, and exact-head focused run 32908363585 passedchannels-stop-start-hermeswith all lifecycle and cleanup phases successful.Signed-off-by: Rebecca Sliter 571084+rsliter@users.noreply.github.com
Summary by CodeRabbit