feat(miner): enforce AMS network-egress deny-by-default + allowlist#8284
Merged
Conversation
…7857) Implements #7648's ratified default for the miner's Docker/Compose deployment path: dnsmasq + iptables/ipset (a well-established domain-allowlist pattern, not novel), deny-by-default OUTPUT with a curated allowlist punched through via dnsmasq's own ipset= directive on each DNS resolution. The whole container's egress is restricted (no per-attempt process isolation exists today), so the miner's own necessary calls (broker, Sentry, discovery-index, Neon) are allowed automatically whenever their corresponding feature is actually configured -- not a blanket allowance. The image now starts as root so the entrypoint can configure the firewall, then drops to the unprivileged node user via gosu before running anything else -- verified as byte-identical to prior behavior from that point on. Fails closed: a firewall-setup failure aborts the container rather than running untrusted code with no restriction. Verified empirically against a real built image, not just unit tests: scripts/verify-egress-firewall.sh builds the real image and asserts on real network calls -- default allowlist blocks/allows correctly, operator-declared ecosystems/extraHosts correctly extend it live, LOOPOVER_MINER_DISABLE_EGRESS_FIREWALL correctly disables it. All 7 checks pass. Scoped to Docker/Compose -- the primary documented self-hosting path. Kubernetes (blocked by a hard runAsNonRoot admission constraint) and systemd need their own privilege-setup mechanism, split into #8282.
![superagent-security[bot]](https://avatars.githubusercontent.com/in/3287076?s=60&v=4){kind=small}
superagent-security
Bot
added
the
pr:flagged
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8284 +/- ##
==========================================
- Coverage 92.16% 91.65% -0.52%
==========================================
Files 788 791 +3
Lines 79175 79245 +70
Branches 23933 23945 +12
==========================================
- Hits 72974 72633 -341
- Misses 5062 5524 +462
+ Partials 1139 1088 -51
Flags with carried forward coverage won't be shown. Click here to find out more.
|
loopover-orb
Bot
added
the
gittensor:bug
Contributor
|
Warning ⏸️ LoopOver review result - manual review recommendedReview updated: 2026-07-24 01:14:22 UTC
Review summary Nits — 2 non-blocking
Decision drivers
Context & advisory signals — never blocks the verdict
Review context
Contributor next steps
Signal definitions
🧪 Chat with LoopOverAsk LoopOver a question about this PR directly in a comment — grounded only in the same cached, public-safe facts shown above, never a new claim.
Full command reference: https://loopover.ai/docs/loopover-commands 🧪 Experimental — new and may change. 🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed 💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →. Checked by LoopOver, a quiet PR intelligence layer for OSS maintainers.
|
…erage, security review
- Regenerate packages/loopover-miner/docs/env-reference.md and
apps/loopover-ui/src/lib/ams-env-reference.ts (a separate generator from
the deployment-docs audit already checked locally; missed it the first
pass -- both now include LOOPOVER_MINER_DISABLE_EGRESS_FIREWALL and the
other new vars).
- generate-egress-firewall-config.ts's main() was 100% uncovered by patch
(real Codecov gap, not the local coverage-merge artifact the other two new
files showed): refactored to accept injectable IO, matching
scripts/check-miner-deployment-docs.ts's own main(env, io) pattern already
in this codebase, so it's exercised in-process rather than needing an
untestable subprocess. New tests cover missing-args, single-arg, success,
the underlying-failure catch path, and the real default-IO construction.
- Fixed both Superagent-flagged findings, verified empirically against a
real built image before and after:
- The entrypoint script and the root-invoked config-generation script were
node-owned despite executing with root privilege -- a compromised
node-level process (the exact thing this whole mechanism sandboxes)
could rewrite either to persist a root backdoor across a restart. Now
root:root. Scoped to these two files, not their full transitive
dependency graph -- documented as a deliberate, bounded scope in the
Dockerfile's own comment.
- dnsmasq's privilege drop is now explicit (--user=nobody --group=nogroup)
rather than relying on its own default. The scanner's specific claim
("running as root") was empirically false -- confirmed via /proc/<pid>/status
that dnsmasq already drops to nobody (uid 65534) unprompted -- and its
specific suggested fix (--user=node) would have been a regression (wider
privilege than dnsmasq needs, not narrower). Fixed the real gap
(implicit vs. explicit) without following the incorrect specific
recommendation.
Re-verified full unsharded npm run test:coverage (21177/21198 passing, 0
failures) and the real 7-check verify-egress-firewall.sh against a freshly
rebuilt image after both security fixes -- all still passing.
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
loopover-ui | 818f7ee | Commit Preview URL Branch Preview URL |
Jul 24 2026, 12:20 AM |
superagent-security
Bot
removed
the
pr:flagged
Bundle ReportChanges will increase total bundle size by 332 bytes (0.0%) ⬆️. This is within the configured threshold ✅ Detailed changes
Affected Assets, Files, and Routes:view changes for bundle: loopover-uiAssets Changed:
|
…irewall-config.ts Both are the same class of gap bin/loopover-miner.ts's own dispatcher tail already carries a v8-ignore for (see the packages/loopover-miner/bin note in vitest.config.ts's coverage.include): code only reachable by actually running the file as a script, not by importing it. main()'s own body is fully unit-covered in-process via the injectable-IO tests added earlier; only the self-invocation guard's true branch and one defensive non-Error fallback (unreachable given this call site's real, always-Error-throwing dependencies) remain, both marked consistent with the established convention. Confirmed locally: 100% line+branch coverage in isolation, full test suite green, real docker build + verify-egress-firewall.sh (7/7) still passing.
superagent-security
Bot
added
the
pr:flagged
…entrypoints
Owning only the entrypoint script and generate-egress-firewall-config.{js,ts}
left every module they import (ams-policy.js, egress-*.js, @loopover/engine,
and all of node_modules) node:node-owned. A compromised node-level process --
the exact threat this mechanism sandboxes against -- could tamper with any of
those, and root would load and execute the tampered code on the container's
next restart.
COPY --chown=root:root across the whole code tree closes that persistence
vector while staying world-readable (COPY --chown only changes ownership, not
mode bits), and the miner's real runtime writes all resolve under
LOOPOVER_MINER_CONFIG_DIR or a target-repo checkout, never /app -- verified
empirically that node can no longer write ams-policy.js but can still read
and require() it, and that the full verify-egress-firewall.sh suite still
passes end to end.
Contributor
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
superagent-security
Bot
removed
the
pr:flagged
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
1 participant
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.
Closes #7857 (#7648's ratified decision) for the miner's Docker/Compose deployment path -- the primary, documented self-hosting option. Kubernetes and systemd equivalents are split into #8282 (see Scope below), matching the same product-agnostic split pattern used for #8202/#8246 earlier.
Summary
No isolation mechanism existed today. AMS runs the coding-agent CLI subprocess (
claude/codex) as a barechild_process.spawn()sibling inside the same container as the long-running miner, no network namespace boundary. I researched this deeply before implementing (see #7857's own comment history) and confirmed: no per-process UID separation is available without a bigger restructure, so enforcement has to apply to the whole container.Mechanism
dnsmasq + iptables/ipset -- the standard, well-established Linux pattern for domain-based (not just IP-based) egress allowlisting, not something novel:
/etc/resolv.confrewritten to point at it -- confirmed empirically that Docker's own generated file allows this). Oneipset=/host/setnamedirective per allowed hostname: dnsmasq automatically adds the resolved IP to that ipset in-band with the real lookup, no separate polling needed to track IP rotation on CDN-backed hosts.OUTPUTpolicyDROPby default; loopback allowed; outbound DNS allowed only to the exact upstream resolvers dnsmasq itself forwards to (forces every lookup through the controlled resolver); egress allowed to anything already in the ipset, on standard ports.packages/loopover-miner/lib/egress-allowlist.tsresolves Decide default network-egress policy for AMS sandboxed execution (deny/allowlist, not open) #7648's three ratified categories (OS package registries, GitHub as the target-repo-git-remote proxy -- this product is GitHub-only, confirmed no other forge path exists -- and the operator's own declarednetworkAllowlist.ecosystems/extraHosts, already-shipped config surface from earlier work) plus the miner's own necessary platform hosts (Orb broker, Sentry, discovery-index, Neon), each gated behind whether that specific feature is actually configured on this instance -- not a blanket allowance.Privilege model change
The image now starts as root (was
USER node) so the entrypoint can configureNET_ADMIN-requiring iptables/ipset rules, thenexec gosu node loopover-miner ...drops privileges and replaces the process -- the actual miner command runs asnodefor its entire life, byte-identical to before this change from that point on.docker-compose.miner.ymlgrantsNET_ADMIN/NET_RAW(same capability class as this repo's existingtailscaleservice).Fails closed, deliberately (
set -euin the entrypoint): if firewall setup itself fails, the container aborts rather than silently running untrusted code with no restriction.LOOPOVER_MINER_DISABLE_EGRESS_FIREWALLis the documented escape hatch (decided in TypeScript via a no-op ruleset, not a shell-level skip, so it's testable and shows up correctly in the deployment-docs audit).Security review (Superagent), both findings verified empirically and fixed:
node-owned despite executing with root privilege -- a compromisednode-level process (the exact thing this mechanism sandboxes) could rewrite either to persist a root backdoor across a container restart. Nowroot:root. Scoped to these two files, not their full transitive dependency graph (documented as a deliberate, bounded scope in the Dockerfile's own comment).--user=nobody --group=nogroup) rather than relying on its own default. The scanner's specific claim ("running as root") was empirically false -- confirmed via/proc/<pid>/statusthat dnsmasq already drops tonobody(uid 65534) unprompted on this base image -- and its specific suggested fix (--user=node) would have been a regression (wider privilege than dnsmasq needs). Fixed the real gap (implicit vs. explicit) without following the incorrect specific recommendation.Verified empirically, not just unit tests
packages/loopover-miner/scripts/verify-egress-firewall.shbuilds the real Docker image and asserts on real network calls from inside a real running container with realNET_ADMIN. All 7 checks pass (re-verified after the security fixes too):ecosystems:[npm]declared.extraHostsaddition succeeds; an undeclared host still blocked even with other config present.LOOPOVER_MINER_DISABLE_EGRESS_FIREWALLcorrectly disables enforcement.Scope
Docker/Compose only. Kubernetes's
runAsNonRoot: trueis a hard, kubelet-enforced admission constraint -- a root-then-drop entrypoint is structurally impossible there; needs a k8s-nativeinitContainerinstead. systemd needs its ownExecStartPre=+mechanism. Both split into #8282 (blocked-by this PR) rather than bundled here -- each needs a genuinely different privilege-setup mechanism, not a copy-paste of the Docker entrypoint.k8s/miner-deployment.yamlandsystemd/loopover-miner.service.exampleboth carry a comment pointing at #8282.Tests
test/unit/miner-egress-allowlist.test.ts: default categories, every ecosystem resolves to real hosts, extraHosts, dedup (case-insensitive), and each platform host's gating condition (broker mode on/off, customORB_BROKER_URL, discovery-index/Sentry opt-in, Neon's all-or-nothing gate) -- 16 tests.test/unit/miner-egress-firewall-config.test.ts: dnsmasq/iptables text generation, custom upstream resolvers, invalid-hostname rejection (defense in depth, even though inputs are already validated upstream), the no-op disabled ruleset -- 14 tests.test/unit/miner-generate-egress-firewall-config.test.ts: the real.loopover-ams.yml-> resolved config -> written files pipeline against real temp-dir filesystem I/O, including the disable-flag path, file permissions, andmain()'s own CLI-entry logic (injectable IO, matchingscripts/check-miner-deployment-docs.ts's own established pattern in this codebase) across missing-args/success/failure/real-default-IO paths -- 11 tests.Note on coverage:
egress-allowlist.tsandegress-firewall-config.tsshow 100% line+branch coverage in the real Codecov report despite an artifact in my own local full-suite coverage run (a v8 coverage-merging quirk for files imported from multiple test files at very large scale -- confirmed via isolated local runs matching Codecov's real number, not my own local full-suite number).generate-egress-firewall-config.ts'smain()was a genuine gap on the first push (its CLI-entry logic wasn't exercised at all) -- fixed with real, dedicated tests via the injectable-IO refactor described above.Verified: full unsharded
npm run test:coverage(21177/21198 passing, only pre-existing skips, 0 failures -- run three times across this PR's revisions),tsc --noEmit --incremental falseclean,npm run build:minerclean,npm run test:miner-deployment-docs-auditclean,npm run miner:env-reference:checkclean,docs:drift-check/manifest:drift-check/branding-drift:checkclean,git diff --checkclean, realdocker build+ the 7-check verify script all passing (re-verified after the security fixes).Test plan
npm run test:coverage(full unsharded, 21177/21198 passing, 0 failures)tsc --noEmit --incremental falsenpm run build:minernpm run test:miner-deployment-docs-auditnpm run miner:env-reference:checkdocker build -f packages/loopover-miner/Dockerfile .(real build)packages/loopover-miner/scripts/verify-egress-firewall.sh(7/7 real network-behavior checks, re-verified after security fixes)git diff --check