feat(messaging): onboard Mattermost for OpenClaw - #5592
Closed
sandl99 wants to merge 13 commits into
Closed
Conversation
Contributor
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage in the Show a code coverage summary of the most covered files.
TypeScript / code-coverage/cliThe overall coverage in the Show a code coverage summary of the most covered files.
Updated |
2 tasks
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 Mattermost as a built-in NemoClaw messaging channel using a new generated policy template mechanism. The change introduces a ChangesMattermost Channel Onboarding
Sequence DiagramssequenceDiagram
participant CLI
participant addSandboxChannel
participant applyChannelPolicyIfAvailable
participant applyGeneratedMessagingPolicyTemplates
participant policies as policies (applyPresetContent)
participant applyChannelPresetIfAvailable
CLI->>addSandboxChannel: channel="mattermost"
addSandboxChannel->>addSandboxChannel: derive preset names from manifest.policyPresets
addSandboxChannel->>applyChannelPolicyIfAvailable: sandboxName, channelName, plan
applyChannelPolicyIfAvailable->>applyGeneratedMessagingPolicyTemplates: sandboxName, plan, channelId
applyGeneratedMessagingPolicyTemplates->>policies: applyPresetContent("mattermost", content, options)
policies-->>applyGeneratedMessagingPolicyTemplates: true
applyGeneratedMessagingPolicyTemplates-->>applyChannelPolicyIfAvailable: ["mattermost"]
applyChannelPolicyIfAvailable->>applyChannelPolicyIfAvailable: refresh policy context
applyChannelPolicyIfAvailable-->>addSandboxChannel: true (templates applied)
addSandboxChannel-->>CLI: channel enabled
sequenceDiagram
participant ManifestCompiler
participant planNetworkPolicy
participant renderPolicyTemplate
participant renderTemplateUrlContext
participant readPolicyTemplate as readPolicyTemplate (mattermost.yaml)
ManifestCompiler->>planNetworkPolicy: manifests, context, inputRegistry
planNetworkPolicy->>renderPolicyTemplate: channelId="mattermost", spec, agent, inputs
renderPolicyTemplate->>renderTemplateUrlContext: baseUrl value, "http-url"
renderTemplateUrlContext-->>renderPolicyTemplate: host/port/basePath
renderPolicyTemplate->>readPolicyTemplate: "mattermost.yaml"
readPolicyTemplate-->>renderPolicyTemplate: raw YAML
renderPolicyTemplate->>renderPolicyTemplate: substitute tokens, inject binaries, parse YAML
renderPolicyTemplate-->>planNetworkPolicy: SandboxMessagingNetworkPolicyTemplatePlan
planNetworkPolicy-->>ManifestCompiler: plan with templates: [mattermost]
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related issues
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Contributor
E2E Advisor RecommendationRequired E2E: Dispatch hint: Full advisor summaryE2E Recommendation AdvisorBase: Required E2E
Optional E2E
New E2E recommendations
Dispatch hint
|
Contributor
Vitest E2E Scenario RecommendationRequired Vitest E2E scenarios: Dispatch required Vitest E2E scenarios:
Full Vitest E2E advisor summaryVitest E2E Scenario AdvisorBase: Required Vitest E2E scenarios
Optional Vitest E2E scenarios
Relevant changed files
|
Contributor
PR Review Advisor — BlockedMerge posture: Do not merge until addressed Action checklist
Findings index
🚨 Required before mergeAddress these before merging unless a maintainer explicitly overrides the advisor with rationale.
|
![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: 3
🧹 Nitpick comments (1)
nemoclaw-blueprint/policies/templates/mattermost.yaml (1)
4-37: 🧹 Nitpick | 🔵 TrivialRun the targeted
network-policy-e2eslice before merge.This change is in
nemoclaw-blueprint/policies/**, so validating deny-by-default/whitelist/hot-reload/SSRF behavior is important for release confidence.As per coding guidelines, changes under
nemoclaw-blueprint/policies/**should run thenetwork-policy-e2ejob.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@nemoclaw-blueprint/policies/templates/mattermost.yaml` around lines 4 - 37, Before merging this PR with changes to the Mattermost network policy configuration in nemoclaw-blueprint/policies/templates/mattermost.yaml, run the network-policy-e2e test slice to validate that the policy configuration correctly enforces deny-by-default behavior, whitelist rules, hot-reload functionality, and SSRF protection. This is required per coding guidelines for any changes under the nemoclaw-blueprint/policies/** directory to ensure release confidence.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/lib/messaging/channels/mattermost/hooks/reachability.ts`:
- Around line 46-47: The normalizeMattermostBaseUrl function only strips URL
suffixes but does not enforce strict validation before the baseUrl is used in
fetch requests with Authorization Bearer tokens. Add explicit validation after
the baseUrl normalization to ensure it only accepts HTTP or HTTPS schemes,
rejects URLs containing credentials, and blocks any query strings or fragment
identifiers. Apply this stricter validation at both call sites (around line
46-47 and the other location at 155-157) before the baseUrl is used in any
authenticated fetch requests.
In `@src/lib/messaging/channels/mattermost/manifest.ts`:
- Around line 89-99: The Mattermost manifest in the policyTemplates field does
not align with the standard coding guideline for channel manifests. Rename the
policyTemplates field to policyPresets to match the convention used by other
channel manifests like slack, wechat, and whatsapp. Additionally, add the
missing agentPackages field at the same level as policyPresets to complete the
required manifest structure that includes auth, inputs, credentials,
policyPresets, render, runtime, agentPackages, state, and hooks.
In `@test/channels-add-generated-policy.test.ts`:
- Around line 20-27: The env object in the spawned-script invocation spreads all
of process.env which can leak unrelated messaging credentials like TELEGRAM_*,
DISCORD_* and cause test flakiness. Remove the spread operator (...process.env)
from the env object initialization and instead explicitly include only the
specific environment variables needed for this test (such as HOME,
NEMOCLAW_NON_INTERACTIVE, NEMOCLAW_SKIP_MATTERMOST_AUTH_VALIDATION,
MATTERMOST_BOT_TOKEN, and MATTERMOST_URL). This makes the test hermetic by
preventing unrelated environment variables from affecting the spawned process
behavior.
---
Nitpick comments:
In `@nemoclaw-blueprint/policies/templates/mattermost.yaml`:
- Around line 4-37: Before merging this PR with changes to the Mattermost
network policy configuration in
nemoclaw-blueprint/policies/templates/mattermost.yaml, run the
network-policy-e2e test slice to validate that the policy configuration
correctly enforces deny-by-default behavior, whitelist rules, hot-reload
functionality, and SSRF protection. This is required per coding guidelines for
any changes under the nemoclaw-blueprint/policies/** directory to ensure release
confidence.
🪄 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: Enterprise
Run ID: 9aac4e67-211e-4422-a0a0-9ca047855288
📒 Files selected for processing (40)
agents/openclaw/manifest.yamlci/test-file-size-budget.jsonnemoclaw-blueprint/policies/templates/mattermost.yamlsrc/lib/actions/sandbox/policy-channel.tssrc/lib/agent/defs.test.tssrc/lib/messaging-channel-config.test.tssrc/lib/messaging/applier/host-state-applier.tssrc/lib/messaging/applier/policy.tssrc/lib/messaging/applier/setup-applier.test.tssrc/lib/messaging/applier/types.tssrc/lib/messaging/channels/built-ins.tssrc/lib/messaging/channels/manifests.test.tssrc/lib/messaging/channels/mattermost/hooks/index.tssrc/lib/messaging/channels/mattermost/hooks/reachability.test.tssrc/lib/messaging/channels/mattermost/hooks/reachability.tssrc/lib/messaging/channels/mattermost/manifest.tssrc/lib/messaging/channels/mattermost/template-resolver.test.tssrc/lib/messaging/channels/mattermost/template-resolver.tssrc/lib/messaging/channels/metadata.test.tssrc/lib/messaging/channels/metadata.tssrc/lib/messaging/channels/template-resolver.tssrc/lib/messaging/compiler/engines/policy-resolver.tssrc/lib/messaging/compiler/engines/policy-template-renderer.test.tssrc/lib/messaging/compiler/engines/policy-template-renderer.tssrc/lib/messaging/compiler/manifest-compiler.test.tssrc/lib/messaging/compiler/manifest-compiler.tssrc/lib/messaging/compiler/workflow-planner.tssrc/lib/messaging/diagnostics.test.tssrc/lib/messaging/diagnostics.tssrc/lib/messaging/hooks/builtins.tssrc/lib/messaging/hooks/hook-runner.test.tssrc/lib/messaging/manifest/types.tssrc/lib/messaging/persistence.tssrc/lib/onboard.tssrc/lib/onboard/messaging-policy-templates.tssrc/lib/onboard/messaging-prep.test.tssrc/lib/sandbox/channels.test.tstest/channels-add-generated-policy.test.tstest/channels-add-preset.test.tstest/sandbox-provider-cleanup.test.ts
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/lib/onboard/policy-selection.ts`:
- Line 474: The call to applyGeneratedMessagingPolicyTemplatesFromRegistry is
currently placed after the syncPresetSelection call, but the function exits
early when policyMode is set to skip, none, or no (around line 408), meaning
generated messaging templates never get applied in non-interactive onboard
paths. Move the applyGeneratedMessagingPolicyTemplatesFromRegistry call to
execute before the policy mode exit condition so that generated templates are
always applied to selected channels regardless of the skip mode, ensuring
channels receive required network policy rules.
🪄 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: Enterprise
Run ID: f660a666-35e8-4d0e-8583-da4e9822d0f1
📒 Files selected for processing (1)
src/lib/onboard/policy-selection.ts
14 tasks
sandl99
added
enhancement
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
…ariable' Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com>
Signed-off-by: San Dang <sdang@nvidia.com>
10 tasks
Signed-off-by: San Dang <sdang@nvidia.com>
Signed-off-by: San Dang <sdang@nvidia.com>
Signed-off-by: San Dang <sdang@nvidia.com>
…rd-mattermost-openclaw
sandl99
changed the base branch from
main
to
fix/openclaw-fetch-guard-default-proxy
June 24, 2026 07:57
5 tasks
sandl99
marked this pull request as draft
June 26, 2026 05:58
|
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. |
This was referenced Jun 27, 2026
cv
added a commit
that referenced
this pull request
Jun 30, 2026
## Summary Broadens the OpenClaw compatibility patch so `resolveGuardedFetchMode()` defaults omitted guarded-fetch modes to `trusted_env_proxy` inside OpenShell sandboxes. This avoids local DNS lookups for bare `fetchWithSsrFGuard` callers while preserving explicit modes and strict behavior outside `OPENSHELL_SANDBOX=1`. ## Related Issue Related: #396 (fetch-guard/local DNS portion only; Gemini policy/docs remain out of scope) Fixes #4687 Related: #5129 Related: #5592 ## Changes - Patch OpenClaw's central guarded-fetch mode resolver in the sandbox image instead of rewriting individual call sites. - Remove the deprecated `proxy: "env"` compatibility branch from the patched resolver and drop the old cron-preflight call-site patch. - Cover additional bare `fetchWithSsrFGuard` users, including the Mattermost/OpenClaw call site discussed in #5592. - Update fetch-guard patch regression tests for sandbox defaults, explicit-mode precedence, out-of-sandbox strict behavior, and removal of the narrower Patch 6 rewrite. ## Localized compatibility patch review - **Invalid state:** OpenClaw 2026.5.27 resolves an omitted guarded-fetch mode to strict DNS pinning. In an OpenShell proxy-only sandbox, that performs local DNS before proxy dispatch, so sandbox-only and proxy-routed names fail before policy enforcement. - **Source boundary:** NemoClaw consumes the pinned compiled OpenClaw distribution in the sandbox image; it does not own the upstream resolver source. The image build therefore classifies and patches the reviewed `resolveGuardedFetchMode()` shape centrally and fails closed if that shape drifts. - **Source-fix constraint:** The durable fix belongs in OpenClaw: either select `trusted_env_proxy` for OpenShell sandboxes at the resolver boundary or make every sandbox-sensitive caller pass that mode explicitly. A permanent NemoClaw fork of the third-party runtime is out of scope. - **Regression proof:** `test/fetch-guard-patch-regression.test.ts` verifies the reviewed resolver shape, explicit-mode precedence, sandbox-only default, strict behavior outside the sandbox, removal of the deprecated opt-in, representative SSRF denials, and fail-closed drift handling. The exact merged head passes all 23 focused tests. - **Removal condition:** Remove Patch 4 when the pinned OpenClaw release provides an equivalent OpenShell-aware default, or when every sandbox-sensitive caller explicitly selects trusted proxy mode and the runtime acceptance checks pass without this patch. ## Security boundary and E2E Inspection of the pinned OpenClaw 2026.5.27 implementation confirms that explicit modes still override the default, hostname validation runs before proxy dispatcher construction, loopback/metadata/private/special-use IPv4 and IPv6 literals remain rejected, and redirects are handled manually and revalidated on each hop. DNS-resolution enforcement intentionally moves to the OpenShell L7 proxy and network-policy boundary only for omitted-mode calls inside the sandbox. A unified exact-head E2E dispatch covers `cron-preflight-inference-local`, `inference-routing`, `network-policy`, and `common-egress-agent`: https://github.com/NVIDIA/NemoClaw/actions/runs/28424214692 ## 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) ## Verification - [x] PR description includes the DCO sign-off declaration and every commit appears as `Verified` in GitHub - [x] Normal commit and push hooks passed on the current head - [x] Targeted test passes: 23/23 - [x] Tests added or updated for changed behavior - [x] No secrets, API keys, or credentials committed - [ ] Exact-head unified E2E is passing (in progress) --- Signed-off-by: San Dang <sdang@nvidia.com> --------- Signed-off-by: San Dang <sdang@nvidia.com> Signed-off-by: Carlos Villela <cvillela@nvidia.com> Co-authored-by: Carlos Villela <cvillela@nvidia.com>
21 tasks
Hadar301
pushed a commit
to Hadar301/NemoClaw-OpenShift
that referenced
this pull request
Jul 12, 2026
…#5713) ## Summary Broadens the OpenClaw compatibility patch so `resolveGuardedFetchMode()` defaults omitted guarded-fetch modes to `trusted_env_proxy` inside OpenShell sandboxes. This avoids local DNS lookups for bare `fetchWithSsrFGuard` callers while preserving explicit modes and strict behavior outside `OPENSHELL_SANDBOX=1`. ## Related Issue Related: NVIDIA#396 (fetch-guard/local DNS portion only; Gemini policy/docs remain out of scope) Fixes NVIDIA#4687 Related: NVIDIA#5129 Related: NVIDIA#5592 ## Changes - Patch OpenClaw's central guarded-fetch mode resolver in the sandbox image instead of rewriting individual call sites. - Remove the deprecated `proxy: "env"` compatibility branch from the patched resolver and drop the old cron-preflight call-site patch. - Cover additional bare `fetchWithSsrFGuard` users, including the Mattermost/OpenClaw call site discussed in NVIDIA#5592. - Update fetch-guard patch regression tests for sandbox defaults, explicit-mode precedence, out-of-sandbox strict behavior, and removal of the narrower Patch 6 rewrite. ## Localized compatibility patch review - **Invalid state:** OpenClaw 2026.5.27 resolves an omitted guarded-fetch mode to strict DNS pinning. In an OpenShell proxy-only sandbox, that performs local DNS before proxy dispatch, so sandbox-only and proxy-routed names fail before policy enforcement. - **Source boundary:** NemoClaw consumes the pinned compiled OpenClaw distribution in the sandbox image; it does not own the upstream resolver source. The image build therefore classifies and patches the reviewed `resolveGuardedFetchMode()` shape centrally and fails closed if that shape drifts. - **Source-fix constraint:** The durable fix belongs in OpenClaw: either select `trusted_env_proxy` for OpenShell sandboxes at the resolver boundary or make every sandbox-sensitive caller pass that mode explicitly. A permanent NemoClaw fork of the third-party runtime is out of scope. - **Regression proof:** `test/fetch-guard-patch-regression.test.ts` verifies the reviewed resolver shape, explicit-mode precedence, sandbox-only default, strict behavior outside the sandbox, removal of the deprecated opt-in, representative SSRF denials, and fail-closed drift handling. The exact merged head passes all 23 focused tests. - **Removal condition:** Remove Patch 4 when the pinned OpenClaw release provides an equivalent OpenShell-aware default, or when every sandbox-sensitive caller explicitly selects trusted proxy mode and the runtime acceptance checks pass without this patch. ## Security boundary and E2E Inspection of the pinned OpenClaw 2026.5.27 implementation confirms that explicit modes still override the default, hostname validation runs before proxy dispatcher construction, loopback/metadata/private/special-use IPv4 and IPv6 literals remain rejected, and redirects are handled manually and revalidated on each hop. DNS-resolution enforcement intentionally moves to the OpenShell L7 proxy and network-policy boundary only for omitted-mode calls inside the sandbox. A unified exact-head E2E dispatch covers `cron-preflight-inference-local`, `inference-routing`, `network-policy`, and `common-egress-agent`: https://github.com/NVIDIA/NemoClaw/actions/runs/28424214692 ## 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) ## Verification - [x] PR description includes the DCO sign-off declaration and every commit appears as `Verified` in GitHub - [x] Normal commit and push hooks passed on the current head - [x] Targeted test passes: 23/23 - [x] Tests added or updated for changed behavior - [x] No secrets, API keys, or credentials committed - [ ] Exact-head unified E2E is passing (in progress) --- Signed-off-by: San Dang <sdang@nvidia.com> --------- Signed-off-by: San Dang <sdang@nvidia.com> Signed-off-by: Carlos Villela <cvillela@nvidia.com> Co-authored-by: Carlos Villela <cvillela@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
1 participant
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
Onboards Mattermost as an OpenClaw messaging channel using the manifest-first messaging pipeline. The integration supports self-hosted HTTP/HTTPS Mattermost URLs, generated network policy templates, bot token reachability checks, and OpenClaw config rendering without enabling Hermes support yet.
DON'T add docs in this PR yet.
Key features:
Result
Related Issue
Part of #5492
Changes
channels addpolicy application.channels-add-presettest-file budget after splitting generated-policy coverage into a focused test file.Type of Change
Verification
Verifiedin GitHubnpx prek run --from-ref main --to-ref HEADpassesnpm testpasses (broad runtime changes only)npm run docsbuilds without warnings (doc changes only)Signed-off-by: San Dang sdang@nvidia.com
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Tests