Skip to content

fix(security): bound asg capacity and validate custom image references#76

Merged
Derssa merged 2 commits into
mainfrom
fix/s3-input-bounds
Jul 19, 2026
Merged

fix(security): bound asg capacity and validate custom image references#76
Derssa merged 2 commits into
mainfrom
fix/s3-input-bounds

Conversation

@OthmaneZ05

@OthmaneZ05 OthmaneZ05 commented Jul 19, 2026

Copy link
Copy Markdown
Collaborator

Summary of Changes

The ASG capacity endpoints accepted any desiredCapacity (unbounded container spawn: a single request could ask Docker for thousands of replicas), the network-config save accepted absurd minCapacity/maxCapacity values, and custom Docker image references reached docker.createContainer without any format validation. On top of that, the ASG modal silently swallowed every API error, so a backend rejection was invisible to the user.

This PR bounds those inputs server-side and surfaces rejections in the UI:

  • Capacity bounds (asgCapacity.ts, new): desiredCapacity must be an integer in 0..10 on /scale (0 = stop) and 1..10 on /deploy; minCapacity/maxCapacity/desiredCapacity inside networkConfig.asgs must be integers in 1..10 with min ≤ desired ≤ max. Violations answer a classified 400 { error, code: 'INVALID_CAPACITY' } before any service call. Validation of config entries is lenient (only fields that are present), so existing saved projects keep loading and saving.
  • Image reference validation (imageReference.ts, new): customImage is checked against the Docker reference grammar ([registry[:port]/]name[:tag][@digest], length caps) at the single choke point dockerContainerProvider.createContainer; invalid references throw and classify as 400 INVALID_IMAGE_REFERENCE via the existing sendDockerError path. No allowlist — the image runs locally under the user's responsibility (doc note added in docs/adding-a-node.md).
  • UI surfacing: AsgModal now checks every response and renders an inline role="alert" banner with the backend message (readErrorMessage reused); useNetworkConfig propagates the backend's error message instead of a generic HTTP status string. New asg.details.requestFailed key in both locales. No client-side clamp was added: the backend is the source of truth, and the rejection stays demonstrable from the UI.

Types of Changes

  • New feature / node type addition
  • Bug fix (non-breaking change resolving an issue)
  • Refactoring / structural cleanup
  • Documentation update

Verification & Testing

Automated Checks

  • Run npm run lint successfully with no errors
  • Run npm run build successfully with no compilation errors
  • Run npm test successfully (all tests pass)

Backend: 34 suites / 373 tests green (new: asgCapacity.test.ts, imageReference.test.ts, asgController.test.ts; extended: dockerErrors.test.ts, projectController.test.ts). Frontend: 20 files / 232 tests green (new: AsgModal.test.tsx, 7 tests covering banner on 400, success, retry-clears-error, stop failure, non-JSON fallback, deploy rejection, config-save rejection).

Manual Verification

Against real Docker (dev servers + curl + Playwright/Chrome):

$ curl -X POST .../containers/asg/demo/scale -d '{"desiredCapacity":11,"subnetIds":["s1"]}'
{"error":"Desired capacity must be between 0 and 10.","code":"INVALID_CAPACITY"}   (HTTP 400)

$ curl ... scale -d '{"desiredCapacity":2.5,...}'
{"error":"Desired capacity must be a whole number.","code":"INVALID_CAPACITY"}     (HTTP 400)

$ curl ... deploy -d '{"parentNodeId":"x","desiredCapacity":0,...}'
{"error":"Desired capacity must be between 1 and 10.","code":"INVALID_CAPACITY"}   (HTTP 400)

$ curl -X POST .../network-config -d '{"networkConfig":{"asgs":{"asg-demo":{"minCapacity":1,"desiredCapacity":2,"maxCapacity":50}}}}'
{"error":"ASG \"asg-demo\": maxCapacity must be a whole number between 1 and 10.","code":"INVALID_CAPACITY"} (HTTP 400)
  • Deploy at desiredCapacity: 10 → HTTP 200, docker ps --filter label=akal.asg.id=... = 10 running replicas (~19 s).
  • Playwright UI run on a real project (ubuntu template + ASG node): stepped Max/Desired to 11 in the modal → Save → red inline banner with the exact backend message (... desiredCapacity must be a whole number between 1 and 10.); stepped back to 10 → Save → banner cleared, ASG scaled to 10 real containers; Stop button (scale to 0) still works and terminated all replicas.
  • Existing configs without an asgs section re-save with HTTP 200 (backward compatible).

Checklist

  • My code follows the repository's code style and lint standards
  • I have updated the documentation or instructions if necessary
  • All unit and integration tests are passing

CI note

The first backend-integration-tests run failed (twice) in containerOwnership.itest.ts setup with network with name akal-lab-network already exists — a pre-existing check-then-create race: both itest suites run ensureSharedNetwork() concurrently on a fresh runner (jest parallelizes suites by default), so both see the network missing and both create it. The same job also failed on main (run 29657312416) with the mid-run flavor of the same cross-suite interference. Fixed here with maxWorkers: 1 in jest.integration.config.js — the suites share one Docker daemon (network, iptables chains, real containers), so they must run serially.

@OthmaneZ05
OthmaneZ05 requested a review from Derssa as a code owner July 19, 2026 01:50
@Derssa
Derssa merged commit 2876823 into main Jul 19, 2026
3 checks passed
@Derssa
Derssa deleted the fix/s3-input-bounds branch July 19, 2026 20:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants