Skip to content

fix: keep group activation owner controlled [AI]#97838

Merged
pgondhi987 merged 2 commits into
openclaw:mainfrom
pgondhi987:fix/fix-778
Jun 30, 2026
Merged

fix: keep group activation owner controlled [AI]#97838
pgondhi987 merged 2 commits into
openclaw:mainfrom
pgondhi987:fix/fix-778

Conversation

@pgondhi987

@pgondhi987 pgondhi987 commented Jun 29, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

What Problem This Solves

Fixes an issue where group chat users who were authorized to run regular commands could change the group activation mode even when they were not configured as the owner.

Why This Change Was Made

The activation command now uses the same authorized-sender and owner checks as neighboring session-state commands before it writes activation state. The change keeps the existing owner success path and adds regression coverage for authorized non-owner attempts.

User Impact

Group activation settings remain owner-controlled. Authorized non-owner senders can still use commands allowed by their authorization level, but they cannot change the per-group activation mode. This stricter /activation behavior is intentional because the command is documented as owner-only.

Evidence

  • node scripts/run-vitest.mjs src/auto-reply/reply/commands-session.activation.test.ts
  • node --import tsx --eval '<real handleActivationCommand owner/non-owner probe>' showed an authorized non-owner left groupActivation=mention and persisted no session key, while the owner changed groupActivation=always and persisted the owner session key.

AI-assisted: yes.

@openclaw-barnacle openclaw-barnacle Bot added size: S maintainer Maintainer-authored PR labels Jun 29, 2026
@clawsweeper

clawsweeper Bot commented Jun 29, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed June 29, 2026, 11:33 AM ET / 15:33 UTC.

Summary
The PR routes /activation through the shared unauthorized and non-owner command gates before writing group activation state, and adds focused Vitest coverage for owner and non-owner attempts.

PR surface: Source +2, Tests +95. Total +97 across 2 files.

Reproducibility: yes. from source inspection: current main lets an authorized non-owner pass the /activation gate and reach the session persistence path. I did not run tests because this review is read-only.

Review metrics: 1 noteworthy metric.

  • Command authorization change: 1 command made owner-only. The patch changes an existing group management command from authorized-sender access to owner-only access, which matters for upgrade compatibility.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🐚 platinum hermit
Patch quality: 🦞 diamond lobster
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] Add a short live Telegram transcript or Mantis proof if maintainers want transport-level evidence before accepting the compatibility change.

Mantis proof suggestion
A live Telegram transcript would directly show the non-owner /activation rejection and owner success path that this PR changes. A maintainer can ask Mantis to capture proof by posting this exact PR comment:

@openclaw-mantis telegram live: verify an authorized non-owner in a group cannot change /activation while the configured owner still can.

Risk before merge

  • [P1] Merging intentionally makes /activation fail closed for command-authorized non-owner group users; existing groups that relied on native command authorization without an explicit owner path may need owner configuration before they can change activation mode.

Maintainer options:

  1. Accept Owner-Only Activation (recommended)
    Land the patch with the PR-body release-note context if maintainers agree ordinary authorized group users must no longer manage activation.
  2. Add Explicit Delegation
    If non-owner delegates should manage activation, add a documented delegation or config path instead of reusing ordinary command authorization.
  3. Require Live Telegram Proof
    If maintainers want transport-level proof, request a short Telegram live transcript showing non-owner rejection and owner success before merge.

Next step before merge

  • [P2] Human maintainer review remains the right path because the protected label and compatibility-sensitive permission tightening require acceptance, not an automated repair.

Security
Cleared: The diff tightens an existing command authorization boundary and adds tests; it does not change dependencies, workflows, secrets handling, or supply-chain execution surfaces.

Review details

Best possible solution:

Land the shared owner-gate implementation after maintainers confirm owner-only activation is the intended upgrade behavior for group chats.

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

Yes from source inspection: current main lets an authorized non-owner pass the /activation gate and reach the session persistence path. I did not run tests because this review is read-only.

Is this the best way to solve the issue?

Yes for the code shape: reusing the shared unauthorized and non-owner gates is the narrowest maintainable fix and matches /send. The remaining question is maintainer acceptance of the stricter upgrade behavior, not a different implementation layer.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 1052652a7168.

Label changes

Label changes:

  • add proof: sufficient: Contributor real behavior proof is sufficient. The updated PR body/comment provides redacted terminal proof from the real command handler and persistence path for both non-owner rejection and owner success.
  • add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🦞 diamond lobster.
  • add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The updated PR body/comment provides redacted terminal proof from the real command handler and persistence path for both non-owner rejection and owner success.
  • remove rating: 🦪 silver shellfish: Current PR rating is rating: 🐚 platinum hermit, so this older rating label is no longer current.
  • remove status: 📣 needs proof: Current PR status label is status: 👀 ready for maintainer look.

Label justifications:

  • P2: The PR fixes a bounded group command authorization bug with limited blast radius but real user-facing permission impact.
  • merge-risk: 🚨 compatibility: The stricter /activation gate can stop existing authorized non-owner group users from changing activation mode until owner identity is configured.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🦞 diamond lobster.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The updated PR body/comment provides redacted terminal proof from the real command handler and persistence path for both non-owner rejection and owner success.
  • proof: sufficient: Contributor real behavior proof is sufficient. The updated PR body/comment provides redacted terminal proof from the real command handler and persistence path for both non-owner rejection and owner success.
  • mantis: telegram-visible-proof: Mantis should capture Telegram visible proof. The PR changes visible Telegram group command authorization behavior that can be demonstrated in a short Telegram proof.
Evidence reviewed

PR surface:

Source +2, Tests +95. Total +97 across 2 files.

View PR surface stats
Area Files Added Removed Net
Source 1 7 5 +2
Tests 1 95 0 +95
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 2 102 5 +97

What I checked:

Likely related people:

  • steipete: GitHub commit history for src/auto-reply/group-activation.ts shows the original group activation command and multiple later parser/runtime helper refactors under this account. (role: introduced behavior and recent area contributor; confidence: medium; commits: f10c8f2b4c6e, 6d4d313d440e, 00d8d7ead059; files: src/auto-reply/group-activation.ts, src/auto-reply/reply/commands-session.ts)
  • vincentkoc: Authored and merged the command-gate work that introduced rejectNonOwnerCommand, the helper this PR applies to /activation. (role: command-gate contributor; confidence: high; commits: 08aa57a3de37; files: src/auto-reply/reply/command-gates.ts, src/auto-reply/reply/commands-config.ts)
  • drobison00: Authored and merged the sibling /send owner-gating fix that this PR mirrors for group activation state. (role: adjacent invariant contributor; confidence: high; commits: 47dc7fe81600, 2aa93d44a1b2; files: src/auto-reply/reply/commands-session.ts, src/auto-reply/command-auth.ts)
  • pgondhi987: Besides authoring this PR, this account appears in prior merged command authorization history for native command owner enforcement and gateway command scopes. (role: authorization contributor; confidence: medium; commits: 758051322da4, 652f5f9b1087; files: src/auto-reply/command-auth.ts, src/auto-reply/reply/command-gates.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 rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. mantis: telegram-visible-proof Mantis should capture Telegram visible proof. P2 Normal backlog priority with limited blast radius. merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. labels Jun 29, 2026
@pgondhi987

pgondhi987 commented Jun 29, 2026

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

Added the requested real behavior proof to the PR body.

Redacted terminal proof from the PR branch using the real handleActivationCommand module and real persistSessionEntry path, with no mocks:

  • authorized non-owner /activation always: shouldContinue=false, groupActivation stayed mention, groupActivationNeedsSystemIntro=false, persisted session keys [].
  • owner /activation always: shouldContinue=false, reply Group activation set to always, groupActivation changed to always, groupActivationNeedsSystemIntro=true, persisted session key telegram:group:owner.

Exact commands already run for this head:

  • node --import tsx --eval '<real handleActivationCommand owner/non-owner probe>'
  • node scripts/run-vitest.mjs src/auto-reply/reply/commands-session.activation.test.ts

@clawsweeper

clawsweeper Bot commented Jun 29, 2026 •
edited
Loading

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.

@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: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Jun 29, 2026
@pgondhi987

pgondhi987 commented Jun 29, 2026

Copy link
Copy Markdown
Contributor Author

Verification before merge:

Behavior addressed: /activation now preserves the owner-only command boundary for group activation changes; authorized non-owner group senders cannot persist activation state.
Real environment tested: local source checkout and GitHub Actions PR CI. No completed live Telegram transcript was used; ClawSweeper downgraded that to optional maintainer-requested proof.
Exact steps or command run after this patch:

  • node scripts/run-vitest.mjs src/auto-reply/reply/commands-session.activation.test.ts
  • scripts/pr review-tests 97838 src/auto-reply/reply/commands-session.activation.test.ts
  • timeout 1800 .agents/skills/autoreview/scripts/autoreview --mode branch --base origin/main
  • auto-pr.sh issue 778 --run-stage ghsa_dry_run
  • auto-pr.sh issue 778 --run-stage ghsa_real_gate
    Evidence after fix:
  • Head beccb452f9f19d137298faae1f6c21790c43c4f6: focused Vitest passed 1 file / 2 tests.
  • Real module probe: authorized non-owner left groupActivation=mention and persisted no key; owner changed it to always and persisted the owner key.
  • review-pr: passed, READY for /prepare-pr, zero findings; autoreview: passed, no accepted/actionable findings.
  • GHSA dry-run: passed; GHSA real gate: passed; GitHub PR checks: 64 pass, 34 skipping, no non-pass buckets.
    Observed result after fix: the mutation path now reaches session persistence only after both command authorization and owner checks pass.
    What was not tested: live Telegram UI transcript was not completed; no known untested critical code path for the handler-level owner gate.

Regression Risk:
Medium. This intentionally tightens auth for one documented owner-only chat command. Auth/policy behavior changes only for /activation; config, migrations, storage shape, provider/plugin/channel transport code, sandboxing, dependencies, approvals, and persisted credential handling are unchanged.

Best fix verdict: appropriate. The command handler owns this session-state write, and sibling owner-only commands already use the same shared gates. A dispatch-level owner gate would be too broad; a transport-specific fix would miss the globally registered command.

User behavior change:
Before, a command-authorized non-owner in a group could set /activation mention|always. After, only the owner can change that per-group activation mode; authorized non-owners keep their existing non-owner command access.

@openclaw-mantis

openclaw-mantis Bot commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

Mantis Telegram Desktop Proof

Summary: Mantis captured Telegram Desktop before/after GIFs for /activation always; this PR still showed the group activation setting changed.

Main screenshot This PR screenshot
Baseline native Telegram Desktop screenshot Candidate native Telegram Desktop screenshot
Main This PR
Baseline native Telegram Desktop proof GIF Candidate native Telegram Desktop proof GIF

Motion-trimmed clips:

Raw QA files: https://artifacts.openclaw.ai/mantis/telegram-desktop/pr-97838/run-28382849080-1/index.json

@pgondhi987
pgondhi987 merged commit 3d4b7ca into openclaw:main Jun 30, 2026
xialonglee pushed a commit to xialonglee/openclaw that referenced this pull request Jun 30, 2026
@pgondhi987 @xialonglee
fix: gate group activation changes by owner (openclaw#97838)
90dc531
github-actions Bot pushed a commit to Desicool/openclaw that referenced this pull request Jul 1, 2026
@pgondhi987
fix: gate group activation changes by owner (openclaw#97838)
9eaa58b
QiuYuang pushed a commit to QiuYuang/openclaw that referenced this pull request Jul 1, 2026
@pgondhi987 @QiuYuang
fix: gate group activation changes by owner (openclaw#97838)
5ccd234
chenyangjun-xy pushed a commit to chenyangjun-xy/openclaw that referenced this pull request Jul 1, 2026
@pgondhi987 chenyangjun-xy
fix: gate group activation changes by owner (openclaw#97838)
fa35e98
chenyangjun-xy pushed a commit to chenyangjun-xy/openclaw that referenced this pull request Jul 3, 2026
@pgondhi987 @chenyangjun-xy
fix: gate group activation changes by owner (openclaw#97838)
21bc657
Rorqualx pushed a commit to Rorqualx/cortex that referenced this pull request Jul 6, 2026
@pgondhi987 cron
fix: gate group activation changes by owner (openclaw#97838)
1f8ea63 
(cherry picked from commit 3d4b7ca)
Rorqualx pushed a commit to Rorqualx/cortex that referenced this pull request Jul 8, 2026
@pgondhi987 cron
fix: gate group activation changes by owner (openclaw#97838)
1270d69 
(cherry picked from commit 3d4b7ca)
wheakerd pushed a commit to wheakerd/clawdbot that referenced this pull request Jul 15, 2026
@pgondhi987 @RomneyDa
fix: gate group activation changes by owner (openclaw#97838)
69f94b4 
(cherry picked from commit 3d4b7ca)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

maintainer Maintainer-authored PR mantis: telegram-visible-proof Mantis should capture Telegram visible proof. merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. P2 Normal backlog priority with limited blast radius. proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. size: S status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@pgondhi987