Compare commits
7
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
ce765fa70e | ||
|
|
9b6869fab7 | ||
|
|
20ad89c86b | ||
|
|
a55d1a1812 | ||
|
|
6dc35e5bf2 | ||
|
|
c7ee3cb5ce | ||
|
|
a972249a2b |
+13
-5
@@ -21,11 +21,19 @@ variables:
|
|||||||
- &enable_pnpm 'corepack enable'
|
- &enable_pnpm 'corepack enable'
|
||||||
|
|
||||||
when:
|
when:
|
||||||
# PR + manual CI run on any branch — the pull_request pipeline is the merge gate.
|
# PR + manual CI run on any branch: the pull_request pipeline is the merge
|
||||||
# push CI is restricted to protected branches (main) so a feature-branch push no
|
# gate (next is protected and the default branch since 2026-08-19).
|
||||||
# longer fires a redundant SECOND pipeline alongside its PR pipeline. This ~halves
|
# Push CI runs on main only. next deliberately runs NO push ci: post-merge
|
||||||
# CI load on the storage-constrained runner with zero loss of gating (branch
|
# verification on next is carried by publish.yml's `verify` step
|
||||||
# protection requires no push/ci status context; main still gets full push CI).
|
# (pnpm verify:release), which mirrors this pipeline's complete mandatory
|
||||||
|
# set step-for-step, enforced by scripts/verify-release.test.mjs. PR CI
|
||||||
|
# tests the PR HEAD tree (refs/pull/N/head, measured 2026-08-19), not a
|
||||||
|
# merge ref, so if next advances before a merge the landed tree differs
|
||||||
|
# from the tested one; publish verify re-runs the full set on the landed
|
||||||
|
# tree (PGlite path). Measured 2026-08-19: the 21 most recent push events
|
||||||
|
# on next each ran exactly one pipeline (publish), zero ci.
|
||||||
|
# Keeping push ci off next also avoids a redundant second full-suite run
|
||||||
|
# per merge on the storage-constrained runner.
|
||||||
- event: [pull_request, manual]
|
- event: [pull_request, manual]
|
||||||
- event: push
|
- event: push
|
||||||
branch: main
|
branch: main
|
||||||
|
|||||||
@@ -0,0 +1,332 @@
|
|||||||
|
import { type Type } from '@nestjs/common';
|
||||||
|
import { Test, type TestingModule } from '@nestjs/testing';
|
||||||
|
import type { SlashCommandPayload } from '@mosaicstack/types';
|
||||||
|
import { describe, expect, it, vi } from 'vitest';
|
||||||
|
import { AgentService, type AgentSession } from '../agent/agent.service.js';
|
||||||
|
import { ProviderService } from '../agent/provider.service.js';
|
||||||
|
import { AppModule } from '../app.module.js';
|
||||||
|
import { CommandAuthorizationService } from '../commands/command-authorization.service.js';
|
||||||
|
import { CommandExecutorService } from '../commands/command-executor.service.js';
|
||||||
|
import { CommandsModule } from '../commands/commands.module.js';
|
||||||
|
import { CommandRuntimeApprovalVerifier } from '../commands/runtime-approval-verifier.js';
|
||||||
|
import { PreferencesModule } from '../preferences/preferences.module.js';
|
||||||
|
import { SystemOverrideService } from '../preferences/system-override.service.js';
|
||||||
|
|
||||||
|
const fakeDb = {
|
||||||
|
$client: { exec: async (): Promise<void> => {} },
|
||||||
|
execute: async (): Promise<{ rows: unknown[] }> => ({ rows: [] }),
|
||||||
|
select: () => ({
|
||||||
|
from: () => ({
|
||||||
|
where: async (): Promise<Array<{ count: number }>> => [{ count: 1 }],
|
||||||
|
}),
|
||||||
|
}),
|
||||||
|
insert: () => ({ values: async (): Promise<void> => {} }),
|
||||||
|
};
|
||||||
|
|
||||||
|
const fakeProviderService = {
|
||||||
|
onModuleInit: async (): Promise<void> => {},
|
||||||
|
onModuleDestroy: (): void => {},
|
||||||
|
getRegistry: () => ({ getAvailable: () => [], getAll: () => [], find: () => undefined }),
|
||||||
|
getDefaultModel: () => undefined,
|
||||||
|
listAvailableModels: () => [],
|
||||||
|
listProviders: () => [],
|
||||||
|
getAdapter: () => undefined,
|
||||||
|
getProvidersHealth: () => [],
|
||||||
|
};
|
||||||
|
|
||||||
|
function compileRealAppGraph(): Promise<TestingModule> {
|
||||||
|
return Test.createTestingModule({ imports: [AppModule] })
|
||||||
|
.overrideProvider('DB_HANDLE')
|
||||||
|
.useValue({ db: fakeDb, close: async (): Promise<void> => {} })
|
||||||
|
.overrideProvider('DB')
|
||||||
|
.useValue(fakeDb)
|
||||||
|
.overrideProvider('STORAGE_ADAPTER')
|
||||||
|
.useValue({
|
||||||
|
name: 'required-security-wiring-test',
|
||||||
|
migrate: async (): Promise<void> => {},
|
||||||
|
close: async (): Promise<void> => {},
|
||||||
|
})
|
||||||
|
.overrideProvider('AUTH')
|
||||||
|
.useValue({})
|
||||||
|
.overrideProvider('BRAIN')
|
||||||
|
.useValue({ conversations: {}, agents: {} })
|
||||||
|
.overrideProvider('LOG_SERVICE')
|
||||||
|
.useValue({})
|
||||||
|
.overrideProvider('MEMORY')
|
||||||
|
.useValue({})
|
||||||
|
.overrideProvider('MEMORY_ADAPTER')
|
||||||
|
.useValue({})
|
||||||
|
.overrideProvider(ProviderService)
|
||||||
|
.useValue(fakeProviderService)
|
||||||
|
.compile();
|
||||||
|
}
|
||||||
|
|
||||||
|
function providerToken(provider: unknown): unknown {
|
||||||
|
return typeof provider === 'function' ? provider : (provider as { provide?: unknown })?.provide;
|
||||||
|
}
|
||||||
|
|
||||||
|
interface MaskingConsumer {
|
||||||
|
moduleType: Type<unknown>;
|
||||||
|
token: Type<unknown>;
|
||||||
|
useValue: object;
|
||||||
|
}
|
||||||
|
|
||||||
|
async function compileWithoutProvider(
|
||||||
|
moduleType: Type<unknown>,
|
||||||
|
missingToken: Type<unknown>,
|
||||||
|
maskingConsumer: MaskingConsumer,
|
||||||
|
): Promise<{ error: unknown; moduleRef: TestingModule | undefined }> {
|
||||||
|
const touchedModules = new Set([moduleType, maskingConsumer.moduleType]);
|
||||||
|
const originals = Array.from(touchedModules, (touchedModule: Type<unknown>) => ({
|
||||||
|
moduleType: touchedModule,
|
||||||
|
providers: (Reflect.getMetadata('providers', touchedModule) ?? []) as unknown[],
|
||||||
|
exports: (Reflect.getMetadata('exports', touchedModule) ?? []) as unknown[],
|
||||||
|
}));
|
||||||
|
|
||||||
|
for (const original of originals) {
|
||||||
|
const providers = original.providers.flatMap((provider: unknown): unknown[] => {
|
||||||
|
const token = providerToken(provider);
|
||||||
|
if (original.moduleType === moduleType && token === missingToken) return [];
|
||||||
|
if (original.moduleType === maskingConsumer.moduleType && token === maskingConsumer.token) {
|
||||||
|
return [{ provide: maskingConsumer.token, useValue: maskingConsumer.useValue }];
|
||||||
|
}
|
||||||
|
return [provider];
|
||||||
|
});
|
||||||
|
const exports = original.exports.filter(
|
||||||
|
(exported: unknown): boolean =>
|
||||||
|
original.moduleType !== moduleType || providerToken(exported) !== missingToken,
|
||||||
|
);
|
||||||
|
Reflect.defineMetadata('providers', providers, original.moduleType);
|
||||||
|
Reflect.defineMetadata('exports', exports, original.moduleType);
|
||||||
|
}
|
||||||
|
|
||||||
|
let moduleRef: TestingModule | undefined;
|
||||||
|
let error: unknown;
|
||||||
|
try {
|
||||||
|
moduleRef = await compileRealAppGraph();
|
||||||
|
} catch (caught: unknown) {
|
||||||
|
error = caught;
|
||||||
|
} finally {
|
||||||
|
for (const original of originals) {
|
||||||
|
Reflect.defineMetadata('providers', original.providers, original.moduleType);
|
||||||
|
Reflect.defineMetadata('exports', original.exports, original.moduleType);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
return { error, moduleRef };
|
||||||
|
}
|
||||||
|
|
||||||
|
async function closeIfCompiled(moduleRef: TestingModule | undefined): Promise<void> {
|
||||||
|
if (moduleRef) await moduleRef.close();
|
||||||
|
}
|
||||||
|
|
||||||
|
describe('required security wiring — real AppModule startup refusal', () => {
|
||||||
|
it('FL-01 positive control: the real graph compiles when CommandAuthorizationService is bound', async () => {
|
||||||
|
const moduleRef = await compileRealAppGraph();
|
||||||
|
try {
|
||||||
|
expect(moduleRef.get(CommandAuthorizationService, { strict: false })).toBeInstanceOf(
|
||||||
|
CommandAuthorizationService,
|
||||||
|
);
|
||||||
|
} finally {
|
||||||
|
await moduleRef.close();
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|
||||||
|
it('FL-01 negative control: absence read as permission is refused at module compilation', async () => {
|
||||||
|
const { error, moduleRef } = await compileWithoutProvider(
|
||||||
|
CommandsModule,
|
||||||
|
CommandAuthorizationService,
|
||||||
|
{
|
||||||
|
moduleType: CommandsModule,
|
||||||
|
token: CommandRuntimeApprovalVerifier,
|
||||||
|
useValue: {},
|
||||||
|
},
|
||||||
|
);
|
||||||
|
await closeIfCompiled(moduleRef);
|
||||||
|
|
||||||
|
expect(
|
||||||
|
error,
|
||||||
|
'absence read as permission: AppModule compilation accepted a missing CommandAuthorizationService binding',
|
||||||
|
).toBeInstanceOf(Error);
|
||||||
|
if (!(error instanceof Error)) return;
|
||||||
|
expect(error.message).toContain('CommandExecutorService');
|
||||||
|
expect(error.message).toContain('CommandAuthorizationService');
|
||||||
|
});
|
||||||
|
|
||||||
|
it('FL-11 positive control: the real graph compiles when SystemOverrideService is bound', async () => {
|
||||||
|
const moduleRef = await compileRealAppGraph();
|
||||||
|
try {
|
||||||
|
expect(moduleRef.get(SystemOverrideService, { strict: false })).toBeInstanceOf(
|
||||||
|
SystemOverrideService,
|
||||||
|
);
|
||||||
|
} finally {
|
||||||
|
await moduleRef.close();
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|
||||||
|
it('FL-11 negative control: absence read as permission is refused at module compilation', async () => {
|
||||||
|
const { error, moduleRef } = await compileWithoutProvider(
|
||||||
|
PreferencesModule,
|
||||||
|
SystemOverrideService,
|
||||||
|
{
|
||||||
|
moduleType: CommandsModule,
|
||||||
|
token: CommandExecutorService,
|
||||||
|
useValue: {},
|
||||||
|
},
|
||||||
|
);
|
||||||
|
await closeIfCompiled(moduleRef);
|
||||||
|
|
||||||
|
expect(
|
||||||
|
error,
|
||||||
|
'absence read as permission: AppModule compilation accepted a missing SystemOverrideService binding',
|
||||||
|
).toBeInstanceOf(Error);
|
||||||
|
if (!(error instanceof Error)) return;
|
||||||
|
expect(error.message).toContain('AgentService');
|
||||||
|
expect(error.message).toContain('SystemOverrideService');
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
const actorScope = { userId: 'security-user', tenantId: 'security-tenant' };
|
||||||
|
const conversationId = 'security-conversation';
|
||||||
|
|
||||||
|
function directExecutorWithoutAuthorization(systemOverrideSet: ReturnType<typeof vi.fn>) {
|
||||||
|
const registry = {
|
||||||
|
getManifest: vi.fn(() => ({
|
||||||
|
version: 1,
|
||||||
|
commands: [
|
||||||
|
{
|
||||||
|
name: 'system',
|
||||||
|
aliases: [],
|
||||||
|
description: 'Set instruction authority',
|
||||||
|
scope: 'agent' as const,
|
||||||
|
execution: 'socket' as const,
|
||||||
|
available: true,
|
||||||
|
},
|
||||||
|
],
|
||||||
|
skills: [],
|
||||||
|
})),
|
||||||
|
};
|
||||||
|
return new CommandExecutorService(
|
||||||
|
registry as never,
|
||||||
|
{ getSession: vi.fn() } as never,
|
||||||
|
{ set: systemOverrideSet, clear: vi.fn() } as never,
|
||||||
|
{ collect: vi.fn() } as never,
|
||||||
|
null,
|
||||||
|
{ agents: {} } as never,
|
||||||
|
null,
|
||||||
|
null,
|
||||||
|
{ getServerStatuses: vi.fn(() => []), getToolDefinitions: vi.fn(() => []) } as never,
|
||||||
|
undefined as never,
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
function directAgentWithoutSystemOverride(piPrompt: ReturnType<typeof vi.fn>): {
|
||||||
|
service: AgentService;
|
||||||
|
session: AgentSession;
|
||||||
|
} {
|
||||||
|
const service = new AgentService(
|
||||||
|
{
|
||||||
|
getDefaultModel: vi.fn(() => null),
|
||||||
|
getRegistry: vi.fn(() => ({})),
|
||||||
|
findModel: vi.fn(),
|
||||||
|
listAvailableModels: vi.fn(() => []),
|
||||||
|
} as never,
|
||||||
|
{} as never,
|
||||||
|
{} as never,
|
||||||
|
{ available: false } as never,
|
||||||
|
{} as never,
|
||||||
|
{ getToolDefinitions: vi.fn(() => []) } as never,
|
||||||
|
{ loadForSession: vi.fn(async () => ({ metaTools: [], promptAdditions: [] })) } as never,
|
||||||
|
undefined as never,
|
||||||
|
null,
|
||||||
|
{ collect: vi.fn().mockResolvedValue(undefined) } as never,
|
||||||
|
null,
|
||||||
|
);
|
||||||
|
const session = {
|
||||||
|
id: conversationId,
|
||||||
|
provider: 'test-provider',
|
||||||
|
modelId: 'test-model',
|
||||||
|
piSession: { prompt: piPrompt },
|
||||||
|
listeners: new Set(),
|
||||||
|
unsubscribe: vi.fn(),
|
||||||
|
createdAt: Date.now(),
|
||||||
|
promptCount: 0,
|
||||||
|
channels: new Set(),
|
||||||
|
skillPromptAdditions: [],
|
||||||
|
sandboxDir: process.cwd(),
|
||||||
|
allowedTools: null,
|
||||||
|
userId: actorScope.userId,
|
||||||
|
tenantId: actorScope.tenantId,
|
||||||
|
metrics: {
|
||||||
|
tokens: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0, total: 0 },
|
||||||
|
modelSwitches: 0,
|
||||||
|
messageCount: 0,
|
||||||
|
lastActivityAt: new Date(0).toISOString(),
|
||||||
|
},
|
||||||
|
} as unknown as AgentSession;
|
||||||
|
const internals = service as unknown as { sessions: Map<string, AgentSession> };
|
||||||
|
internals.sessions.set(conversationId, session);
|
||||||
|
return { service, session };
|
||||||
|
}
|
||||||
|
|
||||||
|
describe('required security wiring — malformed direct absence has zero effects', () => {
|
||||||
|
it('FL-01 refuses command execution before any command effect when authorization is absent', async () => {
|
||||||
|
const systemOverrideSet = vi.fn().mockResolvedValue(undefined);
|
||||||
|
const executor = directExecutorWithoutAuthorization(systemOverrideSet);
|
||||||
|
const payload: SlashCommandPayload = {
|
||||||
|
command: 'system',
|
||||||
|
args: 'authority that must not be stored',
|
||||||
|
conversationId,
|
||||||
|
};
|
||||||
|
let error: unknown;
|
||||||
|
|
||||||
|
try {
|
||||||
|
await executor.execute(payload, actorScope);
|
||||||
|
} catch (caught: unknown) {
|
||||||
|
error = caught;
|
||||||
|
}
|
||||||
|
|
||||||
|
expect
|
||||||
|
.soft(
|
||||||
|
error,
|
||||||
|
'absence read as permission: direct executor accepted missing command authorization',
|
||||||
|
)
|
||||||
|
.toBeInstanceOf(Error);
|
||||||
|
expect
|
||||||
|
.soft(
|
||||||
|
systemOverrideSet,
|
||||||
|
'absence read as permission: command effect occurred without command authorization',
|
||||||
|
)
|
||||||
|
.not.toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
|
||||||
|
it('FL-11 refuses prompt execution before any provider or session effect when system override authority is absent', async () => {
|
||||||
|
const piPrompt = vi.fn().mockResolvedValue(undefined);
|
||||||
|
const { service, session } = directAgentWithoutSystemOverride(piPrompt);
|
||||||
|
let error: unknown;
|
||||||
|
|
||||||
|
try {
|
||||||
|
await service.prompt(conversationId, 'must not reach provider', actorScope);
|
||||||
|
} catch (caught: unknown) {
|
||||||
|
error = caught;
|
||||||
|
}
|
||||||
|
|
||||||
|
expect
|
||||||
|
.soft(
|
||||||
|
error,
|
||||||
|
'absence read as permission: direct session accepted missing system override authority',
|
||||||
|
)
|
||||||
|
.toBeInstanceOf(Error);
|
||||||
|
expect
|
||||||
|
.soft(
|
||||||
|
piPrompt,
|
||||||
|
'absence read as permission: provider prompt occurred without system override authority',
|
||||||
|
)
|
||||||
|
.not.toHaveBeenCalled();
|
||||||
|
expect
|
||||||
|
.soft(
|
||||||
|
session.promptCount,
|
||||||
|
'absence read as permission: session state changed without system override authority',
|
||||||
|
)
|
||||||
|
.toBe(0);
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -26,7 +26,7 @@ function makeService(operatorMemory: unknown = null): AgentService {
|
|||||||
{} as never,
|
{} as never,
|
||||||
{ getToolDefinitions: vi.fn(() => []) } as never,
|
{ getToolDefinitions: vi.fn(() => []) } as never,
|
||||||
{ loadForSession: vi.fn(async () => ({ metaTools: [], promptAdditions: [] })) } as never,
|
{ loadForSession: vi.fn(async () => ({ metaTools: [], promptAdditions: [] })) } as never,
|
||||||
null,
|
{ get: vi.fn().mockResolvedValue(null), renew: vi.fn().mockResolvedValue(undefined) } as never,
|
||||||
null,
|
null,
|
||||||
{ collect: vi.fn().mockResolvedValue(undefined) } as never,
|
{ collect: vi.fn().mockResolvedValue(undefined) } as never,
|
||||||
operatorMemory as never,
|
operatorMemory as never,
|
||||||
|
|||||||
@@ -132,9 +132,8 @@ export class AgentService implements OnModuleDestroy {
|
|||||||
@Inject(CoordService) private readonly coordService: CoordService,
|
@Inject(CoordService) private readonly coordService: CoordService,
|
||||||
@Inject(McpClientService) private readonly mcpClientService: McpClientService,
|
@Inject(McpClientService) private readonly mcpClientService: McpClientService,
|
||||||
@Inject(SkillLoaderService) private readonly skillLoaderService: SkillLoaderService,
|
@Inject(SkillLoaderService) private readonly skillLoaderService: SkillLoaderService,
|
||||||
@Optional()
|
|
||||||
@Inject(SystemOverrideService)
|
@Inject(SystemOverrideService)
|
||||||
private readonly systemOverride: SystemOverrideService | null,
|
private readonly systemOverride: SystemOverrideService,
|
||||||
@Optional()
|
@Optional()
|
||||||
@Inject(PreferencesService)
|
@Inject(PreferencesService)
|
||||||
private readonly preferencesService: PreferencesService | null,
|
private readonly preferencesService: PreferencesService | null,
|
||||||
@@ -709,23 +708,22 @@ export class AgentService implements OnModuleDestroy {
|
|||||||
throw new Error(`No agent session found: ${sessionId}`);
|
throw new Error(`No agent session found: ${sessionId}`);
|
||||||
}
|
}
|
||||||
this.assertSessionScope(session, scope);
|
this.assertSessionScope(session, scope);
|
||||||
session.promptCount += 1;
|
|
||||||
|
|
||||||
// Channel attachments are untrusted URI references. Preserve exact,
|
// Channel attachments are untrusted URI references. Preserve exact,
|
||||||
// authenticated metadata for the agent without treating it as authority.
|
// authenticated metadata for the agent without treating it as authority.
|
||||||
const attachmentContext = this.attachmentContext(attachments);
|
const attachmentContext = this.attachmentContext(attachments);
|
||||||
|
|
||||||
// Prepend session-scoped system override if present (renew TTL on each turn)
|
// Prepend session-scoped system override if present (renew TTL on each turn).
|
||||||
|
// Required instruction-authority wiring is consulted before session/provider effects.
|
||||||
let effectiveMessage = `${message}${attachmentContext}`;
|
let effectiveMessage = `${message}${attachmentContext}`;
|
||||||
if (this.systemOverride) {
|
|
||||||
const override = await this.systemOverride.get(sessionId, scope);
|
const override = await this.systemOverride.get(sessionId, scope);
|
||||||
if (override) {
|
if (override) {
|
||||||
effectiveMessage = `[System Override]\n${override}\n\n${effectiveMessage}`;
|
effectiveMessage = `[System Override]\n${override}\n\n${effectiveMessage}`;
|
||||||
await this.systemOverride.renew(sessionId, scope);
|
await this.systemOverride.renew(sessionId, scope);
|
||||||
this.logger.debug(`Applied system override for session ${sessionId}`);
|
this.logger.debug(`Applied system override for session ${sessionId}`);
|
||||||
}
|
}
|
||||||
}
|
|
||||||
|
|
||||||
|
session.promptCount += 1;
|
||||||
try {
|
try {
|
||||||
await session.piSession.prompt(effectiveMessage);
|
await session.piSession.prompt(effectiveMessage);
|
||||||
} catch (err) {
|
} catch (err) {
|
||||||
|
|||||||
@@ -80,6 +80,10 @@ const mockMcpClient = {
|
|||||||
getToolDefinitions: vi.fn(() => []),
|
getToolDefinitions: vi.fn(() => []),
|
||||||
};
|
};
|
||||||
|
|
||||||
|
const allowAuthorization = {
|
||||||
|
authorize: vi.fn().mockResolvedValue({ allowed: true }),
|
||||||
|
};
|
||||||
|
|
||||||
function buildService(
|
function buildService(
|
||||||
redis: typeof mockRedis | null = mockRedis,
|
redis: typeof mockRedis | null = mockRedis,
|
||||||
mcpClient: {
|
mcpClient: {
|
||||||
@@ -98,6 +102,7 @@ function buildService(
|
|||||||
null,
|
null,
|
||||||
mockChatGateway as never,
|
mockChatGateway as never,
|
||||||
mcpClient as never,
|
mcpClient as never,
|
||||||
|
allowAuthorization as never,
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -35,9 +35,8 @@ export class CommandExecutorService {
|
|||||||
@Inject(forwardRef(() => ChatGateway))
|
@Inject(forwardRef(() => ChatGateway))
|
||||||
private readonly chatGateway: ChatGateway | null,
|
private readonly chatGateway: ChatGateway | null,
|
||||||
@Inject(McpClientService) private readonly mcpClient: McpClientService,
|
@Inject(McpClientService) private readonly mcpClient: McpClientService,
|
||||||
@Optional()
|
|
||||||
@Inject(CommandAuthorizationService)
|
@Inject(CommandAuthorizationService)
|
||||||
private readonly authorization: CommandAuthorizationService | null = null,
|
private readonly authorization: CommandAuthorizationService,
|
||||||
) {}
|
) {}
|
||||||
|
|
||||||
async execute(
|
async execute(
|
||||||
@@ -57,13 +56,13 @@ export class CommandExecutorService {
|
|||||||
};
|
};
|
||||||
}
|
}
|
||||||
|
|
||||||
const authorization = await this.authorization?.authorize(
|
const authorization = await this.authorization.authorize(
|
||||||
def,
|
def,
|
||||||
payload,
|
payload,
|
||||||
userId,
|
userId,
|
||||||
payload.approvalId,
|
payload.approvalId,
|
||||||
);
|
);
|
||||||
if (authorization && !authorization.allowed) {
|
if (!authorization.allowed) {
|
||||||
return { command, conversationId, success: false, message: authorization.reason };
|
return { command, conversationId, success: false, message: authorization.reason };
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -171,7 +170,7 @@ export class CommandExecutorService {
|
|||||||
const def = this.registry
|
const def = this.registry
|
||||||
.getManifest()
|
.getManifest()
|
||||||
.commands.find((command) => command.name === payload.command);
|
.commands.find((command) => command.name === payload.command);
|
||||||
if (!def || !this.authorization) return null;
|
if (!def) return null;
|
||||||
return this.authorization.createApproval(def, payload, scope.userId);
|
return this.authorization.createApproval(def, payload, scope.userId);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -55,6 +55,10 @@ const mockMcpClient = {
|
|||||||
reconnectServer: vi.fn().mockResolvedValue(undefined),
|
reconnectServer: vi.fn().mockResolvedValue(undefined),
|
||||||
};
|
};
|
||||||
|
|
||||||
|
const allowAuthorization = {
|
||||||
|
authorize: vi.fn().mockResolvedValue({ allowed: true }),
|
||||||
|
};
|
||||||
|
|
||||||
// ─── Helpers ─────────────────────────────────────────────────────────────────
|
// ─── Helpers ─────────────────────────────────────────────────────────────────
|
||||||
|
|
||||||
function buildRegistry(): CommandRegistryService {
|
function buildRegistry(): CommandRegistryService {
|
||||||
@@ -74,6 +78,7 @@ function buildExecutor(registry: CommandRegistryService): CommandExecutorService
|
|||||||
null, // reloadService (optional)
|
null, // reloadService (optional)
|
||||||
null, // chatGateway (optional)
|
null, // chatGateway (optional)
|
||||||
mockMcpClient as never,
|
mockMcpClient as never,
|
||||||
|
allowAuthorization as never,
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -159,6 +159,7 @@ describe('ReloadService — /reload command sanitizes plugin errors', () => {
|
|||||||
reloadService,
|
reloadService,
|
||||||
mockChatGateway as never,
|
mockChatGateway as never,
|
||||||
mockMcpClient as never,
|
mockMcpClient as never,
|
||||||
|
{ authorize: vi.fn().mockResolvedValue({ allowed: true }) } as never,
|
||||||
);
|
);
|
||||||
|
|
||||||
const payload: SlashCommandPayload = { command: 'reload', conversationId: 'conv-1' };
|
const payload: SlashCommandPayload = { command: 'reload', conversationId: 'conv-1' };
|
||||||
|
|||||||
@@ -0,0 +1,77 @@
|
|||||||
|
# #1179 — Required security DI wiring
|
||||||
|
|
||||||
|
## Objective
|
||||||
|
|
||||||
|
Eliminate the shared fail-open defect class **absence read as permission**:
|
||||||
|
|
||||||
|
- FL-01: missing `CommandAuthorizationService` must refuse Nest startup and must not permit command effects.
|
||||||
|
- FL-11: missing `SystemOverrideService` must refuse Nest startup and must not omit stored instruction authority while allowing provider/session effects.
|
||||||
|
|
||||||
|
## Tracking
|
||||||
|
|
||||||
|
- Issue: #1179, child of #1156
|
||||||
|
- Branch: `fix/1179-required-security-di`
|
||||||
|
- Base: `origin/next` at `216cd72226cd9ee17eea461cfe7cd0e010a22f02`
|
||||||
|
|
||||||
|
## Plan
|
||||||
|
|
||||||
|
1. RED: compile the real `AppModule` graph with each required provider independently removed, with a positive control for each intact binding.
|
||||||
|
2. RED: directly exercise each malformed absence path and assert zero command/provider/session effects.
|
||||||
|
3. Stop and report RED to the coordinator before production implementation.
|
||||||
|
4. After authorization, make both constructor injections required, remove absence-as-permission branches, and update explicit legitimate optional test seams.
|
||||||
|
5. Run focused Gateway tests, typecheck, lint, format, build, independent exact-head verification, and focused security review.
|
||||||
|
|
||||||
|
## Immutable path fence
|
||||||
|
|
||||||
|
Production changes are confined to:
|
||||||
|
|
||||||
|
- `apps/gateway/src/commands/command-executor.service.ts`
|
||||||
|
- `apps/gateway/src/agent/agent.service.ts`
|
||||||
|
|
||||||
|
Tests and task evidence are confined to:
|
||||||
|
|
||||||
|
- `apps/gateway/src/__tests__/required-security-wiring.test.ts`
|
||||||
|
- existing direct-constructor specs that require explicit required arguments
|
||||||
|
- `docs/scratchpads/1179-required-security-di.md`
|
||||||
|
|
||||||
|
No files in #1178, #1072, #1080, or #1054 lanes are in scope. `docs/TASKS.md` is orchestrator-owned and will not be modified.
|
||||||
|
|
||||||
|
## Budget
|
||||||
|
|
||||||
|
No explicit token ceiling was provided. Working assumption: one narrow Gateway security packet; split and stop if either arm requires unrelated module rewiring.
|
||||||
|
|
||||||
|
## Progress
|
||||||
|
|
||||||
|
- Intake read from #1179 and parent #1156.
|
||||||
|
- Base independently resolved from the issue's pre-native-stage ordering and repository `origin/next` ref; branch HEAD verified byte-for-byte against the remote ref.
|
||||||
|
- Real consumers and direct constructors inventoried.
|
||||||
|
|
||||||
|
## Tests
|
||||||
|
|
||||||
|
### RED
|
||||||
|
|
||||||
|
- `required-security-wiring.test.ts`: 4 failed, 2 passed before implementation.
|
||||||
|
- Both real-graph negative controls showed module compilation accepted the missing target binding.
|
||||||
|
- Direct FL-01 showed one unauthorized command effect; direct FL-11 showed one provider prompt and one session counter mutation.
|
||||||
|
|
||||||
|
### GREEN
|
||||||
|
|
||||||
|
- `required-security-wiring.test.ts`: 6/6 passed.
|
||||||
|
- FL-01-only production revert: exactly the two FL-01 test cases failed; all four other cases, including FL-11, passed.
|
||||||
|
- FL-11-only production revert: exactly the two FL-11 test cases failed; all four other cases, including FL-01, passed.
|
||||||
|
- Full Gateway suite: 74 files passed, 7 skipped; 831 tests passed, 17 skipped.
|
||||||
|
- Gateway typecheck: passed.
|
||||||
|
- Gateway lint: passed.
|
||||||
|
- Gateway build: passed.
|
||||||
|
- Changed-file Prettier check: passed.
|
||||||
|
|
||||||
|
### Review
|
||||||
|
|
||||||
|
- Codex code review: APPROVE, 0 findings.
|
||||||
|
- Codex focused security review: risk `none`, 0 findings.
|
||||||
|
- Independent exact-head review remains assigned to Scrappy through the coordinator.
|
||||||
|
|
||||||
|
## Risks / blockers
|
||||||
|
|
||||||
|
- `AgentModule` / `CommandsModule` / `ChatModule` contain a production cycle; the module test therefore uses the real top-level `AppModule` and replaces only storage/network leaves, preserving the target service in each arm while isolating the separate required consumer that would otherwise mask that arm's defect.
|
||||||
|
- No broad module rewrite was required.
|
||||||
@@ -292,6 +292,16 @@ esac
|
|||||||
_build_runtime_bin_prefix() {
|
_build_runtime_bin_prefix() {
|
||||||
local candidates=()
|
local candidates=()
|
||||||
if [ -n "$MOSAIC_RUNTIME_BIN" ]; then candidates+=("$MOSAIC_RUNTIME_BIN"); fi
|
if [ -n "$MOSAIC_RUNTIME_BIN" ]; then candidates+=("$MOSAIC_RUNTIME_BIN"); fi
|
||||||
|
# A host with no system Node gets one bootstrapped here by tools/install.sh, which
|
||||||
|
# records it in ~/.profile. The fleet unit runs `env -i ... bash --noprofile --norc`
|
||||||
|
# by design, so ~/.profile is never read and the directory has to be named here.
|
||||||
|
# The npm probe below cannot cover this: it reports a package prefix
|
||||||
|
# (~/.npm-global), never a Node runtime directory. It sits ahead of the npm probe so
|
||||||
|
# the bootstrapped runtime wins on a host that has both — that is the one the installer
|
||||||
|
# verified — while an explicit MOSAIC_RUNTIME_BIN still outranks it.
|
||||||
|
# Runtime binaries are `#!/usr/bin/env node`, so without this the pane resolves the
|
||||||
|
# binary and then dies on `env: 'node': No such file or directory`.
|
||||||
|
candidates+=("$PANE_HOME/.mosaic/node/current/bin")
|
||||||
if command -v npm >/dev/null 2>&1; then
|
if command -v npm >/dev/null 2>&1; then
|
||||||
local npm_prefix
|
local npm_prefix
|
||||||
npm_prefix=$(npm config get prefix 2>/dev/null) || true
|
npm_prefix=$(npm config get prefix 2>/dev/null) || true
|
||||||
|
|||||||
@@ -520,6 +520,93 @@ for blocked in LD_PRELOAD= BASH_ENV= MOSAIC_UNTRUSTED_SENTINEL=; do
|
|||||||
contains_literal "$pane_environment" "$blocked" && fail "runtime pane received $blocked"
|
contains_literal "$pane_environment" "$blocked" && fail "runtime pane received $blocked"
|
||||||
done
|
done
|
||||||
|
|
||||||
|
# #1256. On a host with no system Node, tools/install.sh bootstraps one into
|
||||||
|
# ~/.mosaic/node/ and writes that directory to ~/.profile. The fleet unit runs
|
||||||
|
# `env -i ... bash --noprofile --norc`, so ~/.profile is never read — correctly, by
|
||||||
|
# design — and _build_runtime_bin_prefix does not list the bootstrap directory. Its
|
||||||
|
# `npm config get prefix` branch cannot cover the gap either: the installer points
|
||||||
|
# npm's prefix at ~/.npm-global, so that branch contributes the npm-global directory
|
||||||
|
# and never the Node one, however it resolves.
|
||||||
|
#
|
||||||
|
# The property under test is not "the string is in PATH". It is that the pane can
|
||||||
|
# EXECUTE a Node-shebang runtime binary — which is what `mosaic` is
|
||||||
|
# (`#!/usr/bin/env node`) and what actually failed: measured on a greenfield VM as
|
||||||
|
# `env: 'node': No such file or directory` after a clean install that reported success.
|
||||||
|
#
|
||||||
|
# So this case runs the pane for real and requires it to have run. A PATH-substring
|
||||||
|
# assertion would pass on a fix that put the directory in the wrong position, and it
|
||||||
|
# would keep passing if the pane later stopped running for some unrelated reason.
|
||||||
|
: > "$TMUX_CALLS"
|
||||||
|
HOME_NODE="$ROOT/bootstrap-node/.config/mosaic"
|
||||||
|
write_generated "$HOME_NODE" "coder-node"
|
||||||
|
NODE_PANE_HOME="${HOME_NODE%/.config/mosaic}"
|
||||||
|
NODE_BOOTSTRAP_BIN="$NODE_PANE_HOME/.mosaic/node/current/bin"
|
||||||
|
mkdir -p "$NODE_BOOTSTRAP_BIN"
|
||||||
|
|
||||||
|
# The bootstrapped runtime. It records that it ran, which is the evidence this case
|
||||||
|
# turns on: no node reachable from the pane means no marker.
|
||||||
|
cat > "$NODE_BOOTSTRAP_BIN/node" <<'SHIM'
|
||||||
|
#!/usr/bin/env bash
|
||||||
|
set -euo pipefail
|
||||||
|
env -0 > "${MOSAIC_HOME:?}/fleet/pane-environment"
|
||||||
|
SHIM
|
||||||
|
chmod +x "$NODE_BOOTSTRAP_BIN/node"
|
||||||
|
|
||||||
|
# write_generated plants its symlinks under the MOSAIC_HOME it is given; here the
|
||||||
|
# pane's HOME is the trusted parent, so the pane's view of "installed" is this
|
||||||
|
# directory instead. `pi` is what #1241 resolves against PANE_PATH; `mosaic` is what
|
||||||
|
# the pane then executes, and it is a Node script — not a bash script that would run
|
||||||
|
# anywhere and quietly hide the defect.
|
||||||
|
mkdir -p "$NODE_PANE_HOME/.npm-global/bin"
|
||||||
|
ln -sf "$FAKE_BIN/pi" "$NODE_PANE_HOME/.npm-global/bin/pi"
|
||||||
|
printf '#!/usr/bin/env node\n' > "$NODE_PANE_HOME/.npm-global/bin/mosaic"
|
||||||
|
chmod +x "$NODE_PANE_HOME/.npm-global/bin/mosaic"
|
||||||
|
|
||||||
|
# The npm branch is modelled ALIVE and still cannot close the gap, which is the
|
||||||
|
# stronger statement. An earlier draft of this case tried to model npm as absent —
|
||||||
|
# true on a real bootstrap host, where npm lives only in the Node directory — and it
|
||||||
|
# refused to run anywhere npm is in the system path, i.e. most machines. It was also
|
||||||
|
# the weaker claim: it would have proven only that a dead branch supplies nothing.
|
||||||
|
#
|
||||||
|
# On a bootstrap host the installer sets npm's prefix to ~/.npm-global. So even with
|
||||||
|
# `command -v npm` true and the branch executing, `npm config get prefix` yields the
|
||||||
|
# npm-global directory and never the Node one. The gap does not depend on whether
|
||||||
|
# that branch runs.
|
||||||
|
NODE_LAUNCHER_BIN="$ROOT/bootstrap-node-launcher-bin"
|
||||||
|
mkdir -p "$NODE_LAUNCHER_BIN"
|
||||||
|
ln -sf "$FAKE_BIN/tmux" "$NODE_LAUNCHER_BIN/tmux"
|
||||||
|
ln -sf "$FAKE_BIN/npm" "$NODE_LAUNCHER_BIN/npm"
|
||||||
|
|
||||||
|
/usr/bin/env -i \
|
||||||
|
"HOME=$NODE_PANE_HOME" \
|
||||||
|
"PATH=$NODE_LAUNCHER_BIN:/usr/bin:/bin" \
|
||||||
|
"MOSAIC_HOME=$HOME_NODE" \
|
||||||
|
"MOSAIC_TEST_TMUX_CALLS=$TMUX_CALLS" \
|
||||||
|
"MOSAIC_TEST_HOME=$NODE_PANE_HOME" \
|
||||||
|
"MOSAIC_TEST_NPM_PREFIX=$NODE_PANE_HOME/.npm-global" \
|
||||||
|
MOSAIC_TEST_FLEET_OWNER=123e4567-e89b-12d3-a456-426614174000 \
|
||||||
|
MOSAIC_TEST_EXECUTE_PANE=1 \
|
||||||
|
"MOSAIC_TEST_PANE_PID=$$" \
|
||||||
|
"$START" coder-node
|
||||||
|
|
||||||
|
[ -f "$HOME_NODE/fleet/pane-environment" ] || \
|
||||||
|
fail "pane could not execute a Node-shebang runtime: $NODE_BOOTSTRAP_BIN is absent from PANE_PATH (#1256)"
|
||||||
|
node_pane_environment=$(tr '\0' '\n' < "$HOME_NODE/fleet/pane-environment")
|
||||||
|
# Colon-pad and match a whole element. A regex with `(^|:)` after `.*` looks like it
|
||||||
|
# does this and does not: an anchor cannot match mid-pattern, so it silently requires
|
||||||
|
# a leading colon and rejects the directory in FIRST position — which is where THIS
|
||||||
|
# FIXTURE puts it: it runs under `env -i` with no MOSAIC_RUNTIME_BIN, so the bootstrap
|
||||||
|
# directory leads. That is a property of the fixture, not of the fix — in general the
|
||||||
|
# directory sits second, after MOSAIC_RUNTIME_BIN. The colon padding makes the
|
||||||
|
# assertion position-independent either way, which is why it is written this way and
|
||||||
|
# not with an anchor. That produced a failure reading "pane ran but PANE_PATH does not
|
||||||
|
# carry <dir>" against a PATH whose first element was that dir.
|
||||||
|
node_pane_path=":$(printf '%s\n' "$node_pane_environment" | sed -n 's/^PATH=//p' | head -1):"
|
||||||
|
case "$node_pane_path" in
|
||||||
|
*":$NODE_BOOTSTRAP_BIN:"*) ;;
|
||||||
|
*) fail "pane ran but PANE_PATH does not carry $NODE_BOOTSTRAP_BIN (PATH=$node_pane_path)" ;;
|
||||||
|
esac
|
||||||
|
|
||||||
write_interaction_generated() {
|
write_interaction_generated() {
|
||||||
local home="$1"
|
local home="$1"
|
||||||
local agent="$2"
|
local agent="$2"
|
||||||
|
|||||||
Reference in New Issue
Block a user