Skip to content

UI: tighten external image-open safety checks and CI guard#25847

Merged
shakkernerd merged 4 commits intomainfrom
fix/ui-chat-image-open-svg-policy
Feb 24, 2026
Merged

UI: tighten external image-open safety checks and CI guard#25847
shakkernerd merged 4 commits intomainfrom
fix/ui-chat-image-open-svg-policy

Conversation

@shakkernerd
Copy link
Member

@shakkernerd shakkernerd commented Feb 24, 2026

Summary

  • Problem: chat image open policy allowed data:image/svg+xml payloads and we had no CI check to prevent new raw window.open callsites outside the shared helper.
  • Why it matters: data:image/svg+xml should be blocked at this boundary, and policy drift is easy without automated enforcement.
  • What changed:
    • Block data:image/svg+xml in resolveSafeExternalUrl (including encoded variants) while keeping allowed schemes the same for safe cases.
    • Keep opener isolation when opening new tabs and add focused tests for SVG denial and opener nulling.
    • Add CI step pnpm lint:ui:no-raw-window-open in the check job.
    • Update changelog wording for this area with both related PR references/authors in one entry.
  • What did NOT change (scope boundary): no change to allowed http:, https:, blob: and raster data:image/* behavior.

Change Type (select all)

  • Bug fix
  • Feature
  • Refactor
  • Docs
  • Security hardening
  • Chore/infra

Scope (select all touched areas)

  • Gateway / orchestration
  • Skills / tool execution
  • Auth / tokens
  • Memory / storage
  • Integrations
  • API / contracts
  • UI / DX
  • CI/CD / infra

Linked Issue/PR

User-visible / Behavior Changes

  • Control UI chat-image opens now reject data:image/svg+xml payloads explicitly.
  • Existing safe URL behavior remains intact (http/https/blob and allowed raster data URLs).

Security Impact (required)

  • New permissions/capabilities? (No)
  • Secrets/tokens handling changed? (No)
  • New/changed network calls? (No)
  • Command/tool execution surface changed? (No)
  • Data access scope changed? (No)
  • If any Yes, explain risk + mitigation:

Repro + Verification

Environment

  • OS: macOS
  • Runtime/container: Node 22 + pnpm
  • Model/provider: N/A
  • Integration/channel (if any): Control UI
  • Relevant config (redacted): default local dev config

Steps

  1. Run pnpm check.
  2. Run pnpm lint:ui:no-raw-window-open.
  3. Run pnpm --dir ui exec vitest run --config vitest.config.ts --browser.enabled=false src/ui/open-external-url.test.ts.

Expected

  • All commands pass and the SVG-base64 policy test is enforced.

Actual

  • Passed locally.

Evidence

Attach at least one:

  • Failing test/log before + passing after
  • Trace/log snippets
  • Screenshot/recording
  • Perf numbers (if relevant)

Human Verification (required)

What you personally verified (not just CI), and how:

  • Verified scenarios:
    • Safe helper still allows supported URL schemes.
    • data:image/svg+xml and base64 form are rejected.
    • CI guard command succeeds and catches only non-helper raw opens.
  • Edge cases checked:
    • Base64-encoded SVG payload rejection in unit tests.
    • Opener-null fallback path still covered.
  • What you did not verify:
    • Full browser-mode Vitest in this environment (Playwright binary missing locally).

Compatibility / Migration

  • Backward compatible? (Yes)
  • Config/env changes? (No)
  • Migration needed? (No)
  • If yes, exact upgrade steps:

Failure Recovery (if this breaks)

  • How to disable/revert this change quickly:
    • Revert this PR commits from main.
  • Files/config to restore:
    • ui/src/ui/open-external-url.ts
    • ui/src/ui/open-external-url.test.ts
    • ui/src/ui/views/chat-image-open.browser.test.ts
    • .github/workflows/ci.yml
    • CHANGELOG.md
  • Known bad symptoms reviewers should watch for:
    • Unexpected block of safe image URLs in chat click-open flow.

Risks and Mitigations

  • Risk:
    • Over-blocking legitimate data-image URLs.
    • Mitigation:
      • Policy explicitly blocks SVG only; raster data-image coverage remains and is tested.

Greptile Summary

This PR adds security hardening to prevent data:image/svg+xml payloads from being opened in chat image clicks and introduces CI enforcement to prevent new raw window.open callsites outside the safe helper.

  • Blocks SVG data URLs (both plain and base64-encoded) in resolveSafeExternalUrl while keeping http:, https:, blob:, and raster data images allowed
  • Adds comprehensive unit tests for SVG blocking and opener isolation
  • Introduces CI check pnpm lint:ui:no-raw-window-open to enforce policy that all window.open calls must go through the safe helper
  • Updates changelog to credit both contributors for related chat-image security work
  • All existing safe URL behavior (http/https/blob and raster data images) remains unchanged

Confidence Score: 5/5

  • This PR is safe to merge with no identified risks
  • The changes are well-scoped security improvements with comprehensive test coverage. The SVG blocking logic is straightforward and correctly handles both plain and base64-encoded variants. The CI enforcement prevents policy drift. All existing safe URL behaviors are preserved and tested.
  • No files require special attention

Last reviewed commit: aeb96a5

@openclaw-barnacle openclaw-barnacle bot added app: web-ui App: web-ui size: S maintainer Maintainer-authored PR labels Feb 24, 2026
@shakkernerd shakkernerd self-assigned this Feb 24, 2026
@shakkernerd shakkernerd merged commit 853f755 into main Feb 24, 2026
25 of 26 checks passed
@shakkernerd shakkernerd deleted the fix/ui-chat-image-open-svg-policy branch February 24, 2026 22:29
@shakkernerd
Copy link
Member Author

Landed via temp rebase onto main.

Thanks @shakkernerd!

gavinwxx-vybers added a commit to Vybers-AI/openclaw that referenced this pull request Feb 25, 2026
* ui: block svg data image opens and harden tests

* changelog: credit both chat-image fix contributors

* test(ui): reject base64 SVG data URLs

* changelog: include openclaw#25847 in chat image safety entry (openclaw#25847) (thanks @shakkernerd)

* refactor(ios): drop legacy talk payload and keychain fallbacks

* chore: sync plugin versions to 2026.2.24

* chore: refresh lockfile after plugin devDependency cleanup

* fix(config): soften antigravity removal fallout (openclaw#25538)

Land openclaw#25538 by @chilu18 to keep legacy google-antigravity-auth config entries non-fatal after removal (see openclaw#25862).

Co-authored-by: chilu18 <chilu.machona@icloud.com>

* fix(security): lock sandbox tmp media paths to openclaw roots

* docs(security): document openclaw temp-folder boundary

* fix(security): restrict default safe-bin trusted dirs

* fix: enforce local media root checks for attachment hydration

* fix(synology-chat): fail closed empty allowlist

* docs(changelog): add synology-chat allowlist fail-closed note

* fix: harden routing/session isolation for followups and heartbeat

* feat(sandbox): block container namespace joins by default

* refactor(sandbox): centralize network mode policy helpers

* fix(channels,sandbox): land hard breakage cluster from reviewed PR bases

Lands reviewed fixes based on openclaw#25839 (@pewallin), openclaw#25841 (@joshjhall), and openclaw#25737/@25713 (@DennisGoldfinger/@peteragility), with additional hardening + regression tests for queue cleanup and shell script safety.

Fixes openclaw#25836
Fixes openclaw#25840
Fixes openclaw#25824
Fixes openclaw#25868

Co-authored-by: Peter Wallin <pwallin@gmail.com>
Co-authored-by: Joshua Hall <josh@yaplabs.com>
Co-authored-by: Dennis Goldfinger <dennisgoldfinger@gmail.com>
Co-authored-by: peteragility <peteragility@users.noreply.github.com>

* refactor(synology-chat): centralize DM auth and fail fast startup

* test: add routing/session isolation edge-case regressions

* refactor: centralize followup origin routing helpers

* refactor(outbound): centralize attachment media policy

* refactor: harden safe-bin trusted dir diagnostics

* fix(zalo): enforce group sender policy in groups

* docs: update changelog for safe-bin hardening

* test(line): align tmp-root expectation after sandbox hardening

* fix(web-search): reduce provider auto-detect log noise

* test(matrix,discord,sandbox): expand breakage regression coverage

* refactor(matrix,tests): extract helpers and inject send-queue timing

* refactor(zalo): split monitor access and webhook logic

* Gateway/Security: protect /api/channels plugin root

* fix(telegram): block unauthorized DM media downloads

* Security: sanitize inherited host exec env

* Changelog: add entry for exec env sanitization

* fix(security): classify hook sessions case-insensitively

* refactor(outbound): unify attachment hydration flow

* refactor(telegram): simplify DM media auth precheck flow

* fix(automation): harden announce delivery + cron coding profile (openclaw#25813 openclaw#25821 openclaw#25822)

Co-authored-by: Shawn <shenghuikevin@shenghuideMac-mini.local>
Co-authored-by: 不做了睡大觉 <user@example.com>
Co-authored-by: Marcus Widing <widing.marcus@gmail.com>

* security(voice-call): detect Telnyx webhook replay

* Auto-reply: add exact stop trigger for do not do that

* Auto-reply tests: assert exact do not do that behavior

* Gateway tests: cover exact do not do that stop matching

* Telegram tests: route exact do not do that to control lane

* Changelog: note exact do not do that stop trigger

* refactor(tmp): harden temp boundary guardrails

* fix(whatsapp): stop retry loop on non-retryable 440 close

* test(types): fix ts narrowing regressions in followup and matrix queue tests

* fix(onboard): avoid false 'telegram plugin not available' block

* fix: normalize "bedrock" provider ID to "amazon-bedrock"

Add "bedrock" and "aws-bedrock" as aliases for the canonical
"amazon-bedrock" provider ID in normalizeProviderId().

Without this mapping, configuring a model as "bedrock/..." causes
the auth resolution fallback to miss the Bedrock-specific AWS SDK
path, since the fallback check requires normalized === "amazon-bedrock".
This primarily affects the main agent when the explicit auth override
is not preserved through config merging.

Fixes openclaw#15716

* docs(changelog): backfill landed fix PR entries

* fix(security): harden system.run companion command binding

* fix(discord): land proxy/media/reaction/model-picker regressions

Reimplements core Discord fixes from openclaw#25277 openclaw#25523 openclaw#25575 openclaw#25588 openclaw#25731 with expanded tests.

- thread proxy-aware fetch into inbound attachment/sticker downloads
- fetch /gateway/bot via proxy dispatcher before ws connect
- wire statusReactions emojis/timing overrides into controller
- compact model-picker custom_id keys with backward-compatible parsing

Co-authored-by: openperf <openperf@users.noreply.github.com>
Co-authored-by: chilu18 <chilu18@users.noreply.github.com>
Co-authored-by: Yipsh <Yipsh@users.noreply.github.com>
Co-authored-by: lbo728 <lbo728@users.noreply.github.com>
Co-authored-by: s1korrrr <s1korrrr@users.noreply.github.com>

* docs(changelog): add reporter credit for exec companion hardening

* fix(macos): guard voice audio paths with no input device (openclaw#25817)

Co-authored-by: Stefan Förster <103369858+sfo2001@users.noreply.github.com>

* fix(macos): prefer openclaw binary while keeping pnpm fallback (openclaw#25512)

Co-authored-by: Peter Machona <7957943+chilu18@users.noreply.github.com>

* Auth: bypass cooldown tracking for OpenRouter

* Auth: use cooldown helper in explicit profile order

* Tests: cover OpenRouter cooldown display bypass

* Tests: skip OpenRouter failure cooldown persistence

* Tests: keep OpenRouter runnable with legacy cooldown markers

* Tests: preserve OpenRouter explicit auth order under cooldown fields

* Changelog: note OpenRouter cooldown bypass

* Changelog: remove unrelated session entries from PR

* Update CHANGELOG.md

* fix(macos): default voice wake forwarding to webchat (openclaw#25440)

Co-authored-by: Peter Machona <7957943+chilu18@users.noreply.github.com>

* fix(macos): keep Return for IME marked text commit (openclaw#25178)

Co-authored-by: jft0m <9837901+bottotl@users.noreply.github.com>

* fix(security): block env depth-overflow approval bypass

* fix(macos): resolve webchat panel corner clipping (openclaw#22458)

Co-authored-by: apethree <3081182+apethree@users.noreply.github.com>
Co-authored-by: agisilaos <3073709+agisilaos@users.noreply.github.com>

* Agents: trust explicit allowlist refs beyond catalog

* Tests: cover allowlist refs missing from catalog

* Gateway tests: accept allowlisted refs absent from catalog

* Gateway tests: include synthetic allowlist models in models.list

* Changelog: note allowlist stale-catalog model selection fix

* fix(discord): harden voice DAVE receive reliability (openclaw#25861)

Reimplements and consolidates related work:
- openclaw#24339 stale disconnect/destroyed session guards
- openclaw#25312 voice listener cleanup on stop
- openclaw#23036 restore @snazzah/davey runtime dependency

Adds Discord voice DAVE config passthrough, repeated decrypt failure
rejoin recovery, regression tests, docs, and changelog updates.

Co-authored-by: Frank Yang <frank.ekn@gmail.com>
Co-authored-by: Do Cao Hieu <admin@docaohieu.com>

* fix(macos): clean warnings and harden gateway/talk config parsing

* docs(discord): document DAVE defaults and decrypt recovery

* test: bridge discord voice private casts via unknown

* docs(changelog): remove next-release shipping sentence

* refactor(exec): split system.run phases and align ts/swift validator contracts

* fix(windows): skip unreliable dev comparison in fs-safe openVerifiedLocalFile

On Windows, device IDs (dev) returned by handle.stat() and fs.lstat()
may differ even for the same file, causing false-positive 'path-mismatch'
errors when reading local media files.

This fix introduces a statsMatch() helper that:
- Always compares inode (ino) values
- Skips device ID (dev) comparison on Windows where it's unreliable
- Maintains full comparison on Unix platforms

Fixes openclaw#25699

* fix: align windows safe-open file identity checks

* refactor: dedupe exec wrapper denial plan and test setup

* fix: harden iMessage echo dedupe and reasoning suppression (openclaw#25897)

* test(media): add win32 dev=0 local media regression

* refactor: extract iMessage echo cache and unify suppression guards

* test: normalize tmp media path assertion for windows

* fix(render): seed Control UI origin config on first boot

The gateway requires controlUi.allowedOrigins when binding to LAN.
On Render, the persistent disk starts empty with no openclaw.json.
Seed a minimal config with dangerouslyAllowHostHeaderOriginFallback
on first boot (safe behind Render's HTTPS reverse proxy).

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* chore(deps): update dependencies except carbon

* fix(agents): normalize SiliconFlow Pro thinking=off payload (openclaw#25435)

Land PR openclaw#25435 from @Zjianru.
Changelog: add 2026.2.24 fix entry with contributor credit.

Co-authored-by: codez <codezhujr@gmail.com>

* fix(telegram): refresh global undici dispatcher for autoSelectFamily (openclaw#25682)

Land PR openclaw#25682 from @lairtonlelis after maintainer rework:
track dispatcher updates when network decision changes to avoid stale global fetch behavior.

Co-authored-by: Ailton <lairton@telnyx.com>

* fix(synology-chat): land @bmendonca3 fail-closed allowlist follow-up (openclaw#25827)

Carry fail-closed empty-allowlist guard clarity and changelog attribution for PR openclaw#25827.

Co-authored-by: Brian Mendonca <brianmendonca@Brians-MacBook-Air.local>

* fix(agents): reduce billing false positives on long text (openclaw#25680)

Land PR openclaw#25680 from @lairtonlelis.
Retain explicit status/code/http 402 detection for oversized structured payloads.

Co-authored-by: Ailton <lairton@telnyx.com>

* fix(render): add docker entrypoint script for config seeding

The inline shell command in render.yaml's dockerCommand wasn't
reliably creating the seed config. Replace with a proper entrypoint
script that creates a minimal openclaw.json with
dangerouslyAllowHostHeaderOriginFallback on first boot, then starts
the gateway bound to LAN on the PORT env var.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix(ui): inherit default model fallbacks in agents overview (openclaw#25729)

Land PR openclaw#25729 from @Suko.
Use shared fallback-resolution helper and add regression coverage for default, override, and explicit-empty cases.

Co-authored-by: suko <miha.sukic@gmail.com>

* fix(heartbeat): default target none and internalize relay prompts

* test(windows): normalize risky-path assertions

---------

Co-authored-by: Shakker <shakkerdroid@gmail.com>
Co-authored-by: Peter Steinberger <steipete@gmail.com>
Co-authored-by: chilu18 <chilu.machona@icloud.com>
Co-authored-by: Peter Wallin <pwallin@gmail.com>
Co-authored-by: Joshua Hall <josh@yaplabs.com>
Co-authored-by: Dennis Goldfinger <dennisgoldfinger@gmail.com>
Co-authored-by: peteragility <peteragility@users.noreply.github.com>
Co-authored-by: Brian Mendonca <brianmendonca@Brians-MacBook-Air.local>
Co-authored-by: Shawn <shenghuikevin@shenghuideMac-mini.local>
Co-authored-by: 不做了睡大觉 <user@example.com>
Co-authored-by: Marcus Widing <widing.marcus@gmail.com>
Co-authored-by: Vincent Koc <vincentkoc@ieee.org>
Co-authored-by: Mark Musson <mark@musson.co.za>
Co-authored-by: suko <miha.sukic@gmail.com>
Co-authored-by: Fred White <fwhite13@users.noreply.github.com>
Co-authored-by: openperf <openperf@users.noreply.github.com>
Co-authored-by: chilu18 <chilu18@users.noreply.github.com>
Co-authored-by: Yipsh <Yipsh@users.noreply.github.com>
Co-authored-by: lbo728 <lbo728@users.noreply.github.com>
Co-authored-by: s1korrrr <s1korrrr@users.noreply.github.com>
Co-authored-by: Stefan Förster <103369858+sfo2001@users.noreply.github.com>
Co-authored-by: Peter Machona <7957943+chilu18@users.noreply.github.com>
Co-authored-by: jft0m <9837901+bottotl@users.noreply.github.com>
Co-authored-by: apethree <3081182+apethree@users.noreply.github.com>
Co-authored-by: agisilaos <3073709+agisilaos@users.noreply.github.com>
Co-authored-by: Frank Yang <frank.ekn@gmail.com>
Co-authored-by: Do Cao Hieu <admin@docaohieu.com>
Co-authored-by: Gavin X. Wang <gavinvybers@Gavins-MacBook-Pro.local>
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Co-authored-by: codez <codezhujr@gmail.com>
Co-authored-by: Ailton <lairton@telnyx.com>
margulans pushed a commit to margulans/Neiron-AI-assistant that referenced this pull request Feb 25, 2026
Jackson3195 pushed a commit to Jackson3195/openclaw-with-a-personal-touch that referenced this pull request Feb 25, 2026
brianleach pushed a commit to brianleach/openclaw that referenced this pull request Feb 26, 2026
execute008 pushed a commit to execute008/openclaw that referenced this pull request Feb 27, 2026
r4jiv007 pushed a commit to r4jiv007/openclaw that referenced this pull request Feb 28, 2026
hughdidit pushed a commit to hughdidit/DAISy-Agency that referenced this pull request Mar 1, 2026
…w#25847) (thanks @shakkernerd)

(cherry picked from commit 853f755)

# Conflicts:
#	CHANGELOG.md
hughdidit pushed a commit to hughdidit/DAISy-Agency that referenced this pull request Mar 3, 2026
…w#25847) (thanks @shakkernerd)

(cherry picked from commit 853f755)

# Conflicts:
#	CHANGELOG.md
zooqueen pushed a commit to hanzoai/bot that referenced this pull request Mar 6, 2026
thebenjaminlee pushed a commit to escape-velocity-ventures/openclaw that referenced this pull request Mar 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

app: web-ui App: web-ui maintainer Maintainer-authored PR size: S

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant