diff --git a/backend/src/modules/learning/engine/engine.itest.ts b/backend/src/modules/learning/engine/engine.itest.ts index aa62ff6..5f3f336 100644 --- a/backend/src/modules/learning/engine/engine.itest.ts +++ b/backend/src/modules/learning/engine/engine.itest.ts @@ -83,7 +83,7 @@ describe('learning engine — real containers (V-4)', () => { const cache = await containerProvider.createContainer(projectId, 'cache', 'redis'); const nosql = await containerProvider.createContainer(projectId, 'nosql', 'mongo'); const lb = await containerProvider.createContainer(projectId, 'lb', 'loadbalancer'); - const asgBoundary = await containerProvider.createContainer(projectId, 'web-asg', 'ubuntu'); + const asgBoundary = await containerProvider.createContainer(projectId, 'web-asg', 'autoscalinggroup'); await containerProvider.createContainer(projectId, 'web-asg-replica-1', 'ubuntu', false, undefined, { 'akal.asg.id': asgBoundary.id, 'akal.asg.instance': 'true', @@ -110,6 +110,14 @@ describe('learning engine — real containers (V-4)', () => { "db.getSiblingDB('test').events.insertOne({ seed: true })" ); + // Realistic security groups: every node in a subnet carries the default SG + // (deny all inbound / allow all outbound — what the canvas gives every + // dropped node), learner rules prepended like the UI does. `cache` stages + // the DENY-shadow scenario: a DENY on 6379 sits above a broader ALLOW. + const defaultSgRules = () => [ + { id: 'sg-default-in', type: 'inbound', action: 'DENY', protocol: 'ALL', port: 'ALL', source: '0.0.0.0/0' }, + { id: 'sg-default-out', type: 'outbound', action: 'ALLOW', protocol: 'ALL', port: 'ALL', source: '0.0.0.0/0' }, + ]; await ProjectService.saveNetworkConfig(projectId, { nodeSubnetMap: { [web.id]: 'public', @@ -118,8 +126,16 @@ describe('learning engine — real containers (V-4)', () => { [nosql.id]: 'private', }, nodeSecurityGroups: { + [web.id]: defaultSgRules(), + [nosql.id]: defaultSgRules(), [db.id]: [ { id: 'sg-web-to-db', type: 'inbound', action: 'ALLOW', protocol: 'TCP', port: '5432', source: web.id }, + ...defaultSgRules(), + ], + [cache.id]: [ + { id: 'sg-web-deny-6379', type: 'inbound', action: 'DENY', protocol: 'TCP', port: '6379', source: web.id }, + { id: 'sg-web-allow-all', type: 'inbound', action: 'ALLOW', protocol: 'ALL', port: 'ALL', source: web.id }, + ...defaultSgRules(), ], }, loadBalancerTargets: { @@ -189,18 +205,34 @@ describe('learning engine — real containers (V-4)', () => { expect(result.status).toBe('pass'); }); - it('edge_exists fails for a pair with no security group rule', async () => { + it('edge_exists fails with only the default security group (no learner rule)', async () => { const result = await runOne('edge_exists', { source: 'web', target: 'nosql', port: 5432 }); expect(result.status).toBe('fail'); expect(result.expected).toBeTruthy(); expect(result.observed).toBeTruthy(); }); - it('port_denied passes for a port with no allow rule', async () => { + it('edge_exists fails when a DENY rule blocks the port despite a broader ALLOW below it', async () => { + const result = await runOne('edge_exists', { source: 'web', target: 'cache', port: 6379 }); + expect(result.status).toBe('fail'); + expect(result.observed).toBe('blocked by a DENY rule'); + }); + + it('edge_exists passes on a port covered by the broader ALLOW next to a specific DENY', async () => { + const result = await runOne('edge_exists', { source: 'web', target: 'cache', port: 80 }); + expect(result.status).toBe('pass'); + }); + + it('port_denied passes for a port with no allow rule (default security group)', async () => { const result = await runOne('port_denied', { source: 'web', target: 'db', port: 9999 }); expect(result.status).toBe('pass'); }); + it('port_denied passes for a port blocked by a DENY rule above a broader ALLOW', async () => { + const result = await runOne('port_denied', { source: 'web', target: 'cache', port: 6379 }); + expect(result.status).toBe('pass'); + }); + it('port_denied fails for a port that is explicitly allowed', async () => { const result = await runOne('port_denied', { source: 'web', target: 'db', port: 5432 }); expect(result.status).toBe('fail'); @@ -208,6 +240,12 @@ describe('learning engine — real containers (V-4)', () => { expect(result.observed).toBeTruthy(); }); + it('port_denied fails when a node sits outside any subnet (nothing is enforced)', async () => { + const result = await runOne('port_denied', { source: 'web', target: 'lb', port: 80 }); + expect(result.status).toBe('fail'); + expect(result.observed).toBe('"lb" is outside any subnet'); + }); + it('lb_upstreams passes when enough replicas are running', async () => { const result = await runOne('lb_upstreams', { node: 'lb', min: 2 }); expect(result.status).toBe('pass'); diff --git a/backend/src/modules/learning/engine/params.ts b/backend/src/modules/learning/engine/params.ts index 32cc58d..d0b5fd7 100644 --- a/backend/src/modules/learning/engine/params.ts +++ b/backend/src/modules/learning/engine/params.ts @@ -21,6 +21,17 @@ export function requireNumberParam(params: Record, name: string return value; } +export function requireNonNegativeIntegerParam( + params: Record, + name: string +): number { + const value = params[name]; + if (typeof value !== 'number' || !Number.isInteger(value) || value < 0) { + throw new InvalidParamsError(`validator param "${name}" must be a non-negative integer`); + } + return value; +} + export function optionalStringParam( params: Record, name: string, diff --git a/backend/src/modules/learning/engine/validators/asgReplicas.test.ts b/backend/src/modules/learning/engine/validators/asgReplicas.test.ts index c936255..02929fa 100644 --- a/backend/src/modules/learning/engine/validators/asgReplicas.test.ts +++ b/backend/src/modules/learning/engine/validators/asgReplicas.test.ts @@ -44,9 +44,28 @@ describe('asgReplicas', () => { expect(outcome.observed).toBe('no container with that name'); }); + it('fails when a same-named node is not an auto-scaling group', async () => { + const outcome = await asgReplicas( + { node: 'web-fleet', count: 0 }, + makeContext({ containers: [makeContainer({ id: 'redis-1', name: 'web-fleet', type: 'redis' })] }) + ); + + expect(outcome.status).toBe('fail'); + expect(outcome.message).toContain('not an auto-scaling group node'); + }); + it('throws InvalidParamsError when "count" is missing', async () => { await expect(asgReplicas({ node: 'web-fleet' }, makeContext())).rejects.toThrow( InvalidParamsError ); }); + + it('throws InvalidParamsError when "count" is negative or fractional', async () => { + await expect( + asgReplicas({ node: 'web-fleet', count: -1 }, makeContext({ containers: [asg] })) + ).rejects.toThrow(InvalidParamsError); + await expect( + asgReplicas({ node: 'web-fleet', count: 1.5 }, makeContext({ containers: [asg] })) + ).rejects.toThrow(InvalidParamsError); + }); }); diff --git a/backend/src/modules/learning/engine/validators/asgReplicas.ts b/backend/src/modules/learning/engine/validators/asgReplicas.ts index 17e9366..19879c9 100644 --- a/backend/src/modules/learning/engine/validators/asgReplicas.ts +++ b/backend/src/modules/learning/engine/validators/asgReplicas.ts @@ -1,6 +1,6 @@ import { ValidatorHandler } from '../types'; -import { requireStringParam, requireNumberParam } from '../params'; -import { countRunningAsgReplicas } from './shared'; +import { requireStringParam, requireNonNegativeIntegerParam } from '../params'; +import { countRunningAsgReplicas, resolveContainerOfType } from './shared'; /** * `asg_replicas` — checks that an auto-scaling group node runs exactly @@ -10,20 +10,13 @@ import { countRunningAsgReplicas } from './shared'; */ export const asgReplicas: ValidatorHandler = async (params, ctx) => { const node = requireStringParam(params, 'node'); - const count = requireNumberParam(params, 'count'); + const count = requireNonNegativeIntegerParam(params, 'count'); const containers = await ctx.getContainers(); - const asgContainer = containers.find((c) => c.name === node); - if (!asgContainer) { - return { - status: 'fail', - message: `No auto-scaling group named "${node}" exists in this project yet. Create it on the canvas first.`, - expected: `an auto-scaling group named "${node}"`, - observed: 'no container with that name', - }; - } + const resolved = resolveContainerOfType(containers, node, ['autoscalinggroup'], 'auto-scaling group'); + if ('outcome' in resolved) return resolved.outcome; - const runningReplicas = countRunningAsgReplicas(containers, asgContainer.id); + const runningReplicas = countRunningAsgReplicas(containers, resolved.container.id); if (runningReplicas !== count) { return { diff --git a/backend/src/modules/learning/engine/validators/edgeExists.test.ts b/backend/src/modules/learning/engine/validators/edgeExists.test.ts index 95f711b..4c08ce7 100644 --- a/backend/src/modules/learning/engine/validators/edgeExists.test.ts +++ b/backend/src/modules/learning/engine/validators/edgeExists.test.ts @@ -1,56 +1,113 @@ import { edgeExists } from './edgeExists'; import { InvalidParamsError } from '../types'; -import { makeContainer, makeContext, makeSemanticRule } from './testSupport'; +import { makeContainer, makeContext, makeNetworkConfig } from './testSupport'; + +// `computeSemanticRules` lists containers to resolve ASG replicas — no ASGs +// here. A plain async function, not a jest.fn(): `resetMocks` would wipe a +// mockResolvedValue between tests. +jest.mock('../../../../infrastructure/docker/DockerClient', () => ({ + __esModule: true, + default: { listContainers: async () => [] }, +})); const web = makeContainer({ id: 'web-1', name: 'web' }); const db = makeContainer({ id: 'db-1', name: 'db' }); +const containers = [web, db]; describe('edgeExists', () => { - it('passes when an ALLOW rule matches source and target on any port', async () => { + it('fails with only the default security groups (deny all inbound)', async () => { const outcome = await edgeExists( { source: 'web', target: 'db' }, + makeContext({ containers, networkConfig: makeNetworkConfig(['web-1', 'db-1']) }) + ); + + expect(outcome.status).toBe('fail'); + expect(outcome.observed).toBe('no matching rule'); + }); + + it('passes when the learner adds an ALLOW rule on the requested port', async () => { + const outcome = await edgeExists( + { source: 'web', target: 'db', port: 5432 }, makeContext({ - containers: [web, db], - semanticRules: [makeSemanticRule({ sourceNodeId: 'web-1', targetNodeId: 'db-1' })], + containers, + networkConfig: makeNetworkConfig(['web-1', 'db-1'], { + 'db-1': [{ type: 'inbound', action: 'ALLOW', protocol: 'TCP', port: '5432', source: 'web-1' }], + }), }) ); expect(outcome.status).toBe('pass'); }); - it('passes when the ALLOW rule matches the requested port', async () => { + it('passes on any port when the learner adds an ALLOW rule', async () => { const outcome = await edgeExists( - { source: 'web', target: 'db', port: 5432 }, + { source: 'web', target: 'db' }, makeContext({ - containers: [web, db], - semanticRules: [ - makeSemanticRule({ sourceNodeId: 'web-1', targetNodeId: 'db-1', port: '5432' }), - ], + containers, + networkConfig: makeNetworkConfig(['web-1', 'db-1'], { + 'db-1': [{ type: 'inbound', action: 'ALLOW', protocol: 'TCP', port: '5432', source: 'web-1' }], + }), }) ); expect(outcome.status).toBe('pass'); }); - it('fails when the ALLOW rule is for a different port', async () => { + it('fails when the ALLOW rule is for a different port (default DENY applies)', async () => { const outcome = await edgeExists( { source: 'web', target: 'db', port: 5432 }, makeContext({ - containers: [web, db], - semanticRules: [ - makeSemanticRule({ sourceNodeId: 'web-1', targetNodeId: 'db-1', port: '80' }), - ], + containers, + networkConfig: makeNetworkConfig(['web-1', 'db-1'], { + 'db-1': [{ type: 'inbound', action: 'ALLOW', protocol: 'TCP', port: '80', source: 'web-1' }], + }), }) ); expect(outcome.status).toBe('fail'); - expect(outcome.observed).toBe('no matching rule'); + expect(outcome.observed).toBe('blocked by a DENY rule'); }); - it('fails when no rule matches source and target at all', async () => { + it('fails when a DENY rule blocks the port despite a broader ALLOW below it', async () => { + const config = makeNetworkConfig(['web-1', 'db-1'], { + 'db-1': [ + { type: 'inbound', action: 'DENY', protocol: 'TCP', port: '5432', source: 'web-1' }, + { type: 'inbound', action: 'ALLOW', protocol: 'ALL', port: 'ALL', source: 'web-1' }, + ], + }); + + const denied = await edgeExists( + { source: 'web', target: 'db', port: 5432 }, + makeContext({ containers, networkConfig: config }) + ); + expect(denied.status).toBe('fail'); + expect(denied.observed).toBe('blocked by a DENY rule'); + + const otherPort = await edgeExists( + { source: 'web', target: 'db', port: 80 }, + makeContext({ containers, networkConfig: config }) + ); + expect(otherPort.status).toBe('pass'); + + const anyPort = await edgeExists( + { source: 'web', target: 'db' }, + makeContext({ containers, networkConfig: config }) + ); + expect(anyPort.status).toBe('pass'); + }); + + it('fails on any port when a DENY ALL rule shadows a later ALLOW', async () => { const outcome = await edgeExists( { source: 'web', target: 'db' }, - makeContext({ containers: [web, db], semanticRules: [] }) + makeContext({ + containers, + networkConfig: makeNetworkConfig(['web-1', 'db-1'], { + 'db-1': [ + { type: 'inbound', action: 'DENY', protocol: 'ALL', port: 'ALL', source: 'web-1' }, + { type: 'inbound', action: 'ALLOW', protocol: 'TCP', port: '5432', source: 'web-1' }, + ], + }), + }) ); expect(outcome.status).toBe('fail'); diff --git a/backend/src/modules/learning/engine/validators/edgeExists.ts b/backend/src/modules/learning/engine/validators/edgeExists.ts index 2acdd7b..db2b65b 100644 --- a/backend/src/modules/learning/engine/validators/edgeExists.ts +++ b/backend/src/modules/learning/engine/validators/edgeExists.ts @@ -1,6 +1,6 @@ import { ValidatorHandler } from '../types'; import { requireStringParam, optionalNumberParam } from '../params'; -import { resolveSourceAndTarget } from './shared'; +import { findMatchingInboundRule, hasEffectiveInboundAllow, resolveSourceAndTarget } from './shared'; /** * `edge_exists` — checks that an allowed connection exists from `source` to @@ -8,6 +8,8 @@ import { resolveSourceAndTarget } from './shared'; * target: string, port?: number }` (docs/roadmap-format.md). Connectivity is * about security-group configuration, not container liveness, so — unlike * `container_running` — a stopped node still counts as long as it exists. + * DENY rules are honored with the same first-match-wins semantics as the + * Network Simulator and the real enforcement (see `findMatchingInboundRule`). */ export const edgeExists: ValidatorHandler = async (params, ctx) => { const source = requireStringParam(params, 'source'); @@ -19,22 +21,25 @@ export const edgeExists: ValidatorHandler = async (params, ctx) => { if ('outcome' in resolved) return resolved.outcome; const rules = await ctx.getSemanticRules(); - const allowed = rules.some( - (rule) => - rule.action === 'ALLOW' && - rule.direction === 'inbound' && - rule.sourceNodeId === resolved.sourceContainer.id && - rule.targetNodeId === resolved.targetContainer.id && - (port === undefined || rule.port === 'ALL' || rule.port === String(port)) - ); + const sourceId = resolved.sourceContainer.id; + const targetId = resolved.targetContainer.id; + + const matched = port === undefined ? undefined : findMatchingInboundRule(rules, sourceId, targetId, port); + const allowed = + port === undefined + ? hasEffectiveInboundAllow(rules, sourceId, targetId) + : matched?.action === 'ALLOW'; const portLabel = port === undefined ? '' : ` on port ${port}`; if (!allowed) { + const deniedByRule = matched?.action === 'DENY'; return { status: 'fail', - message: `There is no allowed connection from "${source}" to "${target}"${portLabel} yet. Add a security group rule allowing it.`, + message: deniedByRule + ? `The connection from "${source}" to "${target}"${portLabel} is blocked by a DENY rule. Remove it or add an ALLOW rule above it.` + : `There is no allowed connection from "${source}" to "${target}"${portLabel} yet. Add a security group rule allowing it.`, expected: `an allowed connection from "${source}" to "${target}"${portLabel}`, - observed: 'no matching rule', + observed: deniedByRule ? 'blocked by a DENY rule' : 'no matching rule', }; } diff --git a/backend/src/modules/learning/engine/validators/portDenied.test.ts b/backend/src/modules/learning/engine/validators/portDenied.test.ts index 169460d..9c8a4b2 100644 --- a/backend/src/modules/learning/engine/validators/portDenied.test.ts +++ b/backend/src/modules/learning/engine/validators/portDenied.test.ts @@ -1,15 +1,44 @@ import { portDenied } from './portDenied'; import { InvalidParamsError } from '../types'; -import { makeContainer, makeContext, makeSemanticRule } from './testSupport'; +import { makeContainer, makeContext, makeNetworkConfig } from './testSupport'; + +// `computeSemanticRules` lists containers to resolve ASG replicas — no ASGs +// here. A plain async function, not a jest.fn(): `resetMocks` would wipe a +// mockResolvedValue between tests. +jest.mock('../../../../infrastructure/docker/DockerClient', () => ({ + __esModule: true, + default: { listContainers: async () => [] }, +})); const web = makeContainer({ id: 'web-1', name: 'web' }); const db = makeContainer({ id: 'db-1', name: 'db' }); +const containers = [web, db]; describe('portDenied', () => { - it('passes when no ALLOW rule covers the port', async () => { + it('fails when no network configuration is applied to the project', async () => { + const outcome = await portDenied( + { source: 'web', target: 'db', port: 5432 }, + makeContext({ containers, networkConfig: null }) + ); + + expect(outcome.status).toBe('fail'); + expect(outcome.observed).toBe('no network configuration applied'); + }); + + it('fails when one of the nodes is outside any subnet', async () => { + const outcome = await portDenied( + { source: 'web', target: 'db', port: 5432 }, + makeContext({ containers, networkConfig: makeNetworkConfig(['db-1']) }) + ); + + expect(outcome.status).toBe('fail'); + expect(outcome.observed).toBe('"web" is outside any subnet'); + }); + + it('passes with only the default security groups (deny all inbound)', async () => { const outcome = await portDenied( { source: 'web', target: 'db', port: 5432 }, - makeContext({ containers: [web, db], semanticRules: [] }) + makeContext({ containers, networkConfig: makeNetworkConfig(['web-1', 'db-1']) }) ); expect(outcome.status).toBe('pass'); @@ -19,10 +48,10 @@ describe('portDenied', () => { const outcome = await portDenied( { source: 'web', target: 'db', port: 5432 }, makeContext({ - containers: [web, db], - semanticRules: [ - makeSemanticRule({ sourceNodeId: 'web-1', targetNodeId: 'db-1', port: '5432' }), - ], + containers, + networkConfig: makeNetworkConfig(['web-1', 'db-1'], { + 'db-1': [{ type: 'inbound', action: 'ALLOW', protocol: 'TCP', port: '5432', source: 'web-1' }], + }), }) ); @@ -34,10 +63,10 @@ describe('portDenied', () => { const outcome = await portDenied( { source: 'web', target: 'db', port: 5432 }, makeContext({ - containers: [web, db], - semanticRules: [ - makeSemanticRule({ sourceNodeId: 'web-1', targetNodeId: 'db-1', port: 'ALL' }), - ], + containers, + networkConfig: makeNetworkConfig(['web-1', 'db-1'], { + 'db-1': [{ type: 'inbound', action: 'ALLOW', protocol: 'ALL', port: 'ALL', source: 'web-1' }], + }), }) ); @@ -45,14 +74,31 @@ describe('portDenied', () => { expect(outcome.observed).toContain('all ports are allowed'); }); - it('passes when an ALLOW rule exists for a different port', async () => { + it('passes when the ALLOW rule is for a different port', async () => { + const outcome = await portDenied( + { source: 'web', target: 'db', port: 5432 }, + makeContext({ + containers, + networkConfig: makeNetworkConfig(['web-1', 'db-1'], { + 'db-1': [{ type: 'inbound', action: 'ALLOW', protocol: 'TCP', port: '80', source: 'web-1' }], + }), + }) + ); + + expect(outcome.status).toBe('pass'); + }); + + it('passes when a DENY rule blocks the port despite a broader ALLOW below it', async () => { const outcome = await portDenied( { source: 'web', target: 'db', port: 5432 }, makeContext({ - containers: [web, db], - semanticRules: [ - makeSemanticRule({ sourceNodeId: 'web-1', targetNodeId: 'db-1', port: '80' }), - ], + containers, + networkConfig: makeNetworkConfig(['web-1', 'db-1'], { + 'db-1': [ + { type: 'inbound', action: 'DENY', protocol: 'TCP', port: '5432', source: 'web-1' }, + { type: 'inbound', action: 'ALLOW', protocol: 'ALL', port: 'ALL', source: 'web-1' }, + ], + }), }) ); diff --git a/backend/src/modules/learning/engine/validators/portDenied.ts b/backend/src/modules/learning/engine/validators/portDenied.ts index 029b8df..088ae38 100644 --- a/backend/src/modules/learning/engine/validators/portDenied.ts +++ b/backend/src/modules/learning/engine/validators/portDenied.ts @@ -1,14 +1,16 @@ import { ValidatorHandler } from '../types'; import { requireStringParam, requireNumberParam } from '../params'; -import { resolveSourceAndTarget } from './shared'; +import { findMatchingInboundRule, resolveSourceAndTarget } from './shared'; /** * `port_denied` — checks that traffic from `source` to `target` on `port` is * blocked. Params: `{ source: string, target: string, port: number }` * (docs/roadmap-format.md). Reads the same computed security-group rules as - * `edge_exists`: the port is "denied" when no `ALLOW` rule covers it, mirroring - * the zero-trust default the enforcement planner applies to real containers — - * no live connection attempt is made (that would be slow and flaky). + * `edge_exists`, first-match-wins (see `findMatchingInboundRule`) — no live + * connection attempt is made (that would be slow and flaky). The zero-trust + * baseline only exists once the network config is applied and both nodes sit + * in subnets: outside of that, containers talk freely on the shared bridge, + * so the check fails instead of blessing an unenforced topology. */ export const portDenied: ValidatorHandler = async (params, ctx) => { const source = requireStringParam(params, 'source'); @@ -19,15 +21,41 @@ export const portDenied: ValidatorHandler = async (params, ctx) => { const resolved = resolveSourceAndTarget(containers, source, target); if ('outcome' in resolved) return resolved.outcome; + const config = await ctx.getNetworkConfig(); + if (!config?.nodeSubnetMap || Object.keys(config.nodeSubnetMap).length === 0) { + return { + status: 'fail', + message: + `Port ${port} is not blocked from "${source}" to "${target}": no network configuration ` + + `is applied to this project yet, so all containers can talk freely. ` + + `Create a VPC and place your nodes in subnets first.`, + expected: `port ${port} blocked from "${source}" to "${target}"`, + observed: 'no network configuration applied', + }; + } + const outsideSubnet = [ + { name: source, id: resolved.sourceContainer.id }, + { name: target, id: resolved.targetContainer.id }, + ].find((node) => !config.nodeSubnetMap?.[node.id]); + if (outsideSubnet) { + return { + status: 'fail', + message: + `Port ${port} is not blocked from "${source}" to "${target}": "${outsideSubnet.name}" is not ` + + `inside a subnet, so no firewall applies to it. Place it in a subnet first.`, + expected: `port ${port} blocked from "${source}" to "${target}"`, + observed: `"${outsideSubnet.name}" is outside any subnet`, + }; + } + const rules = await ctx.getSemanticRules(); - const openRule = rules.find( - (rule) => - rule.action === 'ALLOW' && - rule.direction === 'inbound' && - rule.sourceNodeId === resolved.sourceContainer.id && - rule.targetNodeId === resolved.targetContainer.id && - (rule.port === 'ALL' || rule.port === String(port)) + const matched = findMatchingInboundRule( + rules, + resolved.sourceContainer.id, + resolved.targetContainer.id, + port ); + const openRule = matched?.action === 'ALLOW' ? matched : undefined; if (openRule) { return { diff --git a/backend/src/modules/learning/engine/validators/redisKeyExists.test.ts b/backend/src/modules/learning/engine/validators/redisKeyExists.test.ts index 8f78bc3..edf5225 100644 --- a/backend/src/modules/learning/engine/validators/redisKeyExists.test.ts +++ b/backend/src/modules/learning/engine/validators/redisKeyExists.test.ts @@ -40,6 +40,20 @@ describe('redisKeyExists', () => { expect(outcome.observed).toBe('database not ready yet'); }); + it('fails pedagogically on a redis-cli server error like LOADING', async () => { + const outcome = await redisKeyExists( + { node: 'cache', key: 'session:*' }, + makeContext({ + containers: [redisContainer], + executeRedisCommand: () => + Promise.resolve('(error) LOADING Redis is loading the dataset in memory\n'), + }) + ); + + expect(outcome.status).toBe('fail'); + expect(outcome.observed).toBe('database not ready yet'); + }); + it('fails when the node is not a Redis node', async () => { const outcome = await redisKeyExists( { node: 'cache', key: 'session:*' }, diff --git a/backend/src/modules/learning/engine/validators/redisKeyExists.ts b/backend/src/modules/learning/engine/validators/redisKeyExists.ts index 741bf2f..f04cbc8 100644 --- a/backend/src/modules/learning/engine/validators/redisKeyExists.ts +++ b/backend/src/modules/learning/engine/validators/redisKeyExists.ts @@ -16,7 +16,10 @@ export const redisKeyExists: ValidatorHandler = async (params, ctx) => { const output = await ctx.executeRedisCommand(resolved.container.id, ['KEYS', key]); - if (output.startsWith('ERROR')) { + // `ERROR` is the provider's not-ready sentinel; `(error ...)` is how + // redis-cli reports server errors (LOADING, NOAUTH, ...) on stdout — + // neither is ever a proof that the key exists. + if (output.startsWith('ERROR') || output.trim().startsWith('(error')) { return { status: 'fail', message: `Redis on "${node}" is still starting up. Wait a few seconds and try again.`, diff --git a/backend/src/modules/learning/engine/validators/shared.ts b/backend/src/modules/learning/engine/validators/shared.ts index 309c3ec..b0f9b2f 100644 --- a/backend/src/modules/learning/engine/validators/shared.ts +++ b/backend/src/modules/learning/engine/validators/shared.ts @@ -1,15 +1,20 @@ import { ContainerInfo } from '../../../../infrastructure/docker/providers/containerProvider'; +import { SemanticRule } from '../../../network/models/networkPolicy'; import { ValidatorOutcome } from '../types'; export type ResolvedContainer = { container: ContainerInfo } | { outcome: ValidatorOutcome }; +const an = (label: string): string => (/^[aeiou]/i.test(label) ? `an ${label}` : `a ${label}`); + /** * Resolves a canvas node name to its container, checking it is of one of the - * expected node types and currently running. Never throws: a missing, wrong - * type or stopped node is a pedagogical fail, not an infrastructure error — - * callers check `'outcome' in result` and return it as-is on failure. + * expected node types — without requiring it to be running (`asg_replicas` + * targets the ASG boundary container, which never needs to run itself). + * Never throws: a missing or wrong-type node is a pedagogical fail, not an + * infrastructure error — callers check `'outcome' in result` and return it + * as-is on failure. */ -export function resolveRunningContainer( +export function resolveContainerOfType( containers: ContainerInfo[], node: string, expectedTypes: string[], @@ -22,8 +27,8 @@ export function resolveRunningContainer( status: 'fail', message: `No container named "${node}" exists in this project yet. ` + - `Create the node on the canvas, name it "${node}" and start it.`, - expected: `a running ${expectedLabel} node named "${node}"`, + `Create the node on the canvas and name it "${node}".`, + expected: `${an(expectedLabel)} node named "${node}"`, observed: 'no container with that name', }, }; @@ -34,13 +39,30 @@ export function resolveRunningContainer( return { outcome: { status: 'fail', - message: `"${node}" is not a ${expectedLabel} node (it's a ${type} node). Point this check at your ${expectedLabel} node.`, - expected: `a ${expectedLabel} node named "${node}"`, - observed: `a ${type} node`, + message: `"${node}" is not ${an(expectedLabel)} node (it's ${an(type)} node). Point this check at your ${expectedLabel} node.`, + expected: `${an(expectedLabel)} node named "${node}"`, + observed: `${an(type)} node`, }, }; } + return { container }; +} + +/** + * Like `resolveContainerOfType`, but additionally requires the container to + * be currently running. + */ +export function resolveRunningContainer( + containers: ContainerInfo[], + node: string, + expectedTypes: string[], + expectedLabel: string +): ResolvedContainer { + const resolved = resolveContainerOfType(containers, node, expectedTypes, expectedLabel); + if ('outcome' in resolved) return resolved; + + const { container } = resolved; if (container.state !== 'running') { return { outcome: { @@ -102,3 +124,61 @@ export function countRunningAsgReplicas(containers: ContainerInfo[], asgContaine (c) => c.asgId === asgContainerId && c.isAsgInstance && c.state === 'running' ).length; } + +/** + * Connectivity semantics shared by `edge_exists` and `port_denied`, mirroring + * both the frontend Network Simulator and the real iptables enforcement + * (append-only rules + final zero-trust REJECT): only the destination's + * inbound rules matter, and the FIRST rule matching the pair and port wins — + * ALLOW opens the port, DENY (or no match at all) blocks it. Rules with + * `protocol: 'icmp'` are skipped: these checks are about TCP/UDP ports, and + * `computeSemanticRules` duplicates every ALL-protocol rule into an icmp + * twin that must not shadow the port decision. + */ +export function findMatchingInboundRule( + rules: SemanticRule[], + sourceId: string, + targetId: string, + port: number +): SemanticRule | undefined { + return rules.find( + (rule) => + rule.direction === 'inbound' && + rule.protocol !== 'icmp' && + rule.sourceNodeId === sourceId && + rule.targetNodeId === targetId && + (rule.port === 'ALL' || rule.port === String(port)) + ); +} + +/** + * Port-agnostic form of the first-match-wins walk, for `edge_exists` without + * a `port` param: is there ANY port on which the pair's first matching rule + * is an ALLOW? An earlier `DENY ALL` shadows everything after it; an earlier + * DENY on a specific port only shadows later ALLOWs on that exact port. + */ +export function hasEffectiveInboundAllow( + rules: SemanticRule[], + sourceId: string, + targetId: string +): boolean { + const deniedPorts = new Set(); + for (const rule of rules) { + if ( + rule.direction !== 'inbound' || + rule.protocol === 'icmp' || + rule.sourceNodeId !== sourceId || + rule.targetNodeId !== targetId + ) { + continue; + } + if (rule.action === 'ALLOW') { + if (rule.port === 'ALL' || !deniedPorts.has(rule.port)) return true; + } else if (rule.port === 'ALL') { + return false; + } else { + deniedPorts.add(rule.port); + } + } + return false; +} diff --git a/backend/src/modules/learning/engine/validators/tableExists.test.ts b/backend/src/modules/learning/engine/validators/tableExists.test.ts index d35720f..4a58f91 100644 --- a/backend/src/modules/learning/engine/validators/tableExists.test.ts +++ b/backend/src/modules/learning/engine/validators/tableExists.test.ts @@ -31,6 +31,20 @@ describe('tableExists', () => { expect(outcome.observed).toBe('no such table'); }); + it('fails when a psql WARNING pollutes the output of a missing table', async () => { + const outcome = await tableExists( + { node: 'db', table: 'users' }, + makeContext({ + containers: [postgresContainer], + executePsqlCommand: () => + Promise.resolve('WARNING: terminal is not fully functional\n'), + }) + ); + + expect(outcome.status).toBe('fail'); + expect(outcome.observed).toBe('no such table'); + }); + it('fails pedagogically when the database is still starting up', async () => { const outcome = await tableExists( { node: 'db', table: 'users' }, diff --git a/backend/src/modules/learning/engine/validators/tableExists.ts b/backend/src/modules/learning/engine/validators/tableExists.ts index fb126a1..c5dc2b7 100644 --- a/backend/src/modules/learning/engine/validators/tableExists.ts +++ b/backend/src/modules/learning/engine/validators/tableExists.ts @@ -35,7 +35,9 @@ export const tableExists: ValidatorHandler = async (params, ctx) => { }; } - if (output.trim().length === 0) { + // The query yields exactly `1` when the table exists — anything else (empty, + // or psql WARNING/NOTICE noise merged into the output) is not a proof. + if (output.trim() !== '1') { return { status: 'fail', message: `No table named "${table}" exists in PostgreSQL on "${node}" yet.`, diff --git a/backend/src/modules/learning/engine/validators/testSupport.ts b/backend/src/modules/learning/engine/validators/testSupport.ts index 457aff7..3b9b46d 100644 --- a/backend/src/modules/learning/engine/validators/testSupport.ts +++ b/backend/src/modules/learning/engine/validators/testSupport.ts @@ -1,5 +1,5 @@ import { ContainerInfo } from '../../../../infrastructure/docker/providers/containerProvider'; -import { SemanticRule } from '../../../network/models/networkPolicy'; +import { NetworkService } from '../../../network/services/networkService'; import { ValidatorContext, ValidatorNetworkConfig } from '../types'; /** Shared test fixtures for validator tests — not a test file itself (no `*.test.ts` suffix). */ @@ -17,19 +17,27 @@ export function makeContainer(overrides: Partial): ContainerInfo interface ContextOverrides { containers?: ContainerInfo[]; networkConfig?: ValidatorNetworkConfig | null; - semanticRules?: SemanticRule[]; executePsqlCommand?: ValidatorContext['executePsqlCommand']; executeRedisCommand?: ValidatorContext['executeRedisCommand']; executeMongoCommand?: ValidatorContext['executeMongoCommand']; executeCustomCommand?: ValidatorContext['executeCustomCommand']; } +/** + * Semantic rules are never injected by hand: they are always derived from the + * (realistic) network config through the real `computeSemanticRules`, exactly + * as the engine wires it — hand-built rules once hid real false-pass bugs + * behind a green suite. Tests exercising this path must mock `DockerClient` + * (`computeSemanticRules` lists containers to resolve ASG replicas). + */ export function makeContext(overrides: ContextOverrides = {}): ValidatorContext { + const config = overrides.networkConfig ?? null; return { projectId: 'project-1', getContainers: () => Promise.resolve(overrides.containers ?? []), - getNetworkConfig: () => Promise.resolve(overrides.networkConfig ?? null), - getSemanticRules: () => Promise.resolve(overrides.semanticRules ?? []), + getNetworkConfig: () => Promise.resolve(config), + getSemanticRules: () => + config ? NetworkService.computeSemanticRules('project-1', config) : Promise.resolve([]), executePsqlCommand: overrides.executePsqlCommand ?? (() => Promise.resolve('')), executeRedisCommand: overrides.executeRedisCommand ?? (() => Promise.resolve('')), executeMongoCommand: overrides.executeMongoCommand ?? (() => Promise.resolve('')), @@ -37,15 +45,40 @@ export function makeContext(overrides: ContextOverrides = {}): ValidatorContext }; } -export function makeSemanticRule(overrides: Partial): SemanticRule { - return { - sourceNodeId: 'src', - targetNodeId: 'dst', - protocol: 'tcp', - port: 'ALL', - action: 'ALLOW', - direction: 'inbound', - ownerNodeId: 'src', - ...overrides, - }; +export interface SecurityGroupRuleFixture { + type: 'inbound' | 'outbound'; + action: 'ALLOW' | 'DENY'; + protocol: string; + port: string; + source: string; +} + +/** + * The default security group every node gets when dropped on the canvas — + * mirror of `createDefaultRules` (frontend `CanvasPage/utils/securityRules.ts`): + * deny all inbound, allow all outbound. + */ +export function makeDefaultSgRules(): SecurityGroupRuleFixture[] { + return [ + { type: 'inbound', action: 'DENY', protocol: 'ALL', port: 'ALL', source: '0.0.0.0/0' }, + { type: 'outbound', action: 'ALLOW', protocol: 'ALL', port: 'ALL', source: '0.0.0.0/0' }, + ]; +} + +/** + * A realistic applied network config: every node sits in a subnet and carries + * the default security group, with the learner's rules prepended — the UI + * always inserts new rules at the top (highest first-match priority). + */ +export function makeNetworkConfig( + nodeIds: string[], + learnerRules: Record = {} +): ValidatorNetworkConfig { + const nodeSubnetMap: Record = {}; + const nodeSecurityGroups: Record = {}; + for (const nodeId of nodeIds) { + nodeSubnetMap[nodeId] = 'subnet-public-1'; + nodeSecurityGroups[nodeId] = [...(learnerRules[nodeId] ?? []), ...makeDefaultSgRules()]; + } + return { nodeSubnetMap, nodeSecurityGroups }; } diff --git a/docs/roadmap-format.md b/docs/roadmap-format.md index 663995c..01dd36d 100644 --- a/docs/roadmap-format.md +++ b/docs/roadmap-format.md @@ -100,11 +100,13 @@ The 9 validator types of format v1: | `edge_exists` | a connection edge exists from `source` to `target` (omit `port` to accept any port) | `{ "source": string, "target": string, "port"?: number }` | | `lb_upstreams` | the load balancer node has **at least** `min` upstream targets | `{ "node": string, "min": number }` | | `port_denied` | traffic from `source` to `target` on `port` is **blocked** | `{ "source": string, "target": string, "port": number }` | -| `asg_replicas` | the auto-scaling group node runs **exactly** `count` instances | `{ "node": string, "count": number }` | +| `asg_replicas` | the auto-scaling group node runs **exactly** `count` instances (`count` is a non-negative integer) | `{ "node": string, "count": number }` | | `http_get_contains` | an HTTP GET request to localhost inside the container responds with a body containing the expected string | `{ "node": string, "port": number, "path": string, "expectedText": string }` | Ports and counts are JSON numbers. +**Connectivity semantics (`edge_exists`, `port_denied`):** both checks evaluate the target's **inbound** security-group rules with the same first-match-wins order as the Network Simulator and the real firewall enforcement — the first rule matching the pair and port decides (`ALLOW` opens, `DENY` blocks), and no match means blocked (zero-trust default). `DENY` rules are honored: an early `DENY` blocks the port even when a broader `ALLOW` sits below it. `port_denied` additionally requires the network to actually be enforced: while no network configuration is applied to the project, or while either node sits outside a subnet, containers can talk freely on the shared bridge, so the check **fails** (pedagogically — the learner is told to place the nodes in subnets first) rather than blessing an unenforced topology. + ## UI Integrations & Localhost Access When a step references a target container, the Torollo learning player UI automatically exposes a clickable `http://localhost:` link below the instruction if the target node meets realistic networking preconditions: