diff --git a/apps/gateway/src/commands/command-authorization.service.spec.ts b/apps/gateway/src/commands/command-authorization.service.spec.ts new file mode 100644 index 0000000..51b25eb --- /dev/null +++ b/apps/gateway/src/commands/command-authorization.service.spec.ts @@ -0,0 +1,58 @@ +import { describe, expect, it } from 'vitest'; +import type { CommandDef, SlashCommandPayload } from '@mosaicstack/types'; +import { CommandAuthorizationService } from './command-authorization.service.js'; + +const adminCommand: CommandDef = { + name: 'gc', + description: 'GC', + aliases: [], + scope: 'admin', + execution: 'socket', + available: true, +}; +const payload: SlashCommandPayload = { command: 'gc', conversationId: 'conversation-1' }; + +function createService(role: string): CommandAuthorizationService { + const entries = new Map(); + const db = { + select: () => ({ from: () => ({ where: () => ({ limit: async () => [{ role }] }) }) }), + }; + const redis = { + get: async (key: string) => entries.get(key) ?? null, + set: async (key: string, value: string) => { + entries.set(key, value); + }, + del: async (key: string) => Number(entries.delete(key)), + }; + return new CommandAuthorizationService(db as never, redis); +} + +describe('CommandAuthorizationService', () => { + it('consumes one exact actor-bound approval once', async (): Promise => { + const service = createService('admin'); + const approval = await service.createApproval('gc', payload, 'admin-1'); + expect( + (await service.authorize(adminCommand, payload, 'admin-1', approval.approvalId)).allowed, + ).toBe(true); + expect( + (await service.authorize(adminCommand, payload, 'admin-1', approval.approvalId)).allowed, + ).toBe(false); + }); + + it('rejects an approval when the structured action is mutated', async (): Promise => { + const service = createService('admin'); + const approval = await service.createApproval('gc', payload, 'admin-1'); + const mutated = { ...payload, conversationId: 'other-conversation' }; + expect( + (await service.authorize(adminCommand, mutated, 'admin-1', approval.approvalId)).allowed, + ).toBe(false); + }); + + it('denies an admin command to a member before approval is considered', async (): Promise => { + const service = createService('member'); + const approval = await service.createApproval('gc', payload, 'member-1'); + expect( + (await service.authorize(adminCommand, payload, 'member-1', approval.approvalId)).allowed, + ).toBe(false); + }); +}); diff --git a/apps/gateway/src/commands/command-authorization.service.ts b/apps/gateway/src/commands/command-authorization.service.ts new file mode 100644 index 0000000..f6249a4 --- /dev/null +++ b/apps/gateway/src/commands/command-authorization.service.ts @@ -0,0 +1,135 @@ +import { createHash, randomUUID } from 'node:crypto'; +import { Inject, Injectable } from '@nestjs/common'; +import { eq, users as usersTable, type Db } from '@mosaicstack/db'; +import type { CommandDef, SlashCommandPayload } from '@mosaicstack/types'; +import { DB } from '../database/database.module.js'; +import { COMMANDS_REDIS } from './commands.tokens.js'; + +export type CommandRole = 'admin' | 'member' | 'viewer'; + +export interface CommandApproval { + approvalId: string; + actionDigest: string; + actorId: string; + command: string; + expiresAt: string; +} + +export interface CommandAuthorizationResult { + allowed: boolean; + reason?: string; +} + +@Injectable() +export class CommandAuthorizationService { + constructor( + @Inject(DB) private readonly db: Db, + @Inject(COMMANDS_REDIS) + private readonly redis: { + get(key: string): Promise; + set(key: string, value: string, ...args: string[]): Promise; + del(key: string): Promise; + }, + ) {} + + async authorize( + command: CommandDef, + payload: SlashCommandPayload, + actorId: string, + approvalId?: string, + ): Promise { + const role = await this.resolveRole(actorId); + if (!role || !this.hasScope(role, command.scope)) { + return { allowed: false, reason: 'not authorized for this command scope' }; + } + if (command.scope !== 'admin') return { allowed: true }; + if (!approvalId) return { allowed: false, reason: 'durable approval is required' }; + const actionDigest = this.actionDigest(command.name, payload); + const approved = await this.consumeApproval(approvalId, actorId, actionDigest); + return approved + ? { allowed: true } + : { + allowed: false, + reason: 'approval is invalid, expired, replayed, or does not match this action', + }; + } + + async createApproval( + command: string, + payload: SlashCommandPayload, + actorId: string, + ): Promise { + const approvalId = randomUUID(); + const expiresAt = new Date(Date.now() + 5 * 60_000).toISOString(); + const approval: CommandApproval = { + approvalId, + actionDigest: this.actionDigest(command, payload), + actorId, + command, + expiresAt, + }; + await this.redis.set(this.key(approvalId), JSON.stringify(approval), 'EX', '300'); + return approval; + } + + private async resolveRole(actorId: string): Promise { + const [user] = await this.db + .select({ role: usersTable.role }) + .from(usersTable) + .where(eq(usersTable.id, actorId)) + .limit(1); + const role = user?.role; + return role === 'admin' || role === 'member' || role === 'viewer' ? role : null; + } + + private hasScope(role: CommandRole, scope: CommandDef['scope']): boolean { + if (role === 'admin') return true; + return role === 'member' && (scope === 'core' || scope === 'agent'); + } + + private async consumeApproval( + approvalId: string, + actorId: string, + actionDigest: string, + ): Promise { + const key = this.key(approvalId); + const encoded = await this.redis.get(key); + if (!encoded) return false; + const parsed: unknown = JSON.parse(encoded); + if ( + !this.isApproval(parsed) || + parsed.actorId !== actorId || + parsed.actionDigest !== actionDigest || + Date.parse(parsed.expiresAt) <= Date.now() + ) + return false; + return (await this.redis.del(key)) === 1; + } + + private actionDigest(command: string, payload: SlashCommandPayload): string { + return createHash('sha256') + .update( + JSON.stringify({ + command, + args: payload.args?.trim() ?? '', + conversationId: payload.conversationId, + }), + ) + .digest('hex'); + } + + private isApproval(value: unknown): value is CommandApproval { + return ( + typeof value === 'object' && + value !== null && + 'approvalId' in value && + 'actionDigest' in value && + 'actorId' in value && + 'expiresAt' in value + ); + } + + private key(approvalId: string): string { + return `tess:command-approval:${approvalId}`; + } +} diff --git a/apps/gateway/src/commands/command-executor-tess-security.spec.ts b/apps/gateway/src/commands/command-executor-tess-security.spec.ts new file mode 100644 index 0000000..cd3cd09 --- /dev/null +++ b/apps/gateway/src/commands/command-executor-tess-security.spec.ts @@ -0,0 +1,73 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import type { SlashCommandPayload } from '@mosaicstack/types'; +import { CommandExecutorService } from './command-executor.service.js'; + +const registry = { + getManifest: vi.fn(() => ({ + version: 1, + commands: [ + { + name: 'gc', + description: 'System-wide garbage collection', + aliases: [], + scope: 'admin' as const, + execution: 'socket' as const, + available: true, + }, + ], + skills: [], + })), +}; + +const sessionGc = { + sweepOrphans: vi.fn().mockResolvedValue({ orphanedSessions: 1, totalCleaned: [], duration: 1 }), +}; + +const authorization = { + authorize: vi.fn((_command: unknown, _payload: unknown, actorId: string) => + Promise.resolve( + actorId === 'member-1' + ? { allowed: false, reason: 'durable approval is required' } + : { allowed: false, reason: 'not authorized for this command scope' }, + ), + ), +}; + +function buildExecutor(): CommandExecutorService { + return new CommandExecutorService( + registry as never, + { getSession: vi.fn() } as never, + { clear: vi.fn(), set: vi.fn() } as never, + sessionGc as never, + { set: vi.fn() } as never, + { agents: {} } as never, + null, + null, + null, + authorization as never, + ); +} + +describe('TESS-M1-SEC-001 command authorization abuse cases', () => { + const payload: SlashCommandPayload = { command: 'gc', conversationId: 'conversation-1' }; + + beforeEach((): void => { + vi.clearAllMocks(); + }); + + it('denies a forged admin identity and does not execute a system-wide command', async (): Promise => { + const result = await buildExecutor().execute(payload, 'admin-forged-by-client'); + + expect(result.success).toBe(false); + expect(result.message).toContain('not authorized'); + expect(sessionGc.sweepOrphans).not.toHaveBeenCalled(); + }); + + it('denies a privileged command without a server-bound durable approval', async (): Promise => { + const result = await buildExecutor().execute(payload, 'member-1'); + + expect(result.success).toBe(false); + expect(result.message).toContain('approval'); + expect(sessionGc.sweepOrphans).not.toHaveBeenCalled(); + }); +}); diff --git a/apps/gateway/src/commands/command-executor.service.ts b/apps/gateway/src/commands/command-executor.service.ts index f7fc0cc..c4ed381 100644 --- a/apps/gateway/src/commands/command-executor.service.ts +++ b/apps/gateway/src/commands/command-executor.service.ts @@ -11,6 +11,7 @@ import { ReloadService } from '../reload/reload.service.js'; import { McpClientService } from '../mcp-client/mcp-client.service.js'; import { BRAIN } from '../brain/brain.tokens.js'; import { COMMANDS_REDIS } from './commands.tokens.js'; +import { CommandAuthorizationService } from './command-authorization.service.js'; import { CommandRegistryService } from './command-registry.service.js'; @Injectable() @@ -33,6 +34,9 @@ export class CommandExecutorService { @Optional() @Inject(McpClientService) private readonly mcpClient: McpClientService | null, + @Optional() + @Inject(CommandAuthorizationService) + private readonly authorization: CommandAuthorizationService | null = null, ) {} async execute( @@ -52,6 +56,16 @@ export class CommandExecutorService { }; } + const authorization = await this.authorization?.authorize( + def, + payload, + userId, + payload.approvalId, + ); + if (authorization && !authorization.allowed) { + return { command, conversationId, success: false, message: authorization.reason }; + } + try { switch (command) { case 'model': diff --git a/apps/gateway/src/commands/commands.module.ts b/apps/gateway/src/commands/commands.module.ts index 1c3a82c..0d6c30d 100644 --- a/apps/gateway/src/commands/commands.module.ts +++ b/apps/gateway/src/commands/commands.module.ts @@ -3,6 +3,7 @@ import { createQueue, type QueueHandle } from '@mosaicstack/queue'; import { ChatModule } from '../chat/chat.module.js'; import { GCModule } from '../gc/gc.module.js'; import { ReloadModule } from '../reload/reload.module.js'; +import { CommandAuthorizationService } from './command-authorization.service.js'; import { CommandExecutorService } from './command-executor.service.js'; import { CommandRegistryService } from './command-registry.service.js'; import { COMMANDS_REDIS } from './commands.tokens.js'; @@ -24,6 +25,7 @@ const COMMANDS_QUEUE_HANDLE = 'COMMANDS_QUEUE_HANDLE'; inject: [COMMANDS_QUEUE_HANDLE], }, CommandRegistryService, + CommandAuthorizationService, CommandExecutorService, ], exports: [CommandRegistryService, CommandExecutorService], diff --git a/docs/scratchpads/tess-m1-sec-001.md b/docs/scratchpads/tess-m1-sec-001.md new file mode 100644 index 0000000..b297170 --- /dev/null +++ b/docs/scratchpads/tess-m1-sec-001.md @@ -0,0 +1,27 @@ +# TESS-M1-SEC-001 — Command authorization and exact-action approval + +- Issue/milestone: #707 / M1 +- Branch: `fix/tess-command-authz` +- Requirement: `TESS-SEC-002`, with approval binding controls from `TESS-SEC-007` +- Scope: `apps/gateway` only, plus required in-repo security/developer documentation. + +## Plan + +1. Locate the gateway command executor, command metadata, authorization context, and existing test conventions. +2. Write abuse/authz tests before production changes. Expected red cases: non-admin blocked from admin/system command; forged caller scope cannot authorize; privileged/destructive action requires durable exact-action approval; expired/replayed/mutated approvals deny. +3. Implement server-derived role/scope enforcement and durable approval validation/consumption with audit results. +4. Run focused security tests, then repository baseline gates: typecheck, lint, format-check, test. +5. Run independent security/code review, commit, queue-guard, push, and open the PR to `main` through the stated Gitea API fallback. Stop after PR creation. + +## Assumptions + +- The existing gateway persistence interface is the available durable approval boundary. If no persistence abstraction exists, a minimal injectable repository interface will be introduced rather than an in-memory approval implementation, because TESS-SEC-002/007 require durable enforcement. +- “Exact action” is a canonical digest over structured command identity and normalized arguments; role/scope checks always use authenticated server context, not client-declared claims. + +## TDD evidence + +- Pending: abuse/authz test written and observed red before implementation. + +## Verification evidence + +- Pending.