Support Aspire in huddle - #82
Conversation
…61) Aspire's DCP inspects a persistent container (deterministic name) before creating it. The socket-proxy's ownership check returned 403 for a container this devcontainer did not own — including one that did not exist yet — so DCP concluded the container existed but was unreachable, never created it, and the resource stayed in state 'Unknown'. From a devcontainer's point of view, any container it did not create simply does not exist. The inspect/logs/top/archive/stats read path now returns Docker's own "No such container" 404 for everything that is not owned: - Fixes #61: a not-yet-created persistent container reads as absent, so DCP creates it. - Closes an existence oracle: 'foreign' and 'missing' are indistinguishable, so a devcontainer can no longer probe which container names exist outside its sandbox (previously foreign→403 vs missing→404). - No TOCTOU: the 404 is synthesized after the ownership check instead of forwarding the original request, so a container created in the race window can't be inspected. A foreign-container probe (a name that really exists but isn't ours) is logged for the operator while still returning 404 to the caller. Ownership classification is factored into a pure ownershipFromInspect() with unit tests. Fixes #61 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…container Published ports of docker-outside-of-docker workloads bind on the host's loopback and were unreachable from the devcontainer (no default route, own loopback), so Aspire/DCP, Testcontainers and plain `docker run -p` saw health checks and connections hang or get refused. Mirror the docker.sock mechanism: per published TCP port the gateway listens on /tmp/dc-sockets/<owner>/ports/<hostPort>.sock (shared into the devcontainer as /var/run/huddle/ports) and pipes it to the workload container. An in-devcontainer forwarder (Node, installed via docker exec) mirrors those sockets onto 127.0.0.1/[::1]:<hostPort> and maps host.docker.internal to loopback in /etc/hosts. Relays are built before the buffered start/restart response returns (closes the dynamic-port race, confirmed via <port>.ready/.err handshake) and torn down on stop/kill/remove. The backend dial connects to containerIP:containerPort on the workload's network. Because Docker's inter-bridge isolation silently drops SYNs, the gateway joins that network on demand (idempotent, refcounted, detaches on last-relay teardown so `docker network rm` keeps working; dc-net-* is never detached) and every dial has a 5s timeout that fails fast instead of hanging clients. Security, aligned with the #82 ownership model: - relays and network joins only for containers with huddle.parent == requesting devcontainer, re-checked per connection; - proxy (:80) now default-denies sources that resolve to no huddle-managed container: no global-rule fallback, no requested-rule pollution (with a forced container-map refresh on miss to avoid false denials); - API (:3000) drops TCP connections originating from relay-joined workload network subnets, so a join adds no new API surface. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
TimMahyIS
changed the title
…container fast-path De e2e-check op de PR faalde omdat de boundary-test bij `docker inspect huddle` nog de oude 403 "not owned" verwachtte, terwijl de lees-tak sinds de #61-fix bewust een gesynthetiseerde 404 teruggeeft (foreign en missing zijn niet te onderscheiden — geen bestaans-oracle). De test pint nu met `--type container` het containerpad vast ("No such container", en expliciet géén "not owned"/"not permitted" in het antwoord); zonder --type viel de CLI na de 404 terug op image-inspect en botste op de aparte image.inspect-actietoggle. Daarnaast het goedkope devcontainerIds-fast-path terug op de inspect-tak (review-opmerking Aikido): een devcontainer is per definitie nooit 'own', dus de Docker-round-trip van de ownership-check kan daar over worden geslagen. Het antwoord is exact dezelfde 404 als de trage route, dus er ontstaat geen onderscheidbaar antwoord. Nieuwe e2e-test dekt dit pad (self-inspect van de devcontainer leest óók als niet-bestaand). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
| client.pause(); | ||
| hasOwnLabel('container', inspectCt, containerName).then(ok => { | ||
| if (ok) { | ||
| classifyContainerOwnership(inspectCt, containerName).then(ownership => { |
There was a problem hiding this comment.
Addressed in 89adaa2: het goedkope devcontainerIds.has()-fast-path staat terug vóór classifyContainerOwnership, met exact dezelfde gesynthetiseerde 404 als de trage route (dus geen onderscheidbaar antwoord / bestaans-oracle). Kanttekening: het oude pad deed op deze tak óók al een Docker-round-trip (hasOwnLabel), dus de winst zit alleen in het devcontainer-geval. Een e2e-test dekt het fast-path nu af.
![aikido-pr-checks[bot]](https://avatars.githubusercontent.com/in/898896?s=60&v=4){kind=small}
| const aliasIndex = new Map<string, string>(); | ||
|
|
||
| function portsDirFor(owner: string): string { | ||
| return path.join(SOCKET_DIR, owner, 'ports'); |
There was a problem hiding this comment.
Potential file inclusion attack via reading file - high severity
If an attacker can control the input leading into the ReadFile function, they might be able to read sensitive files and launch further attacks with that information.
Show fix
Remediation: Ignore this issue only after you've verified or sanitized the input going into this function. This issue is only relevant in the backend, not in the frontend!
Reply @AikidoSec ignore: [REASON] to ignore this issue.
More info
There was a problem hiding this comment.
@AikidoSec ignore: Gefixt in 812d346 — de input is nu gesanitized: owner wordt op beide publieke entry points (syncContainerRelays, ensurePortForwarder) én in portsDirFor zelf door assertSafeOwner gehaald (zelfde Docker-naamgrammatica als assertSafeContainerName in socket-proxy.ts: geen slashes, geen leidende punt), vóór alle I/O. Traversal buiten /tmp/dc-sockets is daarmee onmogelijk; unit-tests dekken de guard.
There was a problem hiding this comment.
✅ Based on your feedback, we ignored this issue because of the following reason:
Gefixt in 812d346 — de input is nu gesanitized:
ownerwordt op beide publieke entry points (syncContainerRelays, ensurePortForwarder) én in portsDirFor zelf doorassertSafeOwnergehaald (zelfde Docker-naamgrammatica als assertSafeContainerName in socket-proxy.ts: geen slashes, geen leidende punt), vóór alle I/O. Traversal buiten /tmp/dc-sockets is daarmee onmogelijk; unit-tests dekken de guard.
| // gebruik in de devcontainer). Timeout is geen fout: de relay werkt dan alsnog | ||
| // zodra de forwarder bijtrekt, alleen kunnen eerste connecties racen. | ||
| async function waitForForwarderReady(dir: string, spec: RelaySpec, owner: string): Promise<void> { | ||
| const readyPath = path.join(dir, `${spec.hostPort}.ready`); |
There was a problem hiding this comment.
Potential file inclusion attack via reading file - medium severity
If an attacker can control the input leading into the ReadFile function, they might be able to read sensitive files and launch further attacks with that information.
Show fix
Remediation: Ignore this issue only after you've verified or sanitized the input going into this function. This issue is only relevant in the backend, not in the frontend!
Reply @AikidoSec ignore: [REASON] to ignore this issue.
More info
There was a problem hiding this comment.
@AikidoSec ignore: Pad is afgeleid van spec.hostPort, dat in extractRelaySpecs met parseInt + Number.isInteger + >0 gevalideerd is; dir komt uit portsDirFor, waar owner sinds 812d346 door assertSafeOwner gaat. Geen attacker-controlled pad mogelijk.
There was a problem hiding this comment.
✅ Based on your feedback, we ignored this issue because of the following reason:
Pad is afgeleid van
spec.hostPort, dat in extractRelaySpecs met parseInt + Number.isInteger + >0 gevalideerd is;dirkomt uit portsDirFor, waarownersinds 812d346 door assertSafeOwner gaat. Geen attacker-controlled pad mogelijk.
| // zodra de forwarder bijtrekt, alleen kunnen eerste connecties racen. | ||
| async function waitForForwarderReady(dir: string, spec: RelaySpec, owner: string): Promise<void> { | ||
| const readyPath = path.join(dir, `${spec.hostPort}.ready`); | ||
| const errPath = path.join(dir, `${spec.hostPort}.err`); |
There was a problem hiding this comment.
Potential file inclusion attack via reading file - medium severity
If an attacker can control the input leading into the ReadFile function, they might be able to read sensitive files and launch further attacks with that information.
Show fix
Remediation: Ignore this issue only after you've verified or sanitized the input going into this function. This issue is only relevant in the backend, not in the frontend!
Reply @AikidoSec ignore: [REASON] to ignore this issue.
More info
There was a problem hiding this comment.
@AikidoSec ignore: Zelfde als hierboven — integer-gevalideerde hostPort + assertSafeOwner-gegarandeerde directory (812d346).
There was a problem hiding this comment.
✅ Based on your feedback, we ignored this issue because of the following reason:
Zelfde als hierboven — integer-gevalideerde hostPort + assertSafeOwner-gegarandeerde directory (812d346).
| if (fs.existsSync(readyPath)) return; | ||
| if (fs.existsSync(errPath)) { | ||
| let msg = ''; | ||
| try { msg = fs.readFileSync(errPath, 'utf8').trim(); } catch {} |
There was a problem hiding this comment.
Potential file inclusion attack via reading file - medium severity
If an attacker can control the input leading into the ReadFile function, they might be able to read sensitive files and launch further attacks with that information.
Show fix
Remediation: Ignore this issue only after you've verified or sanitized the input going into this function. This issue is only relevant in the backend, not in the frontend!
Reply @AikidoSec ignore: [REASON] to ignore this issue.
More info
There was a problem hiding this comment.
@AikidoSec ignore: errPath is opgebouwd uit de integer-gevalideerde hostPort binnen de assertSafeOwner-gegarandeerde ports-directory (812d346); het bestand wordt bovendien alleen gelezen om een foutmelding te loggen.
There was a problem hiding this comment.
✅ Based on your feedback, we ignored this issue because of the following reason:
errPathis opgebouwd uit de integer-gevalideerde hostPort binnen de assertSafeOwner-gegarandeerde ports-directory (812d346); het bestand wordt bovendien alleen gelezen om een foutmelding te loggen.
| } | ||
|
|
||
| for (const spec of tcp) { | ||
| const sockPath = path.join(dir, `${spec.hostPort}.sock`); |
There was a problem hiding this comment.
Potential file inclusion attack via reading file - medium severity
If an attacker can control the input leading into the ReadFile function, they might be able to read sensitive files and launch further attacks with that information.
Show fix
Remediation: Ignore this issue only after you've verified or sanitized the input going into this function. This issue is only relevant in the backend, not in the frontend!
Reply @AikidoSec ignore: [REASON] to ignore this issue.
More info
There was a problem hiding this comment.
@AikidoSec ignore: Zelfde afleiding: integer-gevalideerde hostPort + assertSafeOwner-gegarandeerde directory (812d346).
There was a problem hiding this comment.
✅ Based on your feedback, we ignored this issue because of the following reason:
Zelfde afleiding: integer-gevalideerde hostPort + assertSafeOwner-gegarandeerde directory (812d346).
|
|
||
| // Installeer/start de forwarder in een (draaiende) devcontainer. Aangeroepen | ||
| // bij devcontainer-aanmaak, bij een start via het portal en bij gateway-start. | ||
| export async function ensurePortForwarder(owner: string, containerRef?: string): Promise<void> { |
There was a problem hiding this comment.
ensurePortForwarder injects and starts a dynamically-written script inside the devcontainer; this runtime code injection obscures behavior and should be made explicit and auditable.
Details
✨ AI Reasoning
ensurePortForwarder calls dockerRequestJson to create and start an exec inside the devcontainer, which runs the buildForwarderSetupScript payload that decodes and launches the embedded FORWARDER_JS. The chain: generate base64 → exec create/start inside target container → decode/write/execute there — results in code being delivered and executed at runtime in another environment. This dynamic install-and-run pattern can hide the runtime behavior from static review and should be explicitly justified and audited. It resembles covert code injection techniques.
🔧 How do I fix it?
Ensure code is transparent and not intentionally obfuscated. Avoid hiding functionality from code review. Focus on intent and deception, not specific patterns.
Reply @AikidoSec feedback: [FEEDBACK] to get better review comments in the future.
Reply @AikidoSec ignore: [REASON] to ignore this issue.
More info
There was a problem hiding this comment.
@AikidoSec ignore: Bewust ontwerp, geen verhulling: FORWARDER_JS is een statische, reviewbare constante in deze module (niet dynamisch geconstrueerd) en het install-via-exec-patroon spiegelt het bestaande docker.sock-mechanisme van de gateway (per-container mount + door de gateway beheerde helper). Base64 dient uitsluitend als quoting-veilig transport door sh -c; het script is idempotent (cmp -s + kill -0) en logt naar /tmp/huddle-port-forwarder.log.
There was a problem hiding this comment.
✅ Based on your feedback, we ignored this issue because of the following reason:
Bewust ontwerp, geen verhulling: FORWARDER_JS is een statische, reviewbare constante in deze module (niet dynamisch geconstrueerd) en het install-via-exec-patroon spiegelt het bestaande docker.sock-mechanisme van de gateway (per-container mount + door de gateway beheerde helper). Base64 dient uitsluitend als quoting-veilig transport door
sh -c; het script is idempotent (cmp -s + kill -0) en logt naar /tmp/huddle-port-forwarder.log.
| if (!selfRefPromise) { | ||
| selfRefPromise = (async () => { | ||
| const candidates: string[] = []; | ||
| try { candidates.push(fs.readFileSync('/etc/hostname', 'utf8').trim()); } catch {} |
There was a problem hiding this comment.
Empty catch swallowing errors when reading /etc/hostname in resolveSelfRef; at least log the error or document why it's safe to ignore failures.
| try { candidates.push(fs.readFileSync('/etc/hostname', 'utf8').trim()); } catch {} | |
| try { candidates.push(fs.readFileSync('/etc/hostname', 'utf8').trim()); } catch (err: any) { | |
| console.warn('[port-relay] failed to read /etc/hostname for self-ref, will fall back to "huddle":', err.message); | |
| } |
Details
✨ AI Reasoning
A try reads /etc/hostname and falls back to candidate 'huddle' when the read fails. The catch block is empty, silently swallowing any error from reading the file. Silently ignoring filesystem errors during identity resolution can hide operational problems and makes debugging harder.
Reply @AikidoSec feedback: [FEEDBACK] to get better review comments in the future.
Reply @AikidoSec ignore: [REASON] to ignore this issue.
More info
There was a problem hiding this comment.
Gefixt in 812d346: de catch logt nu een warn met de reden en de gebruikte fallback.
| cacheExpiry = now + CACHE_TTL_MS; | ||
| } catch {} |
There was a problem hiding this comment.
Empty catch swallowing errors during forced container-map refresh in resolveContainerByIp; at least log the failure for observability.
Show fix
| cacheExpiry = now + CACHE_TTL_MS; | |
| } catch {} | |
| cacheExpiry = now + CACHE_TTL_MS; | |
| } catch (err: any) { | |
| console.warn(`[resolveContainerByIp] forced refresh failed for ${ip}:`, err.message ?? err); | |
| } |
Details
✨ AI Reasoning
resolveContainerByIp adds a second fetch attempt on cache miss that wraps fetchContainerMap in a try/catch with an empty catch. Silently swallowing errors from a forced refresh can lead to obscure 'unknown source' denials and complicate diagnosing why the container map is stale.
Reply @AikidoSec feedback: [FEEDBACK] to get better review comments in the future.
Reply @AikidoSec ignore: [REASON] to ignore this issue.
More info
There was a problem hiding this comment.
Gefixt in 812d346: de catch logt nu een warn zodat een "unknown source"-denial door een mislukte refresh te diagnosticeren is.
| const b64 = Buffer.from(FORWARDER_JS, 'utf8').toString('base64'); | ||
| return `#!/bin/sh | ||
| TARGET=/usr/local/lib/huddle-port-forwarder.js | ||
| PIDFILE=/tmp/huddle-port-forwarder.pid | ||
| mkdir -p /usr/local/lib /var/run/huddle/ports 2>/dev/null || true | ||
| TMP=$(mktemp /tmp/huddle-pf.XXXXXX) | ||
| echo '${b64}' | base64 -d > "$TMP" |
There was a problem hiding this comment.
buildForwarderSetupScript base64-encodes FORWARDER_JS and writes/executes it in the container; avoid embedding/executing dynamically-constructed payloads.
Show fix
| const b64 = Buffer.from(FORWARDER_JS, 'utf8').toString('base64'); | |
| return `#!/bin/sh | |
| TARGET=/usr/local/lib/huddle-port-forwarder.js | |
| PIDFILE=/tmp/huddle-port-forwarder.pid | |
| mkdir -p /usr/local/lib /var/run/huddle/ports 2>/dev/null || true | |
| TMP=$(mktemp /tmp/huddle-pf.XXXXXX) | |
| echo '${b64}' | base64 -d > "$TMP" | |
| return `#!/bin/sh | |
| TARGET=/usr/local/lib/huddle-port-forwarder.js | |
| PIDFILE=/tmp/huddle-port-forwarder.pid | |
| mkdir -p /usr/local/lib /var/run/huddle/ports 2>/dev/null || true | |
| TMP=$(mktemp /tmp/huddle-pf.XXXXXX) | |
| cat > "$TMP" << 'FORWARDER_EOF' | |
| ${FORWARDER_JS}FORWARDER_EOF |
Details
✨ AI Reasoning
A large JavaScript payload is included as FORWARDER_JS and later encoded and injected into a running container. The code constructs a base64 string of FORWARDER_JS, decodes it inside a temporary file in buildForwarderSetupScript, moves it to a target path, and starts it with nohup/node. This sequence dynamically creates and executes code inside another process/container at runtime. Such dynamic generation/execution (base64 encode/decode → write → exec) obscures the delivered code and can hide behavior from code reviewers. Even if the payload is benign forwarding logic, the pattern hides its contents until decode time and resembles obfuscation/hidden execution techniques often used by malware or backdoors.
Reply @AikidoSec feedback: [FEEDBACK] to get better review comments in the future.
Reply @AikidoSec ignore: [REASON] to ignore this issue.
More info
There was a problem hiding this comment.
@AikidoSec ignore: Zie hierboven — de payload is een statische constante in de repo en dus gewoon reviewbaar; base64 is er alleen als quoting-veilig transport (de heredoc-suggestie is juist fragieler: FORWARDER_JS bevat quotes/template-literals en een onge-escapete ${...} in een sh-heredoc introduceert nieuwe injectiekansen).
There was a problem hiding this comment.
✅ Based on your feedback, we ignored this issue because of the following reason:
Zie hierboven — de payload is een statische constante in de repo en dus gewoon reviewbaar; base64 is er alleen als quoting-veilig transport (de heredoc-suggestie is juist fragieler: FORWARDER_JS bevat quotes/template-literals en een onge-escapete
${...}in een sh-heredoc introduceert nieuwe injectiekansen).
…llowed refresh errors Aikido-findings op de port-relay-commit: - `owner` vloeit in path.join() onder de gedeelde sockets-directory en komt o.a. binnen via een operator-API-parameter. Dezelfde Docker-naamgrammatica als assertSafeContainerName (socket-proxy.ts) wordt nu ook op de relay- entry-points afgedwongen (assertSafeOwner, vóór alle I/O) — traversal buiten /tmp/dc-sockets is daarmee ook standalone onmogelijk. Gedupliceerd i.p.v. geïmporteerd: socket-proxy importeert déze module al. De overige geflagde fs-paden zijn afgeleid van de integer-gevalideerde hostPort uit extractRelaySpecs. - Twee lege catch-blokken loggen nu (hostname-fallback in resolveSelfRef, container-map-refresh in resolveContainerByIp): stil geslikte fouten maakten een 'unknown source'-denial of een verkeerde self-ref lastig te diagnosticeren. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…nglish Comment-/testnaam-only sweep over everything this branch adds: port-relay.ts (incl. the FORWARDER_JS embedded comments), the new unit-test files, the socket-proxy/proxy/docker/api/index additions, and the e2e hunks this branch touched. No code, log strings, or assertion matchers changed; pre-existing Dutch from main outside this branch's hunks is left as-is. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
wmeints
left a comment
wmeints
left a comment
There was a problem hiding this comment.
Review
Verified locally on the PR head (6a78d33): tsc --noEmit clean, 249/249 unit tests pass.
Overview
Two changes that together fix #61 (Aspire persistent-container healthcheck stuck on Unknown):
- Read-path ownership → synthesized 404 (
socket-proxy.ts): inspect/logs/top/archive/stats of any container without a matchinghuddle.parentlabel now returns Docker's ownNo such container404 instead of a 403. Fixes DCP's inspect-before-create flow and closes an existence oracle (foreignvsmissingwere previously distinguishable). Classification lives in a pure, unit-testedownershipFromInspect(). - Port relay (
port-relay.ts, new): published ports of owned containers are mirrored onto the devcontainer's loopback via unix sockets in the already-shared mount plus a gateway-installed in-devcontainer forwarder. Includes refcounted network joins, a default-deny guard for unknown proxy sources, and buffered start/stop responses so relays exist before clients race to connect.
The design is sound and unusually well-reasoned in comments — every non-obvious decision (why 404, why buffer the start response, why dial timeouts, why refcounting) is justified in place. Checked: the com.intellij.devcontainer.id filter in initPortRelays matches the project's canonical devcontainer label, the 404 branch sits under GET/HEAD only so PUT /archive stays on the fail-closed mutation path, and wait correctly stays unbuffered (buffering it would deadlock).
Correctness issues (all minor)
- Network-ref leak race —
port-relay.tsrelayConnection(): therelaysById.get(containerId)guard against a concurrent teardown runs beforegatewayNetworks.acquire(). IfteardownContainerRelayswins the race in that window, it has already released its ref, and the connection then acquires a fresh ref that nobody will ever release — the gateway stays joined to the network anddocker network rmfails during Aspire cleanup, the exact problem the refcounting exists to prevent. Suggest re-checkingrelaysById.has(containerId)after the acquire and releasing if the entry is gone. - Global miss-refresh throttle —
docker.ts(lastMissRefresh): the throttle is global, not per-IP. Two containers starting within the same second means the second one's first proxy request can hit the default-deny 403 despite the container existing. Clients typically retry, but a per-IP timestamp map would remove the false negative cheaply. denyNotFoundwrites a body on HEAD —docker cpstats viaHEAD /containers/<id>/archive; a HEAD response must not carry a body. Harmless in practice because the connection is closed immediately, but worth a one-line guard.locksmap never shrinks —createNetworkRefTrackerkeeps one resolved-promise entry per network name forever; Aspire creates a network per session, so a long-running gateway accumulates entries. Trivial memory, but alocks.deletewhen the chain settles would tidy it.
Security
Net effect is stricter, as the PR claims:
- The foreign/missing collapse to 404 removes the existence oracle, and synthesizing the response (rather than forwarding) removes the TOCTOU window. The mutation verbs (
exec/start/stop/…) already return an identical 403 for both foreign and missing, so no oracle re-opens there. One residual: the devcontainer fast-path answers without a Docker round-trip, so devcontainer names are distinguishable from other foreign names by latency even though the bytes are identical. Theoretical; fine to leave, maybe worth a word in the comment. - The proxy default-deny for unknown sources is a genuine hardening — previously any reachable source could piggyback on global allow rules. Good regression test pinning all three contract points.
- The
:3000connection guard is defense-in-depth on top of the existing token auth. Note thatops.subnets()failure is swallowed (.catch(() => [])), silently disabling the guard for that network — the exposure is then limited to the two token-free endpoints (sudo-audit ingest, CA cert), which is acceptable, but aconsole.warnon that failure would match the diagnosability standard the rest of the PR holds itself to. assertSafeOwnerfires before any I/O on both public entry points (unit-tested), and ownership is re-checked on every relay connection against a fresh inspect — good.
Test coverage
Strong on the pure functions (ownershipFromInspect, extractRelaySpecs, resolveTarget, refcounting, ipInSubnet, dial timeout, owner-name guard) and the proxy default-deny contract. Gaps, all covered by the live e2e suite per the PR description but not by units: the buffered start/stop response path (openUpstreamBuffered ordering), the ownership re-check inside relayConnection, and waitForForwarderReady. Acceptable given the e2e coverage; the first would be the most valuable to pin down as a unit test since its ordering guarantee is what prevents the connection-refused race.
Nits
port-relay.tsduplicatesdockerRequestJson, andsocket-proxy.tsadds a near-identicaldockerGetStatus, both justified by import-cycle avoidance. Three copies is the point where a tiny dependency-freedocker-client.tsleaf module would pay for itself.- The PR description is in Dutch while the code comments were translated to English (
6a78d33) — fine for an internal repo, just inconsistent with the now-English code.
Verdict
Approve with the minor fixes above; only the network-ref leak race is worth addressing before merge since it silently defeats the cleanup guarantee the refcounting was built for. The rest are polish.
🤖 Generated with Claude Code
3 participants
Wat & waarom
Fixes #61 — Aspire SQL server healthcheck does not succeed (resource blijft op state
Unknown).Aspire's DCP inspecteert een persistent container (deterministische naam) vóór het 'm aanmaakt (inspect-before-create). De socket-proxy behandelde "container bestaat niet" hetzelfde als "van een andere devcontainer" en gaf een
403. DCP concludeerde daaruit dat de container bestond maar onbereikbaar was, maakte 'm nooit aan, en de resource bleef opUnknownstaan:Oplossing
1. Foreign/missing → 404 op de lees-tak (
20e7087,89adaa2)Vanuit een devcontainer bestaat elke container die het niet zelf heeft aangemaakt simpelweg niet. De lees-/inspect-tak (
GET .../json|logs|top|archive|stats) geeft nu Docker's eigenNo such container404 terug voor alles wat geenhuddle.parent == <devcontainer>heeft:foreignenmissingzijn niet meer te onderscheiden (voorheenforeign→403vsmissing→404), dus een devcontainer kan geen containernamen buiten zijn sandbox meer aftasten.Een probe op een écht bestaande vreemde container wordt voor de operator gelogd, maar de caller ziet nog steeds
404. De classificatie zit in een pureownershipFromInspect()met unit-tests. Voor bekende devcontainers zit er een goedkoop fast-path vóór de ownership-check (geen Docker-round-trip; review-opmerking Aikido) dat byte-voor-byte dezelfde 404 teruggeeft.2. Port-relay: gepubliceerde poorten van owned containers de devcontainer in (
d5b3dbe)Bij de e2e-repro bleek de healthcheck daarna alsnog niet groen te worden: Docker publiceert de SQL-poort op de loopback van de engine-host (
127.0.0.1:<port>), die binnen de devcontainer niet bestaat. Per gepubliceerde TCP-poort legt de gateway nu een unix-socket neer in de al gedeelde per-container mount; een kleine in-devcontainer forwarder luistert op127.0.0.1/::1:<hostPort>en pipe't naar die socket, en de gateway pipe't door naarcontainerIP:containerPortop het netwerk van de eigenaar. Relays worden gesynchroniseerd op start/restart en opgeruimd op stop/kill/remove; de start-respons wordt gebufferd tot de relay er staat, zodat DCP/Testcontainers-clients die direct na de start connecten geen "connection refused" racen.Security
Netto strikter dan het origineel op de lees-tak; er wordt geen nieuwe capability geopend. Mutatie-paden (create/start/exec/archive-upload/delete) zijn ongewijzigd en blijven fail-closed. De relay exposeert uitsluitend poorten die de devcontainer zelf via zijn eigen create-pad heeft gepubliceerd, en alleen ín die devcontainer.
Tests / verificatie
tsc --noEmitschoon; alle 157 unit-tests groen (o.a. pure classificatie-tests + port-relay-tests).huddle) leest als niet-bestaand zonder ownership-lek in het antwoord, en het devcontainer-fast-path geeft dezelfde 404.127.0.0.1:<hostPort>én[::1]:<hostPort>binnen de devcontainer. Relay verdwijnt bij stop/remove en komt terug bij restart (volgt de nieuwe dynamische poort).ContainerLifetime.Persistent):sqlserverwordt aangemaakt met labelhuddle.parent, DCP rapporteertstate=Running, en een echte TDS-handshake bereikt SQL Server via de DCP-proxy.🤖 Generated with Claude Code