fix(learning): harden validators against false passes on network and DB checks

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,25 +205,47 @@ 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');
     expect(result.expected).toBeTruthy();
     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');

backend/src/modules/learning/engine/params.ts

@@ -21,6 +21,17 @@ export function requireNumberParam(params: Record<string, unknown>, name: string
   return value;
 }
 
+export function requireNonNegativeIntegerParam(
+  params: Record<string, unknown>,
+  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<string, unknown>,
   name: string,

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);
+  });
 });

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 {

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');

backend/src/modules/learning/engine/validators/edgeExists.ts

@@ -1,13 +1,15 @@
 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
  * `target` (omit `port` to accept any port). Params: `{ source: string,
  * 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',
     };
   }