mosaicstack/stack
1
0
Fork 0
Code Issues 42 Pull Requests Actions Packages Projects Releases 15 Wiki Activity

fix(tess): enforce command authorization approvals #718

Merged
jason.woltje merged 3 commits from fix/tess-command-authz into main 2026-07-12 23:18:02 +00:00
Conversation 2 Commits 3 Files Changed 10 +473
jason.woltje commented 2026-07-12 21:34:34 +00:00
Owner
Copy Link

Refs #707

Task: TESS-M1-SEC-001

Implements server-side command scope and role enforcement plus actor-bound exact-action approvals for admin commands. Adds abuse coverage for forged identity, missing approval, action mutation, replay, and member denial.

Verification: pnpm typecheck, pnpm lint, pnpm format:check, pnpm test.

Refs #707 Task: TESS-M1-SEC-001 Implements server-side command scope and role enforcement plus actor-bound exact-action approvals for admin commands. Adds abuse coverage for forged identity, missing approval, action mutation, replay, and member denial. Verification: pnpm typecheck, pnpm lint, pnpm format:check, pnpm test.
jason.woltje added 1 commit 2026-07-12 21:34:35 +00:00
fix(tess): enforce command authorization approvals
All checks were successful
ci/woodpecker/push/ci Pipeline was successful
Details
ci/woodpecker/pr/ci Pipeline was successful
Details
6e82b4ab5b
jason.woltje commented 2026-07-12 22:45:59 +00:00
Author
Owner
Copy Link

REQUEST CHANGES reviewer-of-record [W-jarvis:reviewer] head 6e82b4ab5b

Reviewed PR #718 as non-author against main at exact head 6e82b4ab5b2d5616aad1e695cd4c5f2806e0186e.

CI is green on this exact head: Woodpecker push pipeline #1689 and PR pipeline #1692 both success.

Blocking finding:

  • The durable approval path is not wired into the actual command execution ingress. CommandExecutorService.execute() now accepts an optional approvalId, but the WebSocket handler still calls this.commandExecutor.execute(payload, userId) and SlashCommandPayload has no approvalId field. CommandAuthorizationService.createApproval() is also not exposed through any controller/socket/service path in this PR.
  • Result: with CommandAuthorizationService registered in CommandsModule, an admin-scoped command such as /gc requires an approval, but no real client path can create or submit that approval. The PR proves denial cases and direct service consumption, but not an end-to-end approved admin command path.
  • This falls short of the PR claim/task requirement: “server-side command scope and role enforcement plus actor-bound exact-action approvals for admin commands.” It currently makes admin commands fail closed, but does not provide a usable durable exact-action approval execution path.

Expected remediation: wire the approval reference through the command payload/ingress (or add the intended approval creation/consumption API) and add a positive integration/WS test proving an admin command with a valid actor-bound exact-action approval executes once, while replay/mutation still denies.

No merge performed by reviewer.

REQUEST CHANGES reviewer-of-record [W-jarvis:reviewer] head 6e82b4ab5b2d5616aad1e695cd4c5f2806e0186e Reviewed PR #718 as non-author against `main` at exact head `6e82b4ab5b2d5616aad1e695cd4c5f2806e0186e`. CI is green on this exact head: Woodpecker push pipeline #1689 and PR pipeline #1692 both success. Blocking finding: - The durable approval path is not wired into the actual command execution ingress. `CommandExecutorService.execute()` now accepts an optional `approvalId`, but the WebSocket handler still calls `this.commandExecutor.execute(payload, userId)` and `SlashCommandPayload` has no `approvalId` field. `CommandAuthorizationService.createApproval()` is also not exposed through any controller/socket/service path in this PR. - Result: with `CommandAuthorizationService` registered in `CommandsModule`, an admin-scoped command such as `/gc` requires an approval, but no real client path can create or submit that approval. The PR proves denial cases and direct service consumption, but not an end-to-end approved admin command path. - This falls short of the PR claim/task requirement: “server-side command scope and role enforcement plus actor-bound exact-action approvals for admin commands.” It currently makes admin commands fail closed, but does not provide a usable durable exact-action approval execution path. Expected remediation: wire the approval reference through the command payload/ingress (or add the intended approval creation/consumption API) and add a positive integration/WS test proving an admin command with a valid actor-bound exact-action approval executes once, while replay/mutation still denies. No merge performed by reviewer.
jason.woltje force-pushed fix/tess-command-authz from 6e82b4ab5b to e3d98d7390 2026-07-12 22:57:34 +00:00 Compare
jason.woltje commented 2026-07-12 23:17:22 +00:00
Author
Owner
Copy Link

VERIFIED APPROVE reviewer-of-record [W-jarvis:reviewer] head e3d98d7390

Evidence:

  • Exact PR head verified: e3d98d73907f782aaefe08655d0276d7f3e8368a.
  • PR CI green: ci/woodpecker/pr/ci pipeline 1705 success.
  • Re-reviewed current head after prior REQUEST CHANGES on 6e82b4ab; the durable approval ingress is now wired end-to-end: SlashCommandPayload.approvalId, command:approve server-side scope derivation, CommandExecutorService.createApproval(), and execute() consumes the one-time actor/action-bound approval.
  • Coverage includes gateway approval ingress plus denied forged identity, missing approval, action mutation, replay, and member denial cases.
  • git diff --check clean; scope stays within TESS-M1-SEC-001 command authorization.
VERIFIED APPROVE reviewer-of-record [W-jarvis:reviewer] head e3d98d73907f782aaefe08655d0276d7f3e8368a Evidence: - Exact PR head verified: `e3d98d73907f782aaefe08655d0276d7f3e8368a`. - PR CI green: `ci/woodpecker/pr/ci` pipeline 1705 success. - Re-reviewed current head after prior REQUEST CHANGES on `6e82b4ab`; the durable approval ingress is now wired end-to-end: `SlashCommandPayload.approvalId`, `command:approve` server-side scope derivation, `CommandExecutorService.createApproval()`, and `execute()` consumes the one-time actor/action-bound approval. - Coverage includes gateway approval ingress plus denied forged identity, missing approval, action mutation, replay, and member denial cases. - `git diff --check` clean; scope stays within TESS-M1-SEC-001 command authorization.
jason.woltje merged commit 46ca3ce742 into main 2026-07-12 23:18:02 +00:00
jason.woltje deleted branch fix/tess-command-authz 2026-07-12 23:18:02 +00:00
Sign in to join this conversation.
Reviewers
No Reviewers
Labels
Clear labels
fleet-enhancement
Fleet-wide reliability/speed/cost improvements surfaced by the nightly enhance pass
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
jason.woltje
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: mosaicstack/stack#718
Delete Branch "fix/tess-command-authz"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?