Codex review: needs changes before merge.

Summary
The PR gates QQBot /bot-streaming through command authorization, aligns pre-dispatch command allowlist handling with explicit non-wildcard matching, preserves private-chat gating on the framework path, and adds regression coverage.

Reproducibility: yes. from source. Current main documents /bot-streaming as an explicit non-wildcard admin command, but its registry entry lacks requireAuth and the dispatcher only applies the auth guard when that flag is present.

Next step before merge
A repair worker can add the single required changelog entry without changing the QQBot auth implementation.

Security
Cleared: The diff narrows QQBot config-writing command access and does not change dependencies, secrets, workflows, or network calls.

Review findings

• [P3] Add the required changelog entry — extensions/qqbot/src/engine/commands/builtin/register-streaming.ts:33

Not applicable; changelog is handled at merge time.

Quoted comment from @clawsweeper:

Codex review: found issues before merge.

Summary
The PR adds QQBot /bot-streaming to the auth-gated command registry, propagates c2cOnly into framework command registration, adds a framework private-chat guard, and adds regression tests for wildcard-only rejection plus explicit-sender success.

Reproducibility: yes. from source. Current main documents /bot-streaming as explicit-allowlist-only but registers it without requireAuth, and the PR’s remaining mixed-wildcard bypass follows directly from resolveSlashCommandAuth passing * through to createQQBotSenderMatcher.

Next step before merge
The protected maintainer label and the security-sensitive mixed-wildcard auth finding make this explicit maintainer-handled PR review work rather than an automated repair lane.

Security
Needs attention: The diff narrows the command surface, but the mixed wildcard allowlist path leaves an admin-command authorization bypass for /bot-streaming.

Review findings

• [P2] Strip wildcard entries before command auth — extensions/qqbot/src/engine/commands/builtin/register-streaming.ts:33
• [P3] Add the required changelog entry — extensions/qqbot/src/engine/commands/builtin/register-streaming.ts:33

Review details

Best possible solution:

Update QQBot command authorization so requireAuth matches only explicit non-wildcard entries, add regression coverage for mixed wildcard allowlists on /bot-streaming, add the changelog entry, then let a maintainer review and land the narrow hardening.

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

Yes, from source. Current main documents /bot-streaming as explicit-allowlist-only but registers it without requireAuth, and the PR’s remaining mixed-wildcard bypass follows directly from resolveSlashCommandAuth passing * through to createQQBotSenderMatcher.

Is this the best way to solve the issue?

No, not yet. Reusing requireAuth is the right maintainable seam, but the fix is incomplete until command auth ignores wildcard entries for admin-command matching and the changelog is added.

Full review comments:

• [P2] Strip wildcard entries before command auth — extensions/qqbot/src/engine/commands/builtin/register-streaming.ts:33
  Adding requireAuth still leaves /bot-streaming open when allowFrom mixes * with an explicit admin ID: resolveSlashCommandAuth sees an explicit entry, then createQQBotSenderMatcher accepts the wildcard for any sender. That violates the documented non-wildcard admin-command rule and leaves this config write reachable for wildcard DM users.
  Confidence: 0.9
• [P3] Add the required changelog entry — extensions/qqbot/src/engine/commands/builtin/register-streaming.ts:33
  This changes user-visible QQBot command authorization: wildcard/open DM users who could previously toggle /bot-streaming now receive a permission response. Repo policy requires user-facing fix/security changes to include an Unreleased CHANGELOG.md entry, but the PR diff does not touch the changelog.
  Confidence: 0.92

Overall correctness: patch is incorrect
Overall confidence: 0.88

Security concerns:

• [medium] Mixed wildcard allowlists still authorize admin commands — extensions/qqbot/src/engine/commands/slash-command-auth.ts:47
resolveSlashCommandAuth requires some explicit entry but then passes the full list to a matcher that accepts *, so allowFrom: ["*", "ADMIN"] still authorizes any sender for requireAuth commands, including the newly gated streaming config write.
  Confidence: 0.9

