Skip to content

fix(qqbot): allow scoped sandbox media sends#92872

Merged
sliverp merged 18 commits into
openclaw:mainfrom
zhangguiping-xydt:feat/issue-92827
Jul 3, 2026
Merged

fix(qqbot): allow scoped sandbox media sends#92872
sliverp merged 18 commits into
openclaw:mainfrom
zhangguiping-xydt:feat/issue-92827

Conversation

@zhangguiping-xydt

@zhangguiping-xydt zhangguiping-xydt commented Jun 14, 2026

Copy link
Copy Markdown
Contributor

Related: #92827

What Problem This Solves

  • Fix classification: root-cause fix for QQBot sandbox-generated media delivery.
  • Maintainer-ready confidence: high for the QQBot side covered by this PR; webchat rendering is explicitly not changed.
  • Behavior or issue addressed: sandbox-enabled agents can create a file in their workspace, then fail to send it through QQBot when the reply references the sandbox-visible /workspace/... path or a relative generated-file path.
  • Root cause: QQBot outbound delivery did not consistently carry the active agent/session media access context into every QQBot media-send surface, so sandbox-visible paths such as /workspace/report.pdf or report.pdf could reach QQBot as host paths without the current session workspace mapping. The existing local-root/host-read safety checks then either rejected the file as outside allowed roots or could not read the generated file for upload.
  • Why it matters / User impact: a sandboxed agent can generate a document, image, audio, video, or other supported file and send it through QQBot using the path it actually sees in the session, including <qqmedia>/workspace/report.pdf</qqmedia>, structured QQBot media payloads, and explicit block mediaUrl values.

