Feat/69 export import rules rebased - #98
Open
insuT0ver wants to merge 43 commits into
Open
Conversation
insuT0ver
force-pushed
the
feat/69-export-import-rules-rebased
branch
from
August 8, 2026 18:51
90d7cdf to
842600b
Compare
insuT0ver
added a commit
that referenced
this pull request
Aug 8, 2026
- cli(init): build the gateway 'docker run' as an argv array (execFileSync, no shell) instead of interpolating config-writable team-folder paths into a shell string — closes host command injection via firewallRulesFolder/extensionsFolder (also covers 'huddle restart', which reuses runInit). Adds gatewayEnvArgs(). - api(import): in 'replace' mode with an explicit ?container scope, delete that scope even when the document is empty (empty replace no longer fails open). - api(import): call ensurePathModeMarker() for every imported path-scoped rule so imported path rules are actually admitted over HTTPS CONNECT. - firewall-groups(reload): validate every folder file BEFORE clearing the live policy; abort and keep the last-good state if any file fails to parse. - firewall-groups(import): a team-folder reload no longer hijacks manually-created groups/rules into startup-folder state (which the next reload would delete). - ui(bulk): global allow/deny now confirms before applying (firewall + container-detail); bulk flip preserves each rule's expires_at so temporary rules don't silently become permanent. - db(getGroup): select explicit columns; api: log swallowed startup reload error; settings: the extensions folder button is 'Save' (it saves, not reloads).
insuT0ver
added a commit
that referenced
this pull request
Aug 8, 2026
- firewall-groups(reload): apply the folder reload ATOMICALLY — clear the previous startup-folder state and import all files in one transaction, so an import-time collision (e.g. a folder group name clashing with a manual/huddle group) rolls everything back and preserves the last-good policy instead of half-clearing it. - cli(init): convert the remaining docker calls (pull/volume/network/rm/connect + podman machine ssh) to argv via execFileSync — init no longer builds any shell command string, clearing the SAST command-injection finding. - db: the seeded 'huddle' group uses source='manual' (not the ad-hoc 'system') to match the documented source values. - cli(firewall): extract a shared readJsonFile() helper for the two import paths.
…port (#69) Export/import rules as JSON via API, CLI and UI; team-managed rules folder (config-mounted) with reusable firewall groups; share the firewall+groups panel across views and show all rules; path-mode export/apply fix. Includes English translation of comments/messages and Aikido review fixes.
…de examples Pending inbox (firewall page): - checkbox per request + "select all" and a bulk toolbar with the same operations as a single request: Allow / Allow global / Deny / Deny global / Dismiss, applied to all selected at once. Allowed / denied / path-mode rules (groups panel): - checkbox per row + "select all", scoped to the currently visible rows. - bulk actions: Export, Add to group, Path mode, Flip allow/deny, Delete. - Export was implemented but had no UI trigger (only per-group export existed, gated behind selecting a group) — bulk Export now makes exporting any rules possible. It builds the standard JSON envelope client-side and, for a selected path-mode domain, also includes its allowed sub-paths so the export is self-contained. Examples: - nodejs.json: nodejs.org is a dependency-download host, so switch it to path mode (blocked at root, only /dist/* and /download/* allowed) instead of allowing the whole host — consistent with npm-registry.json. - README: document the general-URL (allow) vs dependency-URL (path mode) distinction as guidance for writing groups. No backend changes — all actions use existing endpoints.
…detail page The container-detail firewall tab lists that container's pending requests but had no bulk actions (only the standalone Firewall page did). Add the same checkbox + select-all + bulk toolbar (Allow / Allow global / Deny / Deny global / Dismiss) so pending triage is consistent across both views.
- Export button is no longer disabled when no group is selected. With a group it
exports that group; otherwise it exports the current view (All rules /
Ungrouped, honouring the status filter + search) as the flat rules envelope
(path-mode domains include their allowed sub-paths). Shared rulesEnvelope()
helper reused by bulk export.
- Seed a dedicated 'huddle' group and file the gateway's own self-traffic rule
('huddle' domain) under it, so it is separated from user/team rules. Idempotent.
…pe pie bulk Pending requests are now two labelled sections — Domain requests and Path requests — each with its own select-all and its own bulk control. The bulk control is the same radial pie menu used on a single request (pieConfig for domains, pieConfigPath for paths), not plain buttons: selecting rows and picking a pie action applies it to every selected row of that type at once (allow / temp / deny / global / dismiss / path-mode for domains; allow / prefix / deny / dismiss for paths). Applies on both the Firewall page and the container-detail firewall tab.
Every github.json host is now a path-allowlist (blocked at root) instead of a blanket allow, matching the dependency-URL guidance: github.com / api / raw / codeload / ghcr are scoped to an example 'infosupport' org (edit to your own), and the opaque asset hosts (objects/assets) allow their functional path prefixes. README table + guidance updated.
- Remove the Type, Added and Added-by columns; simplify Actions to just the Allowed/Denied (or Path mode) status pill. Columns now: checkbox, Domain/path, Path mode, Match, Actions, Group, row menu. - table-layout: fixed with header-defined column widths, so widths no longer re-flow per row content (rows stay aligned and stop jumping when the list or filter changes); the Domain column takes the remainder and ellipsizes. - Wrap the table in an overflow-x container so it scrolls instead of spilling past the card on narrow widths.
The static 'Root' Match column was redundant once the Actions column was simplified. Columns are now: checkbox, Domain/path, Path mode, Actions, Group, row menu (colspans + widths updated).
Add a 3-step Quick start (install CLI → huddle init → huddle) right after the hero, so installing/running Huddle is visible in the first screenful instead of buried below the What/Why/Architecture/Features sections. Links down to the full Getting Started guide for Rancher Desktop, runtimes and base images.
Use the GitHub <picture> pattern (dark <source> + light <img> fallback) for the three hero images so each renders for the reader's color scheme. Adds the -light/-dark variants and drops the superseded single-mode PNGs.
- cli(init): build the gateway 'docker run' as an argv array (execFileSync, no shell) instead of interpolating config-writable team-folder paths into a shell string — closes host command injection via firewallRulesFolder/extensionsFolder (also covers 'huddle restart', which reuses runInit). Adds gatewayEnvArgs(). - api(import): in 'replace' mode with an explicit ?container scope, delete that scope even when the document is empty (empty replace no longer fails open). - api(import): call ensurePathModeMarker() for every imported path-scoped rule so imported path rules are actually admitted over HTTPS CONNECT. - firewall-groups(reload): validate every folder file BEFORE clearing the live policy; abort and keep the last-good state if any file fails to parse. - firewall-groups(import): a team-folder reload no longer hijacks manually-created groups/rules into startup-folder state (which the next reload would delete). - ui(bulk): global allow/deny now confirms before applying (firewall + container-detail); bulk flip preserves each rule's expires_at so temporary rules don't silently become permanent. - db(getGroup): select explicit columns; api: log swallowed startup reload error; settings: the extensions folder button is 'Save' (it saves, not reloads).
- firewall-groups(reload): apply the folder reload ATOMICALLY — clear the previous startup-folder state and import all files in one transaction, so an import-time collision (e.g. a folder group name clashing with a manual/huddle group) rolls everything back and preserves the last-good policy instead of half-clearing it. - cli(init): convert the remaining docker calls (pull/volume/network/rm/connect + podman machine ssh) to argv via execFileSync — init no longer builds any shell command string, clearing the SAST command-injection finding. - db: the seeded 'huddle' group uses source='manual' (not the ad-hoc 'system') to match the documented source values. - cli(firewall): extract a shared readJsonFile() helper for the two import paths.
The flat rules import already sets the host-only path_mode=1 marker, but the group import (importGroupEnvelope) and applyGroup inserted path-scoped rules without it — so an imported/applied group whose path rules lacked an explicit marker was denied at HTTPS CONNECT (Aikido MED, api.ts:583). Both paths now call ensurePathModeMarker() for every (domain, scope) that received a path rule, mirroring the flat import and single-rule create. Idempotent — a marker already in the envelope/members is left as-is. Test updated for the auto-marker.
Row menu stopped opening: `.grp__table td { overflow: hidden }` (specificity
0,0,1,1) beat the `.grp__row-menu { overflow: visible }` override (0,0,1,0), so
the menu was clipped inside the 40px cell — and the `.grp__table-wrap`
overflow-x:auto wrapper (which forces overflow-y:auto) clipped it again on the
last row. Render the menu position:fixed, anchored to the button, so no ancestor
overflow can clip it; close it on outside-click / scroll / resize.
Add write-back to the team-managed rules folder (app -> files): a "Sync to
folder" button (POST /api/firewall-rules-folder/sync; CLI `huddle firewall
folder sync`) writes every group out as <slug>.json, mirrors the current set
(prunes envelope files of deleted groups, leaves unrelated files alone), and
re-tags synced groups source='startup-folder' so a later reload updates them in
place instead of aborting on the "don't overwrite a manual group" guard. The CLI
now mounts the firewall-rules folder :rw (was :ro).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… column Combine the "Path mode" and "Actions" columns into one "Action" column: allow/deny rules show their Allowed/Denied pill, path-mode domains show the path control (all/specific select + expandable allowed-paths editor) in its place. Replace the freed column with a "Container / Global" column showing each rule's scope. Allow<->deny and path-mode toggling stay in the bulk actions. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The bulk pie-menu in the pending-requests groups sits flush right (margin-left:auto) with no right margin, so its radial fan-out spilled past the card edge on hover. Give it the same 56px right margin the per-row pie already uses, which also aligns it with the row pies below. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- Ability to flag one of the folders as context, in which it will automatically create the workspace - created and tested for a VS code workspace
…oot, drop context stub - Multi-folder mounts now map each host folder to a chosen absolute container path (host -> container) instead of a name under /workspaces/<name>. - Add an explicit 'Open IDE at' project root (containerWorkspace), auto-suggested from the common parent of the mount targets. - Remove the AI-context flag and the dynamic stub CLAUDE.md across frontend, gateway (api.ts/docker.ts) and CLI; a static project-level CLAUDE.md replaces it. - Modal remembers the last-used layout via localStorage. - CLI: --mount host=container (repeatable) + --workspace-root; drop --context. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Container list (/containers), the dashboard 'Recent Containers' card and the Docker permissions dropdowns labelled each entry with presentableName (the workspace directory leaf), so containers started from the same directory were indistinguishable. Show the always-unique container name instead. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Both pending inboxes now order by last_seen descending so the most recent request is on top: domain requests (previously alphabetical by domain via the detail endpoint's ORDER BY status, domain) and path sub-requests (previously grouped alphabetically by domain). Sorted client-side, leaving the allow/deny sections and the shared buildPathDomains helper untouched. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…card Same clipping as firewall.component (1112802), now in the container view: the bulk pie in the pending-requests group sits flush right (margin-left:auto) with no right margin, so its radial fan-out spilled past the card edge. Give it the same 56px right margin the per-row pie already uses. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Aikido flagged one logic bug and three code-quality issues on this PR. Logic bug (gateway/src/api.ts) — the multi-mount IDE project root fell back with `commonParentPath(...) || '/workspaces'`, but commonParentPath() returns '/' (truthy) when the mounts share no leading segment, so the documented '/workspaces' default was unreachable and such containers opened at '/'. The helper plus the fallback now live in gateway/src/workspace-root.ts — pure path arithmetic, no Fastify/DB/Docker to load — and workspace-root.test.ts pins both the shared-parent case and the fallback. Duplicated validation (gateway/src/db.ts) — validateGroupKeys repeated validateFolderMappingKeys verbatim (allowlist, unknown-key detection, throw, filtered return). Both now delegate to one generic validateUpdateKeys(), so the SQL-identifier allowlist that closes finding #9 has a single implementation. Single responsibility + function length (gateway/src/firewall-groups.ts) — the module mixed envelope validation, DB persistence and folder file I/O, and syncGroupsToFolder did discovery, filename selection, writing, DB retagging and pruning inline. Split by responsibility: firewall-group-envelope.ts envelope shape + fail-closed validation (pure) firewall-group-store.ts DB reads/writes for groups and their rules firewall-rules-folder.ts the team folder on disk (reload + sync) firewall-groups.ts stays as the public surface (re-exports only), so api.ts and the existing tests are untouched. Both long functions are now composed of named helpers (pickGroupFile, writeGroupFile, pruneOrphanEnvelopes, parseEnvelopeFiles, applyParsedEnvelopes), and the member-rules query that export and apply share is one memberRulesSql() so the two can no longer disagree on a group's contents. Behaviour-preserving throughout: gateway suite green (364 tests, 23 files), including the firewall-groups cases covering merge/replace import, apply, folder reload and folder sync. The fourth Aikido comment (cli/src/migrate.ts parseYaml) is about code already merged into main — after the rebase that file is no longer part of this PR's diff. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`rules.json` in the repo root was a `huddle firewall export` dump of the author's own installation — 583 rules including per-container entries (devcontainer-socialekaart) and internal IPs — and `localhost_3000_ (6|7).png` were browser screenshots. Nothing references any of them; the shareable rule sets live in examples/firewall-rules/. .gitignore now covers both patterns so running the README's `huddle firewall export --out rules.json` from the repo root can't commit local policy into a public repo by accident. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
insuT0ver
force-pushed
the
feat/69-export-import-rules-rebased
branch
from
August 17, 2026 07:40
4fbdf29 to
b81a7ea
Compare
insuT0ver
force-pushed
the
feat/69-export-import-rules-rebased
branch
from
August 17, 2026 07:58
2e46cb8 to
b81a7ea
Compare
…-ups Four findings from Aikido's scan of b81a7ea. **containerWorkspace reached a root shell unvalidated (HIGH).** api.ts checked only that the value started with '/', then docker.ts interpolated it into the setup script it runs as root via `sh -c` (PROJ="…", then mkdir -p / chown -R / chmod -R). `/workspaces/x"; touch /tmp/pwned; #` therefore executed as root inside the new container, and a bare `/` aimed `chown -R vscode:vscode` at the whole container filesystem — bypassing the non-root/sudo-grant boundary the rest of Huddle enforces. Two independent layers now: - containerPathError() (workspace-root.ts) rejects non-absolute paths, the '/' root, '.'/'..' segments, control characters, quotes/backticks/$/backslash and absurd lengths. api.ts applies it at one choke point covering all three sources of the value (explicit override, common parent of the mounts, single-mount leaf) and to each mount's containerPath. - shQuote() single-quotes the value where docker.ts substitutes it, and both scripts now reuse "$PROJ" instead of interpolating the path three more times, so the quoting happens in exactly one place. Verified against a real `sh`: every payload above round-trips as a literal string and executes nothing. The single-mount path (`/workspaces/${leaf}` derived from workspaceDir) had the same weakness and predates the multi-mount work; it goes through the same check. **SQL built by string interpolation (MEDIUM).** The member-rules query I de-duplicated in the previous commit passed a column list into a template, which put a constructed string into db.prepare(). It is one literal MEMBER_RULES_SQL again, still shared by export and apply; apply's narrower DISTINCT (it re-targets every rule at one scope, so container_id is irrelevant) now happens in memberRulesForScope() and reproduces the previous SQL semantics exactly. **Unbounded reads following symlinks (MEDIUM).** reloadFirewallRulesFolder() runs during API startup and read every *.json with a plain readFileSync, so `evil.json -> /dev/zero` in the team folder hung the gateway rather than one request, and a symlink out of the folder read files the operator never put there. readEnvelopeFile() now lstats first, refuses anything that is not a regular file, caps size at 5 MiB, and asserts the name is a bare basename. **SELECT * in getGroupByName.** Columns listed explicitly, as getGroup already did. Tests: 373 pass (24 files), up from 364 — containerPathError, shQuote against the documented shell semantics, and folder-reload rejection of a symlinked and an oversized group file. Note the symlink case aborts the reload and keeps the last-good policy, consistent with how the module already treats an unreadable file; that is a deliberate behaviour change for symlinks, which previously loaded. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… mount docs **Aikido: "file inclusion via readFile" on the remaining fs sinks.** The read side already asserted its file name was a bare basename; writeGroupFile() and pruneOrphanEnvelopes() still joined onto the mount themselves. All three now go through envelopePath(), the single place in the module that turns a name into a path, and it rejects anything that is not a plain basename. No taint path existed — names come from readdirSync() or from groupFileSlug(), which strips to [a-z0-9-] — but a group name is attacker-influenceable via the import API, so groupFileSlug() was the only thing standing between it and a write outside the folder, implicitly. Now it is asserted at the sink, and a new test pins it: a group literally named "../../etc/evil" syncs to etc-evil.json inside the mount and creates nothing above it. **Aikido: CLI README documents a syntax the parser rejects.** The multi-folder docs still described the earlier design — `--mount <name>=<path>` mounting under `/workspaces/<name>`, plus a `--context <name>` flag writing a `CLAUDE.md` stub. What ships is `--mount <host>=<container>` with an absolute container path (cli/src/index.ts parseMountFlag, cli/src/start.ts validateContainerPath), and `--context` is not parsed anywhere; following the README failed immediately with an invalid mount path. Rewrote both the prose and the flag list to match the implementation, and documented `--workspace-root`, which is what actually chooses the IDE project root and was missing entirely. Every --mount/--workspace-root value in the file was checked against the shipped parser's rules. Tests: 374 pass (24 files). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… the DB The Settings page was half-and-half: the team-managed folders already lived in the CLI config (~/.huddle/config.json, mounted read-write into the gateway), while the resource limits and the folder mappings were still rows in SQLite — invisible to the team, not reviewable in version control, not hand-editable. Both now live in that same config file, which becomes the single source of truth for everything on the Settings page: - host-config.ts grows a generic merge-write (write-then-rename, so a crash cannot truncate the file the CLI boots from) plus typed accessors for the resource defaults and full CRUD over the folder mappings. Entries are stored camelCase with real booleans, since humans edit this file; every field is coerced on read, so a hand-mangled entry degrades to a harmless default instead of reaching the Docker mount spec as `undefined`. - The HTTP shape is unchanged (snake_case, 0/1 flags), so the portal is unaffected by where the mappings are stored. The fail-closed field allowlist (finding #9) is kept: the keys no longer reach SQL, but a config file is no dumping ground for arbitrary client keys and the portal still relies on the 400 for a typo'd field. - Neither setting needs a remount: the gateway reads the file when it creates the next devcontainer, so an operator edit applies immediately. Only the folder *paths* still require `huddle restart`. - Existing installs are migrated once at startup (settings-migration.ts). Non-destructive — the legacy rows stay put; the presence of the `folderMappings` key marks the config as owner, so clearing every mapping in the portal does not resurrect the DB rows on the next start. Deferred and retried when the config is not mounted yet. - The portal now warns up front when the config is not mounted, instead of silently reporting "Saved" for a write that went nowhere. folder-mapping.test.ts no longer needs the sqlite probe (host-config has no native binding), so it runs in a fresh DMZ devcontainer too.
… them The portal runs in a container and cannot see the host filesystem, so every host path — the workspace to start, a folder mapping, the team-managed rules folder — had to be typed from memory and be right the first time, Windows notation and all. `huddle indexfolder` scans a folder on the host and stores what it finds, giving the gateway a list of folders that demonstrably exist. Deliberately a snapshot, not a live view: typing a path by hand keeps working, because a folder created after the last scan must not become unreachable. - CLI: `huddle indexfolder [path] [--depth N] [--list] [--clear]`, running on the host where the folders actually are. - Gateway: /api/indexed-folders to list, add, delete one, or clear a subtree. - host-path.ts is the one choke point for Windows notation: backslashes, drive letter case and trailing slashes collapse to a single spelling, so `T:\proj\` and `t:/proj` dedupe to one entry instead of two. Stored in SQLite rather than config.json on purpose: this is a scan of THIS machine, not team configuration that belongs in everyone's config. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…g them With the index in place, every host-path field grows a Browse… button that opens a real folder dialog on what `huddle indexfolder` found: a tree for structure, an icon view for scanning, search, breadcrumbs, and one folder as the answer. The text box stays authoritative — the index is a snapshot, so a folder made five minutes ago is still reachable by typing it. The hierarchy is rebuilt client-side from the flat paths (folder-tree.util.ts): no extra API, both Windows spellings merge case-insensitively the way the index dedupes them, and a folder that only exists as the parent of an indexed one is shown and selectable — it demonstrably exists on the host. Picking several folders is a first-class answer, not an error: - Ctrl/Cmd-click adds, Shift-click takes the range as drawn (never reaching into a collapsed branch), plain click replaces. - In the start dialog every picked folder becomes its own mount row with /workspaces/<name> filled in. Pick several while the dialog is still in single-folder mode and it switches to multi-folder mode itself, instead of making you back out, tick the checkbox and pick them all over again. - Fields where several paths mean nothing — folder mappings, the team-managed folders — stay single-select. Settings gained an "Indexed folders" manager at the bottom of the page: search, breadcrumbs, a table with checkboxes and indent guides, and a selection bar that removes each chosen folder with its subtree in one request. Removing only shrinks the index; the folder itself and any mapping pointing at it stay untouched. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…t itself The field never did what it promised: whatever you typed there was sent as containerWorkspace, and the IDE still opened somewhere else — so it was one more thing to fill in, wrong, on every multi-folder start. The gateway already has the answer without asking. When no containerWorkspace comes in, defaultMultiMountWorkspace() takes the deepest directory the mount targets share and falls back to /workspaces when they share nothing — exactly what the field auto-suggested anyway. So stop sending it from the portal and let that path be the only one. The CLI's --workspace-root still works for the rare case where the root has to be something other than the common parent; the API is unchanged.
A click event is delivered to the common ancestor of where the button went down and where it came up. Press inside the dialog — selecting a path in an input, dragging a scrollbar — and release anywhere over the backdrop, and that click landed on the backdrop: the modal closed and everything typed was gone. Bind the backdrop (and the box's stopPropagation guard) to mousedown instead. Now only a press that starts outside the box closes it; where the button comes up no longer matters.
The JetBrains backend was started with its stdout/stderr redirected to "$PROJ/rider-client-diagnose.log" — a leftover from debugging the Rider client. $PROJ is the user's project root, so every devcontainer start dropped an untracked log file in their repo, ready to be committed by accident. Point it at /tmp/huddle-ide-backend.log instead: same diagnostics, reachable with `docker exec`, but nothing lands in the mounted workspace.
The number was allow / (allow + deny) over the container's firewall rules, which reads as a security grade but points the other way: a container where you denied most requests scored red, while one that got everything it asked for scored green. Nobody can act on that, so the column goes rather than gets recoloured. Also removes Container.securityScore — the same idea as a model field, never set by the gateway and never read.
The gateway (portal edits) and the CLI on the host write the same config.json, and both did an unlocked read-modify-write of the whole document. Whichever landed last silently reverted the other's change: an operator could lose a folder mapping or a resource default with no error anywhere, and find the next devcontainer starting with stale or missing mounts. Both sides also shared one `.tmp` path, so two writes could truncate each other's half-written file. Both sides now take the same lock — an exclusive create of a sibling `config.json.lock`, which works across the bind mount that joins the container to the host — read inside it, and write through a temp file that is unique per write. A lock left behind by a writer that died mid-update goes stale after 10s so one crash cannot wedge the file. The CLI's whole-document `writeConfig` is gone; `updateConfig` merges a patch inside the lock, which is what every caller wanted anyway. Aikido: shared config rewrites can silently drop concurrent updates.
`replace` is documented as subtree-scoped, but `root` was optional and an absent one fell through to `clearIndexedFolders(undefined)` — which empties the whole index. Any caller that sent `replace: true` without a usable root turned a re-index of one project into total index loss, and the operator got to rebuild every browseable host folder by hand. Refuse with a 400 instead. Wiping the index stays the DELETE endpoint's job, where it is explicit. Aikido: indexed-folder replace requests wipe the whole index when root is omitted.
`skipped` was set both when config.json is not mounted yet and when the write to it failed, but startup always logged the first reason. An operator chasing a failed write was told to mount a file that is already mounted. Carry the reason on the result and log the one that applies. Aikido: skipped can report the wrong cause for the executed branch.
The legacy reader returns a typed row shape but selected `*`, so a future column would silently change what the migration maps. Aikido: no SELECT *.
The depth of the folder tree is the segment depth of an indexed host path, and nothing on the write side bounds that — so every recursive walk over it let a path decide how deep our call stack goes. A 50k-deep path took `flattenNodes`, `findNode` and `folderRows` down with a RangeError; the regression tests build exactly that. `descendants` (folder picker) and `allNodes` (settings) were the same function twice over; both now call the shared `flattenNodes`, which walks an explicit stack. `subtreeMatches` keeps its own stack so it can still stop at the first hit — it runs once per node per keystroke. Aikido: recursive descent without depth protection (3x).
parseYaml did tokenizing, feature rejection, depth limiting and both mapping and sequence parsing in one function, with four nested helpers sharing a `pos` closure — so nothing inside it could be reached from a test on its own. The position moves into an explicit Cursor and the parse steps become module-level functions, one job each. parseYaml is now the four lines that read: tokenize, reject, parse, coerce. No behaviour change; the existing parser tests cover it. Aikido: long function.
The lock added in the previous round only covered the merge inside updateHostConfig. The folder-mapping CRUD still listed the mappings first and then handed that snapshot in as the patch, so the whole `folderMappings` array it wrote was pre-lock state: a mapping added by another writer in between was merged away, and two concurrent creates both picked the same max+1 id. mutateHostConfig moves the read-modify-write itself under the lock — the mutator gets the file as it is at that moment and returns only the keys to change, or null to abort. create/update/deleteFolderMapping and the settings migration (which likewise decided what to adopt from a pre-lock read) now go through it; updateHostConfig stays as the thin wrapper for values that do not depend on what is already in the file.
Warning and writing anyway reintroduced exactly the clobber the lock exists to prevent: the gateway holding it is mid read-modify-write of the same document, so one of the two edits disappears — and a warning scrolls past in a command that reports success. updateConfig now throws with the path, the timeout and how to clear a stale lock. Every caller aborts before writing anything, and the CLI's top-level handler prints it as a plain error the operator can retry. The wait goes to 5s (the gateway holds the lock for microseconds, so this is headroom, not latency). The one caller that writes from a catch block — the experiment-activation rollback — swallows a failure there so it can never mask why activation failed.
… node folderRows walked each node's subtree to see whether it held a hit, and then walked every child's subtree again to decide whether to open the branch. Each node was therefore re-read once per ancestor: quadratic in the depth of the tree, on every keystroke in the folder filter, on the deep trees `huddle indexfolder` produces. subtreeHits marks the whole tree bottom-up in a single pass — every node tested once, then only looking at children that are already decided — and folderRows reduces to two Set lookups. The deep-tree filter test goes from 5.000 back to the same 50.000 levels as its neighbours, which the old shape could not finish inside the timeout.
Two related invariants that were true but never stated. resolveScanRoot turns whatever comes off the command line (or the shell's cwd) into one absolute, `..`-free path before anything touches the filesystem, and rejects a null byte with a sentence instead of an ERR_INVALID_ARG_VALUE stack trace; the walk, the containment check and the paths posted to the gateway all derive from that single value. `contains` then asserts what the breadth-first walk already relied on: no path outside the folder the operator named is indexed or handed to the next readdirSync. Entries come from readdirSync and symlinks are skipped, so it should never fire — it is there so a future change cannot quietly let a scan escape its root.
The lock was an ownerless path: a contender unlinked whatever file sat there once it was older than LOCK_STALE_MS, and a releasing writer unlinked the path again without checking it still owned it. A writer that stalled past the threshold therefore deleted the lock of the contender that had taken over, letting a third writer in alongside the second — reopening the lost-update race that the in-lock folder-mapping read and the fail-closed CLI path were meant to close. The lock file now holds a token identifying its holder (pid plus a uuid; a pid alone is meaningless across the bind mount that joins the gateway container to the host CLI). releaseLock only unlinks while the on-disk token is still its own, and breakStaleLock re-reads the token and mtime immediately before removing, so a lock another contender has already broken and replaced is left in place.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Related issue
Fixes #
Type of change
fix)feat)docs)refactor/chore)Checklist
mainand focused on a single change.npm --prefix gateway test) and I added/updated tests where relevant.Notes for reviewers
Reviewer guide
This guide explains how to review PR #98 by exercising the new functionality. It can be removed or archived after the PR is merged.
What changed
Area | Before | After -- | -- | -- Firewall policy | Configured manually on every machine | Exportable/importable JSON, reusable groups, and a team-managed rules folder loaded at startup Devcontainers | One host folder per container | Multiple --mount options, with --workspace-root controlling where the IDE opens Host paths | Entered manually, including Windows path notation | Indexed once and selectable from the portal Settings | Split between config.json and SQLite | Consolidated in ~/.huddle/config.json Docker Desktop on Windows | Containers failed to start (#93/#99) | SupportedThis PR is best reviewed by using it, rather than only reading the code. The sections below are independent, so feel free to test one area and leave your findings as a review.
Return to the stable version afterwards:
Testing on a Windows host is especially valuable because several changes address differences between the Windows host and the Linux gateway.
1. Team-managed firewall rules (#69)
Export and restore a policy:
Verify that:
The exported JSON is readable and suitable for committing to a repository.
Deleting a rule in the portal and importing the file restores it.
--replacemirrors the imported policy instead of merging it.Groups work through Portal → Firewall and through:
Test the team-managed rules folder:
Place a file from
examples/firewall-rules/in that folder, such as the GitHub, npm, Node.js, or OpenAI rules, and restart Huddle:The rules should become active.
folder syncshould write your current rules back to the folder.Most important test
Add a broken file to the rules folder—for example:
Invalid JSON
A group name that conflicts with a manually created group
Restart Huddle and verify that the existing firewall policy remains completely unchanged.
The complete folder must be validated before anything is cleared, and all changes must be applied in one transaction. An invalid file must never leave the environment partially firewalled.
Also verify that:
Bulk allow and deny actions ask for confirmation.
Bulk flipping a temporary rule preserves its expiry time.
2. Multi-folder devcontainers
Example:
Verify that:
Both folders are mounted.
The IDE opens at
/workspace, rather than inside one of the mounted folders.VS Code receives a generated workspace file.
Without
--workspace-root, the common parent of the mounts is used.Mounts without a shared parent, such as
/aand/b, fall back to/workspaces—never/.Starting a normal single-folder container without additional flags behaves as before.
Invalid container paths should be rejected with a clear error before a container is created, including:
foo/bar/Paths containing quotes
Paths containing
$These values previously reached a root shell without correct quoting.
3. Indexed host folders
Index a folder using:
Optional flags:
Verify that:
Indexed folders appear under Portal → Settings → Indexed folders.
Indexed folders can be selected from the start-container dialog.
Manually entering an unindexed path still works. The index is a convenience snapshot, not an allowlist.
C:\projects\andc:/projectsresolve to a single entry.Indexing a drive root stops at 1,500 folders and displays a warning.
node_modulesanddistare excluded unless--allis used.Filtering a large index remains responsive while typing.
4. Settings in one file
Settings are now stored in:
Verify that:
Changing resource limits or a folder mapping in the portal updates the file.
Editing the file manually and restarting Huddle updates the portal.
Corrupting one mapping causes that entry to fall back to a default without preventing the portal from loading.
Existing SQLite mappings and limits are migrated automatically on the first start.
A value already present in
config.jsontakes precedence over its SQLite value.Concurrent-write test
While saving a folder mapping in the portal, run:
Both changes should survive.
If the CLI cannot acquire the configuration-file lock, it must fail loudly without changing the file. This is one of the least visible but most fragile parts of the PR and has already gone through several review rounds.
5. Docker Desktop on Windows (#93/#99)
Start a container using Docker Desktop for Windows. That is the primary test.
If possible, also confirm that container startup still works with:
Native WSL2
dockerdPodman
6. Smaller changes
Please review any of these while exercising the main functionality:
Slimmer firewall tables
Row menus are no longer clipped
Pending rules are split into Domain and Path, each with separate bulk actions
Containers are labelled using their unique name ([Bug]: Multiple containers for the same directory dont have unique names in the docker permissions dropdown #103)
Modals close on
mousedown, preventing a dragged selection from accidentally discarding entered dataIDE backend logs no longer appear in the project folder
README quick start and light/dark screenshots
Corrected CLI documentation for
--mountand--workspace-rootRegression pass
If you have the required setup, please test:
A normal single-folder container in IntelliJ or Rider
A normal single-folder container in VS Code
Approving and denying firewall rules through the portal
Approving and denying through
huddle firewall list -iExpiring firewall rules
huddle migratePodman
Native WSL2
dockerdAutomated verification
The following checks are currently green:
274 gateway tests
24 portal tests
Type checking
Gateway and portal builds
New test coverage includes:
Rules-folder reload rollback
Firewall-group round trips
Indexed-folder API
Windows path normalization
Workspace-root fallback
Container-path validation
Shell quoting against a real
shSQLite-to-
config.jsonmigrationConcurrent
config.jsonwritesKnown and accepted limitations
Two scanner findings for
huddle indexfolderare intentionally ignored. The command scans the folder explicitly provided by the local user, using that user's permissions. Applying the suggested restriction would reject valid commands such as:The
config.jsonlocking mechanism also retains a microsecond-scale collision window. Fully eliminating it would require a locking primitive that works reliably between a Windows host and a Linux container over a shared bind mount.In the unlikely worst case, one Settings edit could be lost. The missing change would remain visible to the user and could be submitted again.