What I checked:

• Current docs define the expected admin-command boundary: QQBot docs list /bot-streaming among admin commands that are direct-message-only and require the sender openid in an explicit non-wildcard allowFrom; wildcard chat access is documented as insufficient. Public docs: docs/channels/qqbot.md. (docs/channels/qqbot.md:252, 31161abd40fd)
• Current main lacks the bot-streaming auth flag: /bot-streaming is registered with c2cOnly: true but no requireAuth, while the handler persists QQBot streaming config through replaceConfigFile. (extensions/qqbot/src/engine/commands/builtin/register-streaming.ts:30, 31161abd40fd)
• Dispatcher auth depends on requireAuth: The existing dispatcher rejects unauthorized senders only when cmd.requireAuth && !ctx.commandAuthorized, so adding requireAuth is the right seam for this command. (extensions/qqbot/src/engine/commands/slash-commands.ts:179, 31161abd40fd)
• Mixed wildcard allowlists remain authorized: resolveSlashCommandAuth checks only that at least one explicit entry exists, then passes the full normalized list to createQQBotSenderMatcher, whose wildcard branch authorizes any sender when * is present. (extensions/qqbot/src/engine/commands/slash-command-auth.ts:41, 31161abd40fd)
• PR applies the auth seam but does not cover the mixed wildcard case: The PR diff adds requireAuth: true to /bot-streaming and tests wildcard-only rejection plus explicit-sender success, but it does not change the mixed wildcard matcher behavior. (extensions/qqbot/src/engine/commands/builtin/register-streaming.ts:33, 81161c7b5a3c)
• Required changelog entry is still absent: The PR diff touches QQBot command and test files only, and the active Unreleased fixes section has no QQBot streaming authorization entry. (CHANGELOG.md:14, 31161abd40fd)

Likely related people:

• cxyhhhhh: The QQBot streaming command traces to commit 5ccf179a34a97b165b5d6f149abcf4190e35efcc, and the related fix(qqbot): unify slash command auth, c2cOnly gating, and file delivery #73616 slash-command auth/c2cOnly work traces to 62fb87641eca39821e54ae560e7e90b1684903a3; the provided related context credits fix(qqbot): unify slash command auth, c2cOnly gating, and file delivery #73616 to this contributor. (role: introduced behavior and adjacent QQBot command-auth contributor; confidence: high; commits: 5ccf179a34a9, 62fb87641eca; files: extensions/qqbot/src/engine/commands/builtin/register-streaming.ts, extensions/qqbot/src/engine/commands/slash-commands.ts, extensions/qqbot/src/engine/commands/slash-command-handler.ts)
• steipete: The existing ClawSweeper review context identified steipete as a relevant maintainer for the same QQBot command registry/docs surface, and this PR carries the protected maintainer label requiring explicit maintainer handling. (role: maintainer routing candidate; confidence: medium; commits: 68359cacbf58; files: extensions/qqbot/src/engine/commands/builtin/register-streaming.ts, extensions/qqbot/src/engine/commands/slash-commands.ts, docs/channels/qqbot.md)

Remaining risk / open question:

• The mixed wildcard plus explicit allowFrom case still leaves /bot-streaming reachable to wildcard DM users unless command auth strips wildcard entries before matching.
• The PR still needs an Unreleased CHANGELOG.md entry for the user-visible QQBot command authorization change.
• No targeted test command was run during this read-only review; the focused QQBot command tests should run before merge.

Codex review notes: model gpt-5.5, reasoning high; reviewed against 31161abd40fd.

Re-review progress:

• State: Complete
• Detail: The targeted re-review finished, the durable review comment was updated, and the synced verdict was routed.
• Run: https://github.com/openclaw/clawsweeper/actions/runs/25311181546
• Updated: 2026-05-04T09:22:27.748Z