Why This Change Was Made

  • Why this is root-cause fix: the fix establishes the current agent workspace at the QQBot gateway/message-adapter boundary, then threads that scoped mediaAccess.workspaceDir, authorized local roots, and read callback through QQBot outbound delivery, media tags, block media URLs, structured payloads, and the C2C streaming media path. The lower media sender maps /workspace/... and bare relative generated-file paths through that scoped context before delegating to the existing local-root and host-read validation.
  • Architecture / source-of-truth check: the source of truth for the workspace is the active route/default agent resolved through the existing OpenClaw agent runtime contract; lower helpers do not invent a global sandbox root and do not treat arbitrary host paths as readable. Path containment remains delegated to the existing security/runtime media-loading helpers after the scoped candidate path is formed.
  • What did NOT change: this does not change webchat media rendering, does not add a broad sandbox root, does not relax file-size limits, does not add or require a new public config/schema/protocol field, does not expand data URL/base64 media handling, and rejects /workspace/../... or ../... escapes before checking sibling roots.
  • Out of scope: closing the webchat half of [Bug]: 如果启用沙箱,agent无法将文件发送到webchat和QQBot #92827, adding new generic media transports, and changing attachment bootstrap behavior are out of scope for this QQBot-focused update.
  • Related open PR / same-contract scan: before submit, open PR search for the issue/contract (92827 and QQBot sandbox media keywords) found this PR as the only same-issue QQBot sandbox-media update; another broad search hit (fix(control-ui): don't hard-deny local attachments before bootstrap roots load #78034) is an unrelated control-ui attachment bootstrap PR and does not cover this QQBot outbound contract.
  • Patch quality notes: the final branch is still larger than an ideal XS patch because the same scoped media context has to cross QQBot gateway dispatch, channel/message adapter send paths, tag/block outbound delivery, structured payload handling, host-read/local media upload, and C2C streaming media dispatch. Non-issue expansion was removed from this update: generic data URL/base64 routing tests, sender/media-source hardening, markdown-title parsing changes, and streaming fallback-race changes are no longer part of the submitted scope.

User Impact

  • Scenario locked in: an agent in sandbox writes a file visible as /workspace/report.pdf or report.pdf, then replies through QQBot with a media tag, structured QQBot media payload, or block mediaUrl referring to that generated file.
  • Why this is the smallest reliable guardrail: only the QQBot-owned outbound boundaries receive the active scoped media context; the PR does not broaden process-wide filesystem access. Focused regressions lock the successful mapping path and the failure boundaries for traversal, missing context, kind mismatch, and empty host-read files.
  • Risk labels considered: message-delivery, security-boundary, session-state, compatibility, availability, and automation.
  • Risk explanation: message-delivery changes are limited to QQBot outbound media surfaces; security-boundary risk is controlled by mapping only through the active session workspace and preserving existing root/read validation; session-state risk is limited to resolving the current route/default agent workspace; compatibility risk is reduced by keeping defaults and public config/schema/protocol unchanged; availability risk is limited to media-only block replies continuing to the existing static media path rather than being swallowed by streaming; automation risk is covered by focused regressions plus commit review.
  • Why acceptable: without carrying the scoped media context across these QQBot surfaces, one issue path could be fixed while another still drops the same sandbox-generated file. The remaining changed files are QQBot-owned implementation/tests or QQBot skill guidance, and the PR body now backs both C2C and QQ group delivery with redacted live proof while keeping webchat explicitly out of scope.

Evidence

  • Exact steps or command run after this patch:

    node scripts/run-vitest.mjs run --config test/vitest/vitest.full-extensions.config.ts \
      extensions/qqbot/src/channel.message-adapter.test.ts \
      extensions/qqbot/src/engine/gateway/outbound-dispatch.test.ts \
      extensions/qqbot/src/engine/messaging/outbound-media-send.test.ts \
      extensions/qqbot/src/engine/messaging/outbound-deliver.test.ts \
      extensions/qqbot/src/engine/messaging/reply-dispatcher.test.ts
    
  • Evidence after fix: focused QQBot regression on final local HEAD 5d5bb9be0142fc5d042279e01f7705d3030d6988 passed.

  • Observed result after fix:

    Test Files  5 passed (5)
         Tests  83 passed (83)
    
  • Target test file: extensions/qqbot/src/channel.message-adapter.test.ts, extensions/qqbot/src/engine/gateway/outbound-dispatch.test.ts, extensions/qqbot/src/engine/messaging/outbound-media-send.test.ts, extensions/qqbot/src/engine/messaging/outbound-deliver.test.ts, and extensions/qqbot/src/engine/messaging/reply-dispatcher.test.ts.

  • ClawSweeper commit review passed on HEAD 5d5bb9be0142fc5d042279e01f7705d3030d6988 with result: nothing_found, highest_severity: none, and check_conclusion: success.

  • git diff --check passed for the full origin/main...HEAD PR diff, and the full diff was rescanned before submit.

  • Existing redacted live QQBot C2C proof for this PR exercised the real Tencent inbound auto-reply path with <qqmedia>/workspace/..., reached the PR dispatcher/tag parser/scoped workspace mapping, uploaded the generated file through the QQBot files API, and sent the QQBot media message successfully. Secrets, tokens, sender IDs, message IDs, and QQBot file metadata are redacted.

  • New redacted live QQBot group proof on final local HEAD 5d5bb9be0142fc5d042279e01f7705d3030d6988 used a group-capable QQBot account and a real GROUP_AT_MESSAGE_CREATE event, reached mock-openai after inbound, returned <qqmedia>/workspace/<proof-file></qqmedia>, detected the media tag, mapped the sandbox workspace path, posted to Tencent /v2/groups/<redacted>/files, posted the group media message, and completed dispatch. App IDs, secrets, tokens, group IDs, sender IDs, message IDs, message_scene tokens, file data, and QQBot file metadata are redacted.

  • RF-001: the scoped security boundary is addressed by mapping /workspace only through the active session's mediaAccess.workspaceDir, passing current scoped roots/read callbacks through QQBot outbound, and keeping traversal rejection tests for /workspace/../... and ../....

  • RF-002: the live external proof boundary is now covered for both QQBot C2C and QQBot group delivery; webchat media rendering remains outside this PR.

  • RF-003: the QQBot media skill wording was updated so the generic instruction is <qqmedia>{实际路径或URL}</qqmedia>, with /workspace/... documented only when that is the actual sandbox-visible path.

  • RF-004: the redacted live QQBot C2C and group proofs remain included as supporting real-behavior evidence for the covered QQBot path.

  • RF-005: the remaining path-boundary gaps are included in this final local branch: host-read requires scoped workspace context, broad default sandbox roots are not exposed, and explicit relative block media paths route through the scoped QQBot media resolver.

  • What was not tested: this update does not test or change webchat media rendering.

@openclaw-barnacle openclaw-barnacle Bot added channel: qqbot proof: supplied External PR includes structured after-fix real behavior proof. size: M labels Jun 14, 2026
@clawsweeper

clawsweeper Bot commented Jun 14, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed July 3, 2026, 2:48 AM ET / 06:48 UTC.

Summary
The PR threads scoped mediaAccess, workspace roots, and host-read callbacks through QQBot outbound media paths so sandbox-visible /workspace/... and relative generated-file paths can be uploaded.

PR surface: Source +698, Tests +1998, Docs +3. Total +2699 across 16 files.

Reproducibility: yes. Source inspection of current main and v2026.6.11 shows QQBot outbound paths do not propagate scoped mediaAccess/workspaceDir, matching the linked sandbox-generated file delivery failure.

Review metrics: 1 noteworthy metric.

  • Scoped Outbound Surfaces: 5 groups threaded. Direct sends, gateway/tool/block replies, media tags, structured payloads, and C2C streaming must all preserve the same media-access context before merge.

Stored data model
Persistent data-model change detected: serialized state: extensions/qqbot/src/channel.message-adapter.test.ts, serialized state: extensions/qqbot/src/engine/messaging/outbound-media-send.ts, serialized state: extensions/qqbot/src/engine/messaging/outbound-types.ts, serialized state: extensions/qqbot/src/engine/messaging/outbound.ts. Confirm migration or upgrade compatibility proof before merge.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #92827
Summary: This PR is the QQBot fix candidate for the canonical sandbox media-delivery issue; WebChat attachment rendering remains a partial overlap tracked separately.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🦞 diamond lobster ✨ media proof bonus
Patch quality: 🐚 platinum hermit
Result: ready for maintainer review.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • [P2] Get explicit channel/security owner signoff on the scoped workspace, local roots, and readFile authorization boundary before merge.

Risk before merge

  • [P1] Maintainers need to accept that QQBot media reads are authorized only by the active workspace, scoped local roots, and readFile callback; this is a security-boundary decision even though no concrete defect was found.
  • [P1] The change crosses direct sends, gateway/tool/block replies, media tags, structured payloads, plain mediaUrl routing, and streaming; a missed path would still drop generated media.

Maintainer options:

  1. Require Scoped-Boundary Signoff (recommended)
    Have a channel or security owner confirm that active workspace, scoped roots, and readFile are the complete QQBot media authorization boundary before merge.
  2. Accept Current Risk With Existing Proof
    Maintainers may land with the current code, tests, CI, and live QQBot proof if they are comfortable owning the scoped file-read boundary.
  3. Pause For A Narrower Replacement
    Pause this PR only if maintainers want a smaller replacement that preserves the same route coverage with less cross-file churn.

Next step before merge

  • [P2] The remaining action is maintainer/security review of the scoped QQBot media boundary, not an automated repair.

Security
Cleared: No concrete introduced security defect was found, but the sandbox file-read boundary remains a maintainer merge-risk decision.

Review details

Best possible solution:

Land the scoped QQBot fix after channel/security boundary signoff, while keeping the WebChat attachment-rendering overlap on its separate issue and PR track.

Do we have a high-confidence way to reproduce the issue?

Yes. Source inspection of current main and v2026.6.11 shows QQBot outbound paths do not propagate scoped mediaAccess/workspaceDir, matching the linked sandbox-generated file delivery failure.

Is this the best way to solve the issue?

Yes, with maintainer boundary signoff. Threading the existing scoped mediaAccess/localRoots/readFile contract through QQBot-owned outbound paths is narrower than adding broad roots, mirroring files, or adding new configuration.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 768da09ceb15.

Label changes

Label changes:

  • add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
  • remove rating: 🦞 diamond lobster: Current PR rating is rating: 🐚 platinum hermit, so this older rating label is no longer current.

Label justifications:

  • P2: This is a normal-priority QQBot sandbox media delivery fix with concrete user impact but channel-limited blast radius.
  • merge-risk: 🚨 message-delivery: The diff changes QQBot media path selection and upload routing across several outbound surfaces where mistakes could still drop media.
  • merge-risk: 🚨 security-boundary: The patch maps sandbox-visible workspace paths into host reads and QQBot uploads, which touches the filesystem sandbox boundary.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (screenshot): The PR body/comments provide live QQBot C2C and group proof, and inspected screenshots directly show after-fix QQ file delivery.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body/comments provide live QQBot C2C and group proof, and inspected screenshots directly show after-fix QQ file delivery.
  • proof: 📸 screenshot: Contributor real behavior proof includes screenshot evidence. The PR body/comments provide live QQBot C2C and group proof, and inspected screenshots directly show after-fix QQ file delivery.
Evidence reviewed

PR surface:

Source +698, Tests +1998, Docs +3. Total +2699 across 16 files.

View PR surface stats
Area Files Added Removed Net
Source 10 865 167 +698
Tests 5 1999 1 +1998
Docs 1 14 11 +3
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 16 2878 179 +2699

What I checked:

Likely related people:

  • sliverp: Authored the original QQBot channel addition and provided member review/follow-up commits on this PR's media-skill and boundary direction. (role: original QQBot channel contributor and reviewer; confidence: high; commits: bf6f506dfae6, 257d0cbd9b14, a383fd41cc9e; files: extensions/qqbot/src/channel.ts, extensions/qqbot/skills/qqbot-media/SKILL.md, extensions/qqbot/src/engine/messaging/reply-dispatcher.ts)
  • cxyhhhhh: Authored the major QQBot group/C2C streaming and media upload architecture refactor that introduced central surfaces this PR now threads with scoped media access. (role: QQBot media architecture contributor; confidence: high; commits: 5ccf179a34a9; files: extensions/qqbot/src/engine/gateway/outbound-dispatch.ts, extensions/qqbot/src/engine/messaging/outbound-media-send.ts, extensions/qqbot/src/engine/messaging/streaming-c2c.ts)
  • pgondhi987: Authored prior QQBot local-file storage boundary enforcement that is central to this PR's security constraints. (role: QQBot media security boundary contributor; confidence: high; commits: 604777e4414c; files: extensions/qqbot/src/outbound.ts, extensions/qqbot/src/outbound.security.test.ts)
  • ZengWen-DT: Authored the merged QQBot temp-root trusted-media delivery fix in the same outbound validation area. (role: recent adjacent QQBot media contributor; confidence: medium; commits: cfdcd5cdfdc3; files: extensions/qqbot/src/engine/messaging/outbound-media-send.ts, extensions/qqbot/src/engine/messaging/reply-dispatcher.ts, extensions/qqbot/src/engine/messaging/trusted-media-path.ts)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works
  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. labels Jun 14, 2026
@openclaw-barnacle openclaw-barnacle Bot removed the proof: sufficient ClawSweeper judged the real behavior proof convincing. label Jun 14, 2026
@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. P2 Normal backlog priority with limited blast radius. merge-risk: 🚨 security-boundary 🚨 May affect sandboxing, authorization, credentials, or sensitive data. labels Jun 14, 2026
@openclaw-barnacle openclaw-barnacle Bot removed the proof: sufficient ClawSweeper judged the real behavior proof convincing. label Jun 14, 2026
@clawsweeper clawsweeper Bot added the proof: sufficient ClawSweeper judged the real behavior proof convincing. label Jun 14, 2026
@zhangguiping-xydt
zhangguiping-xydt marked this pull request as draft June 14, 2026 13:04
@openclaw-barnacle openclaw-barnacle Bot added size: L and removed size: M proof: sufficient ClawSweeper judged the real behavior proof convincing. labels Jun 14, 2026
@zhangguiping-xydt
zhangguiping-xydt marked this pull request as ready for review June 14, 2026 15:59
@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. and removed rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. labels Jun 14, 2026
@openclaw-barnacle openclaw-barnacle Bot removed the proof: sufficient ClawSweeper judged the real behavior proof convincing. label Jun 14, 2026
@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. labels Jun 14, 2026
@zhangguiping-xydt

Copy link
Copy Markdown
Contributor Author
C37_2026062907102482479

@sliverp

sliverp commented Jun 29, 2026

Copy link
Copy Markdown
Member

I think the runtime direction here is mostly right, but the QQBot media skill text needs a pre-merge fix.

The important runtime contract is: QQBot outbound media should preserve the current agent/session media access context and translate sandbox-visible paths such as /workspace/... through that scoped context before applying the normal local-root checks. That is the right layer for this bug. The PR is doing that by threading the media context through the QQBot outbound paths and resolving /workspace/... against the agent workspace instead of treating it as a generic QQBot process-local path.

The skill wording, though, is too narrow and likely wrong:

<qqmedia>/workspace/文件名或URL</qqmedia>

/workspace is only the agent-visible path in specific sandbox workspace modes. QQBot media sends also need to work when the agent is not sandboxed, when the path comes from the conversation attachment context, when the file is already under a scoped media root or QQBot media directory, and when the source is a URL. The skill should not teach agents that /workspace/... is the general QQBot media format, and agents should not have to infer whether they are inside a sandbox to choose the only valid syntax.

Please keep the usage generic, for example <qqmedia>路径或URL</qqmedia>, and phrase the rule as path-source guidance:

  • use the path you actually see for a file you just created;
  • in sandboxed workspace-write runs that may be /workspace/...;
  • if the path came from the conversation attachment context, use that path directly;
  • URLs are valid directly;
  • local paths still must stay inside the current session's authorized agent workspace, scoped media roots, or OpenClaw/QQBot media roots.

With that wording fixed, the runtime implementation looks like the right shape. Without it, this PR risks replacing a runtime path bug with an agent-instruction bug where non-sandbox or attachment-return cases get incorrectly pushed toward /workspace/....

@zhangguiping-xydt

zhangguiping-xydt commented Jun 29, 2026

Copy link
Copy Markdown
Contributor Author

Thanks @sliverp, for the detail review. I updated the QQBot media skill wording so /workspace/... is no longer taught as the generic media format.

The generic form is now <qqmedia>路径或URL</qqmedia>, with guidance to use the actual path source: sandbox-created file paths when applicable, attachment-context paths directly, URLs directly, and only authorized scoped local roots for local files. Runtime behavior is unchanged and still maps /workspace/... through the scoped session workspace before normal validation.

@zhangguiping-xydt

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

@zhangguiping-xydt

Copy link
Copy Markdown
Contributor Author

I re-ran the latest PR head against a live QQBot C2C session and the current scoped-media fix looks correct for the covered path.

Evidence from the live run:

  • Started the QQBot gateway from the PR worktree/source CLI; QQBot obtained an access token, connected WebSocket, and reported connected=true.
  • Captured the C2C target from a real inbound user message; full sender/conversation IDs and secrets are redacted.
  • Sent an agent-workspace local file through the public openclaw message send --channel qqbot --target qqbot:c2c:<redacted> --media <agent-workspace-file> --json surface.
  • The send returned dryRun=false, receipt.parts[0].kind="media", and a QQBot platform message id was returned.
  • A same-surface negative probe using --media /workspace/../outside-pr92872-negative.txt failed with Local media path is not under an allowed directory, so the scoped-root boundary still blocks traversal.

One clarification: in this run, the plain CLI text form --message '<qqmedia>...</qqmedia>' went through QQBot markdown text delivery, so I am not counting that as external tag-path proof. The live external proof covers scoped media delivery through --media; the PR's focused regression coverage still covers the <qqmedia>/structured-payload workspace mapping paths.

@zhangguiping-xydt

Copy link
Copy Markdown
Contributor Author
C163_2026070106542417602

@sliverp

sliverp commented Jul 1, 2026

Copy link
Copy Markdown
Member

Pushed a follow-up commit to tighten the two remaining path-boundary gaps: b1485af777.

What changed:

  • Kept the host-read fallback boundary aligned with the current PR contract: /workspace/... is scoped through the active agent workspace, but host-read media sending still uses the full authorized media-access context so previously allowed local media roots are not blocked accidentally.
  • Fixed the plain block payload gap: payload.mediaUrl / payload.mediaUrls with a relative value now enter the same media auto-routing path as <qqmedia>relative.docx</qqmedia> and direct sendMedia({ mediaUrl: "relative.docx" }). The existing resolver then validates the scoped workspace boundary, so traversal like ../outside.docx is rejected instead of being sent.

Added regression coverage for both the successful relative plain payload case and the workspace-escape rejection case.

Proof:

  • node scripts/run-vitest.mjs extensions/qqbot/src/engine/gateway/outbound-dispatch.test.ts extensions/qqbot/src/engine/messaging/outbound-media-send.test.ts extensions/qqbot/src/engine/messaging/reply-dispatcher.test.ts extensions/qqbot/src/channel.message-adapter.test.ts -> 4 files / 59 tests passed
  • .agents/skills/autoreview/scripts/autoreview --mode branch --base origin/main -> clean, no accepted/actionable findings

@sliverp

sliverp commented Jul 3, 2026

Copy link
Copy Markdown
Member

One remaining boundary gap I found while re-reviewing the latest head:

Structured QQBOT_PAYLOAD paths map the candidate file path from /workspace/... to the real scoped workspace path, but the allowed local roots used for validation can remain virtual.

Concretely, validateStructuredPayloadLocalPath() maps:

/workspace/attachments/report.pdf
-> /tmp/agent-workspace/attachments/report.pdf

but then passes raw roots such as:

["/workspace/attachments"]

to resolveLocalPathFromRootsSync(). That makes the real containment check compare /tmp/agent-workspace/attachments/report.pdf against /workspace/attachments, so a valid scoped structured payload can be rejected before host-read/local send runs.

The fix should be small: in extensions/qqbot/src/engine/messaging/reply-dispatcher.ts, map resolveOutboundMediaLocalRoots(ctx) through resolveWorkspaceScopedLocalRoots(..., ctx.mediaAccess?.workspaceDir) before passing roots to resolveLocalPathFromRootsSync(). The corresponding test should assert the scoped real root, not the raw /workspace/... root.

@sliverp

sliverp commented Jul 3, 2026

Copy link
Copy Markdown
Member

Pushed the follow-up fix for the structured QQBOT_PAYLOAD virtual-root gap: a383fd41cc.

The structured payload validator now maps allowed local roots through resolveWorkspaceScopedLocalRoots(resolveOutboundMediaLocalRoots(ctx), ctx.mediaAccess?.workspaceDir) before calling resolveLocalPathFromRootsSync(). That keeps the candidate path and the allowed roots in the same host-path coordinate system:

/workspace/attachments/report.pdf -> <scoped-workspace>/attachments/report.pdf
/workspace/attachments            -> <scoped-workspace>/attachments

Updated the regression assertion so the structured payload test now expects the scoped real root instead of the raw virtual root.

Proof:

  • node scripts/run-vitest.mjs extensions/qqbot/src/channel.message-adapter.test.ts extensions/qqbot/src/engine/gateway/outbound-dispatch.test.ts extensions/qqbot/src/engine/messaging/outbound-media-send.test.ts extensions/qqbot/src/engine/messaging/outbound-deliver.test.ts extensions/qqbot/src/engine/messaging/reply-dispatcher.test.ts -> 5 files / 85 tests passed
  • .agents/skills/autoreview/scripts/autoreview --mode local -> clean, no accepted/actionable findings

zhangguiping-xydt and others added 18 commits July 3, 2026 14:59
Cover the seven QQBot outbound media paths that dropped core scoped
sandbox media access: gateway block/tool/streaming deliver, direct
text qqmedia tags, structured QQBOT_PAYLOAD, and the scoped workspace
relative/missing-voice path resolution. The low-level sendPhoto /
sendVideoMsg / sendDocument now consume core mediaAccess.readFile via
the shared outbound loader with explicit localRoots guard, so
host-mediated workspace media can be sent without widening QQBot FS
access. Voice stays on the existing SILK transcode path to avoid
bypassing QQ-required format conversion.
Gateway dispatchOutbound previously built gatewayMediaContext with only
mediaLocalRoots, so a sandboxed block reply that carried a relative
media tag (e.g. <qqmedia>report.docx</qqmedia>) reached the downstream
sender without mediaAccess.workspaceDir and was rejected by containment
checks before the file could be resolved against the agent workspace.

Resolve the agent workspace dir via the public plugin-sdk agent-runtime
helper and pass it through mediaAccess.workspaceDir alongside the
existing mediaLocalRoots so the gateway block reply, tool forwarding,
QQBOT_PAYLOAD, and official C2C streaming paths all share the same
scoped workspace resolution as direct outbound sends. No public plugin
SDK boundary is widened; the helper is an existing export.

Add a regression test that exercises a relative <qqmedia> tag in a
gateway block reply and confirms it resolves against the configured
agent workspace.
Keep the conflict-resolution update scoped to the QQBot media send files.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@sliverp

sliverp commented Jul 3, 2026

Copy link
Copy Markdown
Member

Landed via temp rebase onto main (gate skipped per request).

Thanks @zhangguiping-xydt!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

channel: qqbot merge-risk: 🚨 message-delivery 🚨 May drop, duplicate, misroute, suppress, or wrongly target messages. merge-risk: 🚨 security-boundary 🚨 May affect sandboxing, authorization, credentials, or sensitive data. P2 Normal backlog priority with limited blast radius. proof: 📸 screenshot Contributor real behavior proof includes screenshot evidence. proof: sufficient ClawSweeper judged the real behavior proof convincing. proof: supplied External PR includes structured after-fix real behavior proof. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. size: XL status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants