Skip to content

fix: restrict trajectory export to owners [AI]#97840

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

fix: restrict trajectory export to owners [AI]#97840
pgondhi987 merged 2 commits into
openclaw:mainfrom
pgondhi987:fix/fix-779

Conversation

@pgondhi987

@pgondhi987 pgondhi987 commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

What Problem This Solves

Fixes an issue where authorized channel users who were not owners could start a trajectory export from chat and receive sensitive session artifacts.

Why This Change Was Made

Trajectory export now uses the same owner-only command gate as other sensitive chat commands before building the export reply. The change keeps the existing authorized-sender check and adds the owner check at the command handler boundary, so the export builder is not reached for non-owner senders.

User Impact

Owners can still export trajectory bundles from the active session. Authorized non-owner users can no longer initiate that export path from chat.

Evidence

  • node scripts/run-vitest.mjs src/auto-reply/reply/commands-info.test.ts passed: 1 file, 17 tests.

AI-assisted: yes.

@openclaw-barnacle openclaw-barnacle Bot added size: XS 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 real behavior proof before merge. Reviewed June 30, 2026, 6:37 AM ET / 10:37 UTC.

Summary
The PR adds the existing non-owner command gate to the shared /export-trajectory handler and adds a regression test for authorized non-owner senders.

PR surface: Source +4, Tests +12. Total +16 across 2 files.

Reproducibility: yes. Current-main source shows an authorized non-owner reaches buildExportTrajectoryCommandReply, and existing command-auth tests show authorized non-owner sender state is real; I did not run a live repro in the read-only checkout.

Review metrics: none identified.

Merge readiness
Overall: 🧂 unranked krab
Proof: 🧂 unranked krab
Patch quality: 🦞 diamond lobster
Result: blocked until stronger real behavior proof is added.

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

Rank-up moves:

  • [P1] Add redacted Telegram or real command proof showing an authorized non-owner is denied before export approval or bundle construction.
  • Include owner-success proof showing an owner still reaches the normal trajectory export approval flow.
  • Update the PR body after proof is added so ClawSweeper can re-review automatically; if it does not, ask a maintainer to comment @clawsweeper re-review.

Proof guidance:

  • [P1] Needs stronger real behavior proof before merge: The Mantis proof is inspectable, but the visible candidate screenshot still shows the private-route-unavailable export reply rather than the owner-gate denial and does not show owner success; add redacted proof before merge. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Mantis proof suggestion
A corrected Telegram Desktop proof would materially settle the remaining live behavior gap for both non-owner denial and owner success. A maintainer can ask Mantis to capture proof by posting this exact PR comment:

@openclaw-mantis telegram desktop proof: verify an authorized non-owner sees the owner-gate denial for /export-trajectory and an owner still reaches export approval.

Risk before merge

  • [P1] Merging intentionally stops authorized non-owner users from starting trajectory exports; that matches the docs-backed owner-only contract but can break setups that relied on broad command allowlists.
  • [P1] The posted Mantis artifacts are inspectable but do not visibly prove the changed owner-gate denial or owner-success path, so real transport proof remains incomplete.

Maintainer options:

  1. Require clearer live proof before merge (recommended)
    Capture redacted Telegram or real command proof at the current PR head showing an authorized non-owner receives the owner-gate denial and an owner still reaches the normal export approval flow.
  2. Accept the compatibility change with maintainer signoff
    Maintainers can intentionally merge the owner-only restriction based on the documented security boundary while owning the behavior change for existing authorized non-owner setups.

Next step before merge

  • [P1] This is a maintainer/proof review item because the remaining action is accepting the compatibility impact and producing clearer live proof, not an automated code repair.

Security
Cleared: Cleared: the diff tightens an existing sensitive command authorization boundary and does not add dependency, workflow, credential, package, or supply-chain surface.

Review details

Best possible solution:

Land the shared handler owner gate after maintainers accept the compatibility change and add redacted live proof showing non-owner denial plus owner export approval still working.

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

Yes. Current-main source shows an authorized non-owner reaches buildExportTrajectoryCommandReply, and existing command-auth tests show authorized non-owner sender state is real; I did not run a live repro in the read-only checkout.

Is this the best way to solve the issue?

Yes for the code shape. Reusing rejectNonOwnerCommand at the shared command-handler boundary is the narrow maintainable fix, but merge readiness still depends on clearer real behavior proof and maintainer acceptance of the compatibility impact.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 738b2be4b49b.

Label changes

Label changes:

  • add proof: 📸 screenshot: Contributor real behavior proof includes screenshot evidence. The Mantis proof is inspectable, but the visible candidate screenshot still shows the private-route-unavailable export reply rather than the owner-gate denial and does not show owner success; add redacted proof before merge.
  • add rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🦞 diamond lobster.
  • remove rating: 🦪 silver shellfish: Current PR rating is rating: 🧂 unranked krab, so this older rating label is no longer current.

Label justifications:

  • P1: The PR fixes a sensitive command authorization gap where authorized non-owner chat users can reach trajectory export construction for session artifacts.
  • merge-risk: 🚨 compatibility: The diff intentionally removes trajectory export access for authorized non-owner senders, which can affect existing setups even though it matches the documented owner-only contract.
  • rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🦞 diamond lobster.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs stronger real behavior proof before merge: The Mantis proof is inspectable, but the visible candidate screenshot still shows the private-route-unavailable export reply rather than the owner-gate denial and does not show owner success; add redacted proof before merge. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
  • proof: 📸 screenshot: Contributor real behavior proof includes screenshot evidence. The Mantis proof is inspectable, but the visible candidate screenshot still shows the private-route-unavailable export reply rather than the owner-gate denial and does not show owner success; add redacted proof before merge.
  • mantis: telegram-visible-proof: Mantis should capture Telegram visible proof. This shared command-handler change is visible in Telegram as command denial and export approval behavior, so a short Telegram proof remains useful.
Evidence reviewed

PR surface:

Source +4, Tests +12. Total +16 across 2 files.

View PR surface stats
Area Files Added Removed Net
Source 1 5 1 +4
Tests 1 13 1 +12
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 2 18 2 +16

What I checked:

Likely related people:

  • pashpashpash: GitHub path history shows this person authored the merged core chat diagnostics/trajectory export routing work that brought trajectory export through exec approval and owner-private routing. (role: introduced sensitive command surface; confidence: high; commits: 6ce1058296cc, 4aa8da375659; files: src/auto-reply/reply/commands-export-trajectory.ts, src/auto-reply/reply/commands-info.ts, docs/tools/trajectory.md)
  • vincentkoc: GitHub path history shows this person introduced rejectNonOwnerCommand for owner-only command enforcement and later touched trajectory export approval helpers. (role: owner-gate helper introducer; confidence: high; commits: 08aa57a3de37, 3826cda4d854, ccf5976d0621; files: src/auto-reply/reply/command-gates.ts, src/auto-reply/reply/commands-config.ts, src/auto-reply/reply/commands-export-trajectory.ts)
  • steipete: Recent file history shows repeated work on auto-reply command docs, private approval expiry, and broader command/runtime refactors adjacent to this handler path. (role: recent adjacent contributor; confidence: medium; commits: 119bb5762714, e708a872a1fd, 26355cc35df6; files: src/auto-reply/reply/command-gates.ts, src/auto-reply/reply/commands-export-trajectory.ts, docs/tools/trajectory.md)
  • pgondhi987: Beyond this PR, GitHub path history shows prior merged work in command-scope enforcement, so this person has adjacent history in the command authorization area. (role: prior adjacent security contributor; confidence: medium; commits: 652f5f9b1087; files: 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.

@pgondhi987

pgondhi987 commented Jun 29, 2026

Copy link
Copy Markdown
Contributor Author

Verification before merge:

Behavior addressed: /export-trajectory now enforces the documented owner-only command boundary before any trajectory export approval or bundle is created.
Real environment tested: local source checkout, review-pr artifact worktree, and GitHub Actions PR CI for head 870c0bff0cc4e76d54e6fd809ab1142238b3c252.
Exact steps or command run after this patch:

  • node scripts/run-vitest.mjs src/auto-reply/reply/commands-info.test.ts
  • scripts/pr review-tests 97840 src/auto-reply/reply/commands-info.test.ts
  • timeout 1800 .agents/skills/autoreview/scripts/autoreview --mode branch --base origin/main
  • /home/ubuntu/openclaw-tracking/skill-based-auto-pr-creation/scripts/auto-pr.sh issue 779 --run-stage ghsa_dry_run
  • /home/ubuntu/openclaw-tracking/skill-based-auto-pr-creation/scripts/auto-pr.sh issue 779 --run-stage ghsa_real_gate
    Evidence after fix:
  • Focused local validation passed: 1 Vitest file, 17 tests.
  • review-pr: passed, READY FOR /prepare-pr, zero findings, artifacts validated.
  • autoreview: passed, no accepted/actionable findings.
  • GHSA dry-run: passed; GHSA real gate: passed.
  • CI: GitHub status checks completed on 870c0bff0cc4e76d54e6fd809ab1142238b3c252 with no active or relevant failing checks; PR mergeability is CLEAN.
    Observed result after fix: authorized non-owners are blocked by rejectNonOwnerCommand before buildExportTrajectoryCommandReply can start export approval routing.
    What was not tested: no live channel DM/manual export flow was run; the command handler boundary and PR CI were tested.

Regression Risk:
Medium. This changes auth/policy behavior for one owner-only chat command by enforcing the existing owner gate; it does not change config, storage, migrations, provider/plugin/channel contracts, schemas, or persisted credentials. The main compatibility impact is intentional: authorized non-owner senders can no longer start trajectory exports.

Best fix verdict: best. The command handler owns this gate, and adding the existing owner check there covers both /export-trajectory and /trajectory before approval/export construction without adding new policy or fallback behavior.

User behavior change:
Before, an authorized non-owner sender could start trajectory export from chat. After, only owners can start it; normal owner export behavior is unchanged, and unrelated commands, providers, plugins, channels, storage, and config remain unchanged.

@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. labels Jun 29, 2026
@pgondhi987

pgondhi987 commented Jun 30, 2026

Copy link
Copy Markdown
Contributor Author

@openclaw-mantis telegram live proof: verify an authorized non-owner cannot run /export-trajectory and that an owner can still request the export.

@pgondhi987

pgondhi987 commented Jun 30, 2026

Copy link
Copy Markdown
Contributor Author

Behavior proof for current head 870c0bff0cc4e76d54e6fd809ab1142238b3c252:

Behavior addressed: /export-trajectory is an owner-only command. This patch blocks an authorized non-owner sender before any trajectory export approval or bundle creation can run.

Real environment tested: local OpenClaw source checkout at the PR head, GitHub Actions PR CI for the same head, plus review-pr and autoreview gates.

Exact steps or commands run after this patch:

  • node scripts/run-vitest.mjs src/auto-reply/reply/commands-info.test.ts
  • scripts/pr review-tests 97840 src/auto-reply/reply/commands-info.test.ts
  • timeout 1800 .agents/skills/autoreview/scripts/autoreview --mode branch --base origin/main
  • GitHub PR status rollup for head 870c0bff0cc4e76d54e6fd809ab1142238b3c252

Evidence after fix:

  • Focused local Vitest: 1 file passed, 17 tests passed.
  • review-pr: READY FOR /prepare-pr, zero findings, artifacts validated.
  • autoreview: clean, no accepted/actionable findings.
  • GitHub PR checks: no active or failing checks on this head; 69 successful checks.

Observed result after fix: the regression test sets senderIsOwner=false on an otherwise authorized /export-trajectory request and verifies buildExportTrajectoryCommandReply is not called, matching the owner-only command boundary.

What was not tested: no live chat transport was driven in this pass; the changed behavior is in the shared command handler before channel-specific approval/export routing.

Regression risk: low. The change uses the existing owner-gate helper in the command handler and does not change config, storage, provider, plugin, channel transport, schema, migration, or dependency behavior.

@pgondhi987

pgondhi987 commented Jun 30, 2026

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jun 30, 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.

@pgondhi987

pgondhi987 commented Jun 30, 2026

Copy link
Copy Markdown
Contributor Author

@openclaw-mantis telegram desktop proof

Please verify this PR head against the baseline for /export-trajectory owner gating:

  • an authorized non-owner Telegram sender cannot run /export-trajectory or start the trajectory export approval/bundle flow;
  • the owner can still request /export-trajectory and reach the normal export approval flow.

Use redacted evidence only. Candidate head: 870c0bff0cc4e76d54e6fd809ab1142238b3c252.

@clawsweeper clawsweeper Bot added mantis: telegram-visible-proof Mantis should capture Telegram visible proof. P1 High-priority user-facing bug, regression, or broken workflow. merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. labels Jun 30, 2026
@clawsweeper
clawsweeper Bot temporarily deployed to qa-live-shared June 30, 2026 08:57 Inactive
@openclaw-mantis

openclaw-mantis Bot commented Jun 30, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Mantis Telegram Desktop Proof

Summary: Mantis captured native Telegram Desktop before/after GIFs for /export-trajectory from an authorized non-owner chat sender.

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-97840/run-28432446146-1/index.json

@pgondhi987

pgondhi987 commented Jun 30, 2026

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

@clawsweeper clawsweeper Bot added proof: 📸 screenshot Contributor real behavior proof includes screenshot evidence. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. and removed rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. labels Jun 30, 2026
@pgondhi987

pgondhi987 commented Jun 30, 2026

Copy link
Copy Markdown
Contributor Author

@openclaw-mantis telegram desktop proof: verify an authorized non-owner sees the owner-gate denial for /export-trajectory and an owner still reaches the normal trajectory export approval flow.

Please capture redacted Telegram Desktop before/after evidence for both paths on the current PR head. The previous Mantis proof did not show the owner-gate denial or owner success, so this request needs those exact behaviors.

@pgondhi987

pgondhi987 commented Jun 30, 2026

Copy link
Copy Markdown
Contributor Author

@openclaw-mantis telegram desktop proof

Please verify this PR head against the baseline for /export-trajectory owner gating:

  • an authorized non-owner Telegram sender sees the owner-gate denial for /export-trajectory before export approval or bundle construction;

  • the owner can still request /export-trajectory and reach the normal export approval flow.

  • PR: fix: restrict trajectory export to owners [AI] #97840

  • Candidate head: 870c0bff0cc4e76d54e6fd809ab1142238b3c252

This is a corrected request after ClawSweeper rejected the prior Mantis proof because it showed the private-route-unavailable reply and did not show owner success.

@pgondhi987
pgondhi987 merged commit 6cb82ea into openclaw:main Jun 30, 2026
github-actions Bot pushed a commit to Desicool/openclaw that referenced this pull request Jul 1, 2026
@pgondhi987
fix: require owner for trajectory export (openclaw#97840)
3490858
QiuYuang pushed a commit to QiuYuang/openclaw that referenced this pull request Jul 1, 2026
@pgondhi987 @QiuYuang
fix: require owner for trajectory export (openclaw#97840)
aa9eb19
chenyangjun-xy pushed a commit to chenyangjun-xy/openclaw that referenced this pull request Jul 1, 2026
@pgondhi987 chenyangjun-xy
fix: require owner for trajectory export (openclaw#97840)
308481a
chenyangjun-xy pushed a commit to chenyangjun-xy/openclaw that referenced this pull request Jul 3, 2026
@pgondhi987 @chenyangjun-xy
fix: require owner for trajectory export (openclaw#97840)
c63f1d2
Rorqualx pushed a commit to Rorqualx/cortex that referenced this pull request Jul 6, 2026
@pgondhi987 cron
fix: require owner for trajectory export (openclaw#97840)
33b666d 
(cherry picked from commit 6cb82ea)
Rorqualx pushed a commit to Rorqualx/cortex that referenced this pull request Jul 8, 2026
@pgondhi987 cron
fix: require owner for trajectory export (openclaw#97840)
c0d0588 
(cherry picked from commit 6cb82ea)
@clawsweeper clawsweeper Bot mentioned this pull request Jul 9, 2026
Closed
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. P1 High-priority user-facing bug, regression, or broken workflow. proof: 📸 screenshot Contributor real behavior proof includes screenshot evidence. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. size: XS status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@pgondhi987