fix(tess): enforce command authorization approvals #718

2026-07-12T21:34:34Z

jason.woltje commented

2026-07-12 21:34:34 +00:00

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

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

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

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

jason.woltje referenced this issue from a commit

2026-07-12 23:18:03 +00:00

fix(tess): enforce command authorization approvals (#718)

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: mosaicstack/stack#718