Skip to content

fix(policy): add tls: terminate to telegram preset - #2126

Merged
ericksoa merged 5 commits into
mainfrom
fix/telegram-preset-tls-terminate
Apr 21, 2026
Merged

fix(policy): add tls: terminate to telegram preset#2126
ericksoa merged 5 commits into
mainfrom
fix/telegram-preset-tls-terminate

Conversation

@ericksoa

@ericksoa ericksoa commented Apr 21, 2026

Copy link
Copy Markdown
Contributor

Summary

Test plan

  • Nightly E2E network-policy-e2e job passes (TC-NET-03)

Fixes regression from #2098.

Summary by CodeRabbit

  • Chores
    • Telegram network preset now uses TLS termination for connections to api.telegram.org:443, changing TLS handling for that endpoint.
  • Tests
    • Test suite updated to reflect TLS behavior: gateway/WebSocket presets assert TLS is skipped, and Telegram preset asserts TLS termination for REST endpoints.

The telegram.yaml preset was missing tls: terminate on the
api.telegram.org endpoint.  The baseline openclaw-sandbox.yaml has it,
but the preset omitted it.  After PR #2098 changed how the gateway
handles the tls field, the omission causes the L7 proxy to return 403
for live policy-add requests (TC-NET-03).
@coderabbitai

coderabbitai Bot commented Apr 21, 2026

Copy link
Copy Markdown
Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 0811d6f3-1469-49d6-80a2-2913437d0bab

📥 Commits

Reviewing files that changed from the base of the PR and between 97855b8 and f2210b0.

📒 Files selected for processing (1)
  • test/policies.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • test/policies.test.ts

📝 Walkthrough

Walkthrough

Added tls: terminate to the Telegram preset endpoint api.telegram.org:443 and updated tests: messaging preset TLS assertions now exclude Telegram, with a new test verifying Telegram’s REST endpoint includes tls: terminate.

Changes

Cohort / File(s) Summary
Telegram Policy Configuration
nemoclaw-blueprint/policies/presets/telegram.yaml
Added tls: terminate to the api.telegram.org:443 endpoint declaration to set TLS handling for Telegram's REST/L7 proxy.
Tests — Preset TLS Assertions
test/policies.test.ts
Removed Telegram from the messaging TLS absence check (now only discord and slack) and added a test that asserts tls: terminate is present for the Telegram preset's REST host.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 I nibbled through the YAML hay,
Spotted tls: terminate in play.
Tests hopped to match the new small change,
Rest endpoints tidy in their range. 🥕

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'fix(policy): add tls: terminate to telegram preset' accurately and concisely summarizes the main change: adding a tls: terminate setting to the telegram preset policy file.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/telegram-preset-tls-terminate

Comment @coderabbitai help to get the list of available commands and usage tips.

The original test asserted all messaging presets must not contain
tls: terminate.  After OpenShell v0.0.15 changed TLS auto-termination
behavior, REST-only presets like telegram need tls: terminate for the
L7 proxy to work.  Discord and Slack use tls: skip on their WebSocket
endpoints only.

Split the assertion: WebSocket presets (discord, slack) must not use
terminate; the telegram REST preset must use it.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@test/policies.test.ts`:
- Around line 609-621: Update the two tests that call policies.loadPreset so
they assert TLS settings on the specific endpoint host blocks rather than via a
global substring check: for the "messaging WebSocket presets use tls: skip, not
terminate" test iterate the presets ("discord","slack"), load the preset via
policies.loadPreset(name) and parse/inspect the host/gateway block for that
service to assert the host block explicitly contains tls: skip (or does not
contain tls: terminate) for that host entry; for the "telegram REST preset uses
tls: terminate for L7 proxy" test load policies.loadPreset("telegram") and
locate the api.telegram.org endpoint block and assert it explicitly contains
tls: terminate. Use the same symbols (policies.loadPreset and the test names) to
locate and change the assertions so they check the endpoint-level block rather
than a simple substring search.
🪄 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 Plus

Run ID: 9dbcea90-8cb3-42ff-99f3-75c149c24443

📥 Commits

Reviewing files that changed from the base of the PR and between 1857a6a and 97855b8.

📒 Files selected for processing (1)
  • test/policies.test.ts

Comment thread test/policies.test.ts Outdated
Use regex patterns that match host + tls on the same endpoint block so
regressions in one endpoint cannot be masked by a correct sibling.

Addresses CodeRabbit review feedback.
@ericksoa
ericksoa merged commit 98bf372 into main Apr 21, 2026
9 checks passed
@xiaoming-nv

Copy link
Copy Markdown

not quite follow, seems "tls: terminate" is deprecated, and is actually the default behavior of openshell now.
why we still need to add it explicitly?

@oparoz

oparoz commented Apr 22, 2026

Copy link
Copy Markdown

On top of that that setting is now ignored... It's the wrong fix.

@wscurran wscurran added the bug-fix PR fixes a bug or regression label Jun 8, 2026
@cv
cv deleted the fix/telegram-preset-tls-terminate branch June 28, 2026 00:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug-fix PR fixes a bug or regression

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants