fix(tess): enforce command authorization approvals
This commit is contained in:
@@ -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<string, string>();
|
||||||
|
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<void> => {
|
||||||
|
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<void> => {
|
||||||
|
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<void> => {
|
||||||
|
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);
|
||||||
|
});
|
||||||
|
});
|
||||||
135
apps/gateway/src/commands/command-authorization.service.ts
Normal file
135
apps/gateway/src/commands/command-authorization.service.ts
Normal file
@@ -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<string | null>;
|
||||||
|
set(key: string, value: string, ...args: string[]): Promise<unknown>;
|
||||||
|
del(key: string): Promise<number>;
|
||||||
|
},
|
||||||
|
) {}
|
||||||
|
|
||||||
|
async authorize(
|
||||||
|
command: CommandDef,
|
||||||
|
payload: SlashCommandPayload,
|
||||||
|
actorId: string,
|
||||||
|
approvalId?: string,
|
||||||
|
): Promise<CommandAuthorizationResult> {
|
||||||
|
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<CommandApproval> {
|
||||||
|
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<CommandRole | null> {
|
||||||
|
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<boolean> {
|
||||||
|
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}`;
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -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<void> => {
|
||||||
|
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<void> => {
|
||||||
|
const result = await buildExecutor().execute(payload, 'member-1');
|
||||||
|
|
||||||
|
expect(result.success).toBe(false);
|
||||||
|
expect(result.message).toContain('approval');
|
||||||
|
expect(sessionGc.sweepOrphans).not.toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -11,6 +11,7 @@ import { ReloadService } from '../reload/reload.service.js';
|
|||||||
import { McpClientService } from '../mcp-client/mcp-client.service.js';
|
import { McpClientService } from '../mcp-client/mcp-client.service.js';
|
||||||
import { BRAIN } from '../brain/brain.tokens.js';
|
import { BRAIN } from '../brain/brain.tokens.js';
|
||||||
import { COMMANDS_REDIS } from './commands.tokens.js';
|
import { COMMANDS_REDIS } from './commands.tokens.js';
|
||||||
|
import { CommandAuthorizationService } from './command-authorization.service.js';
|
||||||
import { CommandRegistryService } from './command-registry.service.js';
|
import { CommandRegistryService } from './command-registry.service.js';
|
||||||
|
|
||||||
@Injectable()
|
@Injectable()
|
||||||
@@ -33,6 +34,9 @@ export class CommandExecutorService {
|
|||||||
@Optional()
|
@Optional()
|
||||||
@Inject(McpClientService)
|
@Inject(McpClientService)
|
||||||
private readonly mcpClient: McpClientService | null,
|
private readonly mcpClient: McpClientService | null,
|
||||||
|
@Optional()
|
||||||
|
@Inject(CommandAuthorizationService)
|
||||||
|
private readonly authorization: CommandAuthorizationService | null = null,
|
||||||
) {}
|
) {}
|
||||||
|
|
||||||
async execute(
|
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 {
|
try {
|
||||||
switch (command) {
|
switch (command) {
|
||||||
case 'model':
|
case 'model':
|
||||||
|
|||||||
@@ -3,6 +3,7 @@ import { createQueue, type QueueHandle } from '@mosaicstack/queue';
|
|||||||
import { ChatModule } from '../chat/chat.module.js';
|
import { ChatModule } from '../chat/chat.module.js';
|
||||||
import { GCModule } from '../gc/gc.module.js';
|
import { GCModule } from '../gc/gc.module.js';
|
||||||
import { ReloadModule } from '../reload/reload.module.js';
|
import { ReloadModule } from '../reload/reload.module.js';
|
||||||
|
import { CommandAuthorizationService } from './command-authorization.service.js';
|
||||||
import { CommandExecutorService } from './command-executor.service.js';
|
import { CommandExecutorService } from './command-executor.service.js';
|
||||||
import { CommandRegistryService } from './command-registry.service.js';
|
import { CommandRegistryService } from './command-registry.service.js';
|
||||||
import { COMMANDS_REDIS } from './commands.tokens.js';
|
import { COMMANDS_REDIS } from './commands.tokens.js';
|
||||||
@@ -24,6 +25,7 @@ const COMMANDS_QUEUE_HANDLE = 'COMMANDS_QUEUE_HANDLE';
|
|||||||
inject: [COMMANDS_QUEUE_HANDLE],
|
inject: [COMMANDS_QUEUE_HANDLE],
|
||||||
},
|
},
|
||||||
CommandRegistryService,
|
CommandRegistryService,
|
||||||
|
CommandAuthorizationService,
|
||||||
CommandExecutorService,
|
CommandExecutorService,
|
||||||
],
|
],
|
||||||
exports: [CommandRegistryService, CommandExecutorService],
|
exports: [CommandRegistryService, CommandExecutorService],
|
||||||
|
|||||||
27
docs/scratchpads/tess-m1-sec-001.md
Normal file
27
docs/scratchpads/tess-m1-sec-001.md
Normal file
@@ -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.
|
||||||
Reference in New Issue
Block a user