Feat/e2e firewall compat by insuT0ver · Pull Request #94 · infosupport/huddle

insuT0ver · 2026-08-04T18:26:45Z

Summary

Related issue

Fixes #

Type of change

Checklist

My branch is up to date with main and focused on a single change.
The project builds and type-checks locally.
Tests pass (npm --prefix gateway test) and I added/updated tests where relevant.
I ran Prettier and did not reformat unrelated code.
Commit messages follow Conventional Commits.
I did not introduce logging of secrets, tokens, or credentials.
Documentation is updated where needed (README / relevant docs).

Notes for reviewers

The per-container Docker socket-proxy validated request bodies with case-sensitive key lookups (body.HostConfig.Privileged, mount.Type, ...), while the daemon's Go json decoder matches struct fields case-insensitively and merges duplicate keys. Every HostConfig guard was bypassable with a different casing (poc-suite.sh: 1a/1b/1c/fs), the exec-create body was never inspected (finding #7), and empty MaskedPaths/ReadonlyPaths unmasked /proc/kcore + /proc/sysrq-trigger (finding `mask`). Hardening (validateHostConfig / validateExecConfig / validateVolumeCreate and the create handlers): - findAmbiguousKey: reject case-insensitive duplicate keys anywhere (fail-closed) - deepLowerKeys: validate on a lowercased view so checks see what the daemon honours - renameKeyCI: canonicalize keys the proxy injects into (HostConfig/Labels/Env/ NetworkingConfig/NetworkMode, Options/Labels on net/vol create) so no second, daemon-merged key survives - inspect the exec-create body and refuse a privileged exec - hard-deny any MaskedPaths/ReadonlyPaths override (incl. empty array) 242 gateway tests pass (new socket-proxy-parser-hardening.test.ts covers each PoC vector; host-config.test.ts updated for the MaskedPaths deny). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

The MaskedPaths/ReadonlyPaths hard-deny used `!== undefined`, but the docker CLI sends `MaskedPaths: null` / `ReadonlyPaths: null` on every `docker create` (null = "daemon applies its secure defaults"). That made the guard fire on every ordinary create, short-circuiting before the real security checks — the e2e boundary regression on finding #8 got "MaskedPaths override not permitted" instead of the expected named-volume ownership denial. Gate on `Array.isArray` instead: only an explicitly supplied list (empty or trimmed) is an override and gets denied — exactly the PoC `mask` attack — while null/absent passes through to the daemon defaults. Adds a regression test for the null (CLI-default) shape. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

…74 DoS) findAmbiguousKey/deepLowerKeys recursed over attacker-controlled create/exec/ volume bodies unbounded. V8's JSON.parse accepts extremely deep nesting, so a single crafted body overflowed the call stack (RangeError) inside the async processInjectedBody / processExecCreate handlers -> unhandled rejection that crashes the shared gateway. Bound recursion to a fail-closed depth limit. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

…ked on expiry Redesign the noot admin-session flow from permanent standing credentials to an ephemeral per-grant model, keeping the vscode/noot separation intact: - noot is created LOCKED with no usable password (still in the sudo group); the build-time password generation and saveCredentials call are removed. - A new sudo grant sets a fresh random password via chpasswd over stdin and unlocks noot for a bounded duration; the password is returned exactly once and never persisted (not even hashed) — closes security-review finding #10. - An active sweeper locks noot again inside the container on expiry; revoke locks immediately. The docker-exec boundary is injected so the grant/expiry logic is unit-testable without a live daemon. - New endpoints POST/GET/DELETE /api/docker/containers/:name/sudo-grant replace the plaintext GET .../credentials endpoint; UI shows a grant button, a one-time password reveal, and an expiry countdown. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

grantSudo failed with "kon noot-wachtwoord niet zetten (exit null)": execInContainer appended the chpasswd stdin ("noot:<pw>\n") into the POST /exec/<id>/start request body under a single Content-Length. Docker parses that body as the JSON ExecStartCheck and rejects trailing bytes, so the exec never ran and the immediate inspect reported ExitCode: null. The raw http.request also ignored res.statusCode, hiding the failure. Fix: send only the JSON options in the start body, stream stdin over the hijacked connection and half-close it so the process sees EOF, surface a non-2xx start as an error instead of a silent null, and poll the exec inspect briefly so a numeric ExitCode is read reliably (WSL2 reap race). Only the chpasswd step (non-empty stdin) was affected; the empty-stdin unlock/lock execs already worked. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

…l-grant)

…hem (#73) Support `*` wildcards anywhere in a firewall path pattern: a mid-path `*` matches within a single segment (never crossing `/`), while a trailing `*` keeps the existing prefix semantics and may span deeper segments. This lets operators approve a broad, predictable endpoint (e.g. an Azure DevOps NuGet feed `/_packaging/*/nuget/v3/*` whose feed GUID changes per request) instead of a fresh per-request approval. Path normalisation still runs first, so traversal tricks fail closed. Add ways to author custom rules directly: - Frontend: an "Add rule" form on the Firewall page (domain, optional path pattern, global/container scope, allow/deny) posting to /api/rules. - CLI: `huddle firewall add <domain> [--path <pattern>] [--deny] [--container <id>]`. Document custom rules and wildcard support (domain `*.` and in-path `*`) in the README with the Azure DevOps NuGet example. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Path wildcard patterns from POST /api/rules were compiled into a backtracking RegExp. Metacharacters were escaped correctly, but multiple wildcards still produced adjacent unbounded quantifiers (`^/a[^/]*a[^/]*a...X$`). Against a long, attacker-controlled request-path segment this backtracks catastrophically (k=3 stars -> ~1.6s, k>=5 -> hang), blocking the gateway event loop (DoS). Replace the RegExp with a linear O(n*m) two-pointer glob matcher that preserves the exact matching semantics (mid `*` stays within a segment, trailing `*` crosses segments with the segment-boundary rule) and collapses consecutive `*`. Verified identical to the old regex over 392k fuzzed non-`**` cases; adversarial payloads now match in <1ms. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

…rd-rules)

specificity() scored an exact-host rule above a wildcard-host rule, and the status tie-break only applied within equal specificity. So an exact-host 'requested' row (auto-created when a host is first blocked) outranked a matching wildcard 'allow', keeping the host blocked even after the operator added a covering rule (finding #7). Make a concrete decision always outrank 'requested'.

`firewall add --path` created only the path rule and never the host-only path_mode=1 marker that isPathMode() requires, so over HTTPS the CONNECT was refused before the path could be read and the rule never fired (finding #6a). The create handler now calls ensurePathModeMarker() to enter path-allowlist mode, matching the portal's path-mode flow.

Add a versioned JSON envelope for sharing firewall rulesets. - Backend: GET /api/rules/export (optional ?container scope) and POST /api/rules/import with merge (upsert) / replace modes. Import validates every rule fail-closed and returns { imported, updated, skipped }; an optional ?container remaps all rules into that scope. - CLI: huddle firewall export [--container <id>] [--out <file>] and huddle firewall import <file> [--replace] [--container <id>]. - Frontend: Export (client-side blob download) and Import buttons on the firewall page. - Tests: gateway/test/rules-export-import.test.ts covers the envelope, merge round-trip, replace scope, upsert and fail-closed validation. - Docs: README section on the JSON format, merge vs replace, scope and CLI/UI/API usage. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

…-rules)

#74) Node emit't WebSocket-handshakes als een SEPARAAT 'upgrade'-event i.p.v. 'request'. De MITM innerHttp-server en de top-level plain-HTTP-server hadden geen upgrade-handler, dus sloot Node de socket en time-outte de handshake — de Codex-CLI-vertraging bij elke prompt. Nu forwarden beide paden Upgrade- handshakes (ws:// en wss://) transparant via een gedeelde forwardUpgrade- helper, met exact dezelfde fail-closed firewall-handhaving (host + pad) als de request-handlers. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

…ket leak A forwarded WebSocket/Upgrade handshake to an allowed host had no timeout. Once Node emits 'upgrade' the raw socket is detached from the http.Server, so server.requestTimeout/headersTimeout no longer apply. An upstream that accepts TCP but never completes the handshake (stalled/slowloris, or a blackholed SYN) therefore held both the client socket and the outbound gateway socket open indefinitely. A devcontainer with a single allowed host could pile up unbounded half-open handshakes and exhaust the shared gateway's file descriptors (DoS). forwardUpgrade now arms a handshake-phase timer (default 30s, WS_UPGRADE_TIMEOUT_MS override for tests) that tears down both sockets if upstream never returns 101 or a response; the timer is cleared on 101/response so established long-lived WebSockets are unaffected. upstreamReq.socket is destroyed explicitly because ClientRequest.destroy() alone can leave the connected socket alive. Firewall enforcement (host+path fail-closed, TLS verification) was independently verified as correct on both the plain-ws:// and MITM-wss:// upgrade paths. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

- forwardUpgrade: replace `if (typeof handshakeTimer.unref === 'function')` (always true under Node) with optional-chaining `unref?.()` — same defensiveness, no "impossible logic". - proxy-websocket test: swap the RFC 6455 sample key (dGhlIHNhbXBsZSBub25jZQ==) for a low-entropy dummy so the secret scanner stops flagging a protocol header as a credential. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

aikido-pr-checks · 2026-08-04T18:27:44Z

+  return line;
+}
+
+export function parseYaml(input: string): ComposeDoc {


parseYaml's recursive helpers (parseNode/parseMapping/parseSequence) lack a recursion depth limit; add an explicit max-depth parameter or switch to an iterative parser to prevent stack-overflow DoS.

Details

✨ AI Reasoning
parseYaml was added to cli/src/migrate.ts. It defines parseNode, parseMapping and parseSequence which call each other recursively based on indentation-driven structure. There is no depth counter or maximum depth check. Maliciously deep YAML input can cause unbounded recursion and stack overflow, crashing the CLI process. This is introduced in this diff (new function).

🔧 How do I fix it?
Add depth limiting via counter parameters that are checked and enforced, or replace with iterative approaches using explicit loops or stack data structures. For graphs, combine depth limiting with visited set tracking.

_{Reply @AikidoSec feedback: [FEEDBACK] to get better review comments in the future.}
_{Reply @AikidoSec ignore: [REASON] to ignore this issue.}
_{More info}

The socket teardown/write catches intentionally ignore errors (destroying or writing to an already-closed peer socket throws harmlessly). Annotate them so the intent is explicit and the empty-catch lint is satisfied; no behavior change.

#81) Rancher Desktop in dockerd (moby) mode looks like a normal Docker engine but exposes its socket at ~/.rd/docker.sock instead of /var/run/docker.sock. resolveRuntime now reads the active `docker context inspect` endpoint and, when it points at a Rancher Desktop socket, mounts that path and marks the engine as remote (it runs in a VM on every OS). Falls back to the default socket for native Docker / Docker Desktop; Podman detection is unchanged. Adds pure helpers parseDockerContextSocket / isRancherDesktopSocket with a vitest unit test (placed in gateway/test, where the repo's test runner and CI live, since the CLI package has no runner). Documents Rancher Desktop support in the README (Getting Started, FAQ, Troubleshooting). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

…ath in rancher-desktop detection The socket path parsed from `docker context inspect` is interpolated unquoted into the `docker run -v <path>:...` shell command in init.ts. An influenceable docker context (Host = unix:///tmp/$(cmd)/.rd/docker.sock) passed both parseDockerContextSocket and isRancherDesktopSocket, enabling command injection during `huddle init`. Restrict parsed paths to a safe allowlist of filesystem characters. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

#66) Add `huddle migrate`, which wires an existing docker-compose Dev Container project behind Huddle without the developer editing proxy/CA/socket settings into their own files. Convention: the developer labels one (internal) network with `huddle.network: "true"`. The command generates a Compose OVERRIDE (`docker-compose.huddle.yml`) that, for each service on that network, injects the proxy env vars (`HTTP(S)_PROXY`, `NO_PROXY` including `huddle`), `NODE_EXTRA_CA_CERTS`, and attaches the service to Huddle's existing internal `devcontainer-net` (as `external: true`). The user's compose/devcontainer.json, extensions, features and lifecycle commands stay untouched. Verified end-to-end with `docker compose config`, which merges both files cleanly. The override generation and a dependency-free compose YAML reader/emitter are pure, unit-tested functions (gateway/test/migrate.test.ts, run from the gateway package where vitest lives). `--docker-socket` generates the filtered-socket mount + DOCKER_HOST but this is explicitly flagged (in output and docs) as generated-not-yet-served: it needs gateway-side socket pre-provisioning, a follow-up on #66. Docs: docs/migrate-devcontainers.md (linked from README). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

…ctive injection in migrate The dependency-free YAML emitter (dumpYaml) wrote object keys unescaped. Service/network names come from an untrusted docker-compose.yml, so a crafted key (e.g. "a: b", "evil #comment", or one containing a newline) could break out of its line and inject sibling compose directives (such as privileged: true) or silently corrupt the generated security override. Keys now go through the same quote/escape path as scalar values, and the escaper additionally handles newline/CR/tab. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

huddleNetworkExists built a shell command string with execSync; Aikido flags that as a command-injection surface. Switch to execFileSync with an argument array so no shell is involved — the runtime name and network name can never be interpreted as shell syntax. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

…ntainers)

…ys) fail-closed The hand-rolled YAML reader silently mis-parses block scalars (`|`/`>`), anchors/aliases and merge keys — a block scalar's indented body was read as structure, which could swallow the following service and drop it from the generated override, leaving it with an unfiltered route out. Detect these constructs up front and refuse with a clear, actionable error instead.

One end-to-end firewall test driven across the OS × container-runtime matrix. The GitHub Actions workflow holds only the platform/runtime config; all test logic lives in tests/firewall/ (run-test.sh + run-test.ps1) sharing one huddle-test-config/cases.env so the assertions stay identical locally and in CI. Milestone 1 (firewall basics): activate runtime → huddle init (real install path, locally built gateway image) → minimal curl devcontainer on the internal net with the migrate-style proxy env + CA → allowed URL reachable (200) → blocked URL stays denied (403) → optional path mode → collect logs → always clean up. The full 3×3 matrix is present from the start; only the combinations that work headless on hosted runners (Linux + Docker/Podman) run for real, the rest are hosted:false and reported as planned until a self-hosted runner exists. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

aikido-pr-checks · 2026-08-04T19:25:27Z

+  }
+  const markedNetwork = marked[0];
+
+  // Pick a collision-free key for the injected egress network.


buildOverride claims a collision-free network key, but only tries one fallback (huddle-egress) without rechecking. If that key already exists, the injected network key still collides.

Show fix

Suggested change

// Pick a collision-free key for the injected egress network.

// Pick a collision-free key for the injected egress network.

if (existingNetKeys.has(huddleNetKey) && huddleNetKey !== markedNetwork) {

let suffix = 2;

while (existingNetKeys.has(`${HUDDLE_NET_KEY}-egress-${suffix}`)) {

suffix++;

}

huddleNetKey = `${HUDDLE_NET_KEY}-egress-${suffix}`;

}

Details

✨ AI Reasoning
1) The code intends to choose a unique network key for injected configuration.
2) It checks one preferred key and, on conflict, switches to one fallback key.
3) It never verifies whether that fallback key is also already present.
4) Therefore, with certain valid inputs, the selected key still collides, contradicting the stated intent and causing override data to target an existing key instead of a unique one.

_{Reply @AikidoSec feedback: [FEEDBACK] to get better review comments in the future.}
_{Reply @AikidoSec ignore: [REASON] to ignore this issue.}
_{More info}

aikido-pr-checks · 2026-08-04T19:25:28Z

+  rejectUnsupportedYaml(lines);
+  let pos = 0;
+
+  function parseNode(indent: number): unknown {


parseNode (and its companions parseMapping/parseSequence) perform unbounded recursive descent over parsed YAML; add a depth limit or iterative approach to avoid stack overflow from deeply-nested input.

Details

✨ AI Reasoning
The parser's core functions parseNode, parseMapping and parseSequence call each other recursively to walk YAML structure. There is no max-depth parameter or depth check enforced by these functions. Maliciously crafted deeply-nested YAML input could trigger unbounded recursion and exhaust the call stack, causing a crash/DoS. Other parts of the diff added explicit MAX_KEY_DEPTH guards for similar recursive walkers, but the YAML parser lacks any such protection, so this change introduced unprotected recursion.

🔧 How do I fix it?
Add depth limiting via counter parameters that are checked and enforced, or replace with iterative approaches using explicit loops or stack data structures. For graphs, combine depth limiting with visited set tracking.

_{Reply @AikidoSec feedback: [FEEDBACK] to get better review comments in the future.}
_{Reply @AikidoSec ignore: [REASON] to ignore this issue.}
_{More info}

aikido-pr-checks · 2026-08-04T19:25:29Z

+
+// Classic greedy glob match with a single remembered wildcard position → O(n·m),
+// no exponential backtracking. A MID_STAR may not eat `/`; a CROSS_STAR may.
+function matchTokens(tokens: Token[], str: string): boolean {


matchTokens implements a subtle stateful glob matcher with multiple responsibilities (star bookkeeping, segment-boundary rules), making it hard to reason about; consider decomposing and adding focused unit tests.

Details

✨ AI Reasoning
1. Is a function introduced/modified in the diff? Yes — matchTokens was added.
2. What is the function trying to accomplish? It performs a greedy two-pointer glob matching algorithm that must respect segment boundaries and wildcard kinds.
3. Does this harm maintainability? The function encodes subtle stateful matching behaviour (star bookkeeping, cross vs mid-star semantics, boundary checks) that is non-trivial to reason about and test; bundling this complexity into a single function makes future maintenance error-prone.
4. Is it appropriate for context? The security motivation is valid, but the internal complexity would benefit from clearer decomposition and more comments or unit tests per sub-step.
5. Fixability within PR scope? Extract small helpers (advance/star stretch, isSeparator) or add more granular tests/documentation.
Confidence computation steps:

Base = 0.5

Function length: ~30 lines -> length <40 does not force zero, but cognitive complexity applies

Cognitive complexity: +0.4 (non-trivial algorithmic state machine)

Not an entry-point → no -0.4

Final confidence = 0.9

🔧 How do I fix it?
Break down long functions into smaller helper functions. Aim for functions under 60 lines with fewer than 10 local variables.

_{Reply @AikidoSec feedback: [FEEDBACK] to get better review comments in the future.}
_{Reply @AikidoSec ignore: [REASON] to ignore this issue.}
_{More info}

insuT0ver and others added 18 commits August 3, 2026 10:07

i18n: translate Dutch comments and messages to English (socket-proxy)

6256a85

i18n: translate Dutch comments and messages to English (sudo-ephemera…

0011d4a

…l-grant)

i18n: translate Dutch comments and messages to English (custom-wildca…

42b1fba

…rd-rules)

i18n: translate Dutch comments and messages to English (export-import…

eb82bd7

…-rules)

i18n: translate Dutch comments and messages to English (websocket-proxy)

f195ea7

insuT0ver requested a review from a team August 4, 2026 18:26

aikido-pr-checks Bot reviewed Aug 4, 2026

View reviewed changes

insuT0ver and others added 10 commits August 4, 2026 18:41

i18n: translate Dutch comments and messages to English (rancher-desktop)

256ccf5

i18n: translate Dutch comments and messages to English (migrate-devco…

eca0a4a

…ntainers)

insuT0ver force-pushed the feat/e2e-firewall-compat branch from 397dfbb to fd1c28e Compare August 4, 2026 19:24

aikido-pr-checks Bot reviewed Aug 4, 2026

View reviewed changes

Navigation Menu

Uh oh!

Feat/e2e firewall compat - #94

Feat/e2e firewall compat#94
insuT0ver wants to merge 28 commits into
mainfrom
feat/e2e-firewall-compat

insuT0ver commented Aug 4, 2026

Uh oh!

aikido-pr-checks Bot Aug 4, 2026

Uh oh!

aikido-pr-checks Bot Aug 4, 2026 •

edited

Loading

Uh oh!

aikido-pr-checks Bot Aug 4, 2026

Uh oh!

aikido-pr-checks Bot Aug 4, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

-  // Pick a collision-free key for the injected egress network.
+  // Pick a collision-free key for the injected egress network.
+    if (existingNetKeys.has(huddleNetKey) && huddleNetKey !== markedNetwork) {
+      let suffix = 2;
+      while (existingNetKeys.has(`${HUDDLE_NET_KEY}-egress-${suffix}`)) {
+        suffix++;
+      }
+      huddleNetKey = `${HUDDLE_NET_KEY}-egress-${suffix}`;
+    }

Uh oh!

Conversation

insuT0ver commented Aug 4, 2026

Summary

Related issue

Type of change

Checklist

Notes for reviewers

Uh oh!

aikido-pr-checks Bot Aug 4, 2026

Choose a reason for hiding this comment

Uh oh!

aikido-pr-checks Bot Aug 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aikido-pr-checks Bot Aug 4, 2026

Choose a reason for hiding this comment

Uh oh!

aikido-pr-checks Bot Aug 4, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

aikido-pr-checks Bot Aug 4, 2026 •

edited

Loading