Remove secondary-disk discovery and whole-disk ZFS creation from the
installer. New installations now always create a sparse ZFS loop-file
pool from free space on /, while existing vpsmgr pools are still adopted.
The installer no longer scans, selects, formats, or modifies devices such
as /dev/vdb, including virtual ISO/CD devices.
Update storage documentation and keep the existing reclaim, pool sizing,
and ZFS quota behavior.
Co-authored-by: CommandCodeBot <noreply@commandcode.ai>
The script accidentally contained two ask_ipv6 definitions. Bash used the
second legacy definition, so the new detector/globalping flow was silently
overridden and installations jumped straight to manual prefix input.
Keep only the detector-based implementation.
Co-authored-by: CommandCodeBot <noreply@commandcode.ai>
Switch the fixture to the 2001:db8:: documentation block so the
workspace carries no real-format provider prefix.
Co-authored-by: CommandCodeBot <noreply@commandcode.ai>
Pool mode was still computing the address with IPv6Addr (the
prefix-mode hash derivation) and showing the /112 block line. Now the
user panel reads the assigned address from the DB (users.ipv6_address)
and shows the bare address only — no block line. Prefix mode is
unchanged.
Verified on the whitelist host: the user page renders the pool address
and no "Address block" row.
Co-authored-by: CommandCodeBot <noreply@commandcode.ai>
The interactive flow already starts pool mode with an EMPTY pool (the
admin adds addresses later in the panel), but the VPSMGR_IPV6_MODE=pool
env override still demanded a non-empty VPSMGR_IPV6_POOL and died. New
installations selecting pool mode through the env path would fail; the
pool list is now optional in both paths.
Co-authored-by: CommandCodeBot <noreply@commandcode.ai>
- net.ipv6_pool is now operator-editable (was immutable): accept bare
addresses or explicit /128 (any other prefix length rejected), stored
normalized to the bare form.
- New admin "IPv6 Pool" page (visible in pool mode): multi-line
batch-add box, list with per-address state (free / used + owner), and
remove for free addresses (assigned ones are refused — a user keeps
its address for life).
- Adding addresses re-applies the host plumbing so any address the
provider bound on the external interface is detached immediately.
- mgr: AddPoolIPv6s / RemovePoolIPv6 / PoolList / PoolEntries with tests.
Co-authored-by: CommandCodeBot <noreply@commandcode.ai>
Whitelist providers typically bind only ONE address on the NIC at boot;
the rest are added by the user in the provider panel when needed. So
auto-collecting "all global addresses on the NIC" at install finds at
most one address and never reaches the pool branch. New flow:
- detector VERIFIED -> prefix mode (as before)
- not verified -> ask: pool mode (starts EMPTY; addresses are added
later in the admin panel) / manual prefix (trust the user's input) /
disable
- no IPv6 on the NIC at all -> manual prefix or disable
The pool is no longer seeded from the NIC at install.
Co-authored-by: CommandCodeBot <noreply@commandcode.ai>
Use the documentation-reserved 2001:db8:: block instead of a real
provider-assigned prefix in examples.
Co-authored-by: CommandCodeBot <noreply@commandcode.ai>
- addr-del must carry the address's actual prefix: the provider binds
whitelist addresses as /64, and a bare-address delete fails with
"Cannot assign requested address", leaving the address on eth0 where
it breaks the routed NIC (rp_filter drops the container's packets).
WireIPv6Pool now probes the live prefix and strips only what is bound.
- Routed containers get public IPv6 nameservers in eth0.network: the
routed NIC has no DHCPv6/RA DNS source, so without them v6-only name
resolution times out.
Verified on the whitelist host: a simulated reboot that re-binds the
addresses as /64 is self-healed by `vps ipv6-reapply` (only the host's
own address stays on eth0), and the container reaches out over v6 with
DNS, over v4, and is reachable from outside (Globalping 0% loss).
Co-authored-by: CommandCodeBot <noreply@commandcode.ai>
Incus's routed NIC refuses to start unless net.ipv6.conf.all.proxy_ndp=1
is set (its startup validation), which the earlier proxy-ndp-on only set
per-interface. Now set both the global and the interface sysctl.
Co-authored-by: CommandCodeBot <noreply@commandcode.ai>
Empirically this is what makes discrete whitelist addresses work:
- The provider binds every address on the host's NIC at boot; while the
host holds an address the kernel treats it as LOCAL and drops the
container's packets using it as a source (source-address validation),
so outbound v6 silently fails. Pool mode now removes each pool address
from the external interface (vps ip6 addr-del), on add/reinstall and
every ipv6-reapply/boot — self-healing after a reboot re-binds them.
- Containers get TWO NICs: eth0 = nictype routed on the external
interface with ipv6.address=<pool /128> (Incus programs the /128 route
+ proxy_ndp + fe80::1 gateway), eth1 = bridged on incusbr0 for the
shared IPv4. systemd-networkd binds the /128 statically on eth0 and
DHCPv4s on eth1.
- The manual bridge /128-route + proxy_ndp plumbing (SetupIPv6BridgePool,
WireIPv6Pool route/neigh) is gone — Incus programs everything.
Verified end-to-end on the whitelist host: container reaches out over v6
(curl -6 returns its own /128) and v4 (eth1 NAT) simultaneously, external
Globalping hits the container /128 with 0% loss, delete releases the
address and a new container reuses it.
Co-authored-by: CommandCodeBot <noreply@commandcode.ai>
Pool mode adds a create-user dropdown (auto first-free / a specific free
address / no IPv6) and an IPv6 column in the user table. Docs cover the
two modes, the installer flow, and the empirically verified per-address
routing.
Co-authored-by: CommandCodeBot <noreply@commandcode.ai>
00-ip-ask.sh now runs the unchanged check-ipv6-support.sh. Whole prefix
verified -> classic prefix mode. Not verified but >1 global address on
the NIC -> list them, keep the first for the host, ask the user to
confirm the rest as the pool. Declined or no addresses -> pure IPv4.
ndppd is only installed in prefix mode; pool mode uses kernel proxy_ndp.
vps ip6 gains neigh-add-proxy for the per-address wiring.
Co-authored-by: CommandCodeBot <noreply@commandcode.ai>
Add picks the first free pool address (or an explicit admin choice; a
"none" opt-out creates a V4-only container), stored atomically with the
user row. Prefix-mode behavior is unchanged. Per-container host plumbing
is the pre-/112 mechanism: /128 route through the bridge + kernel
proxy_ndp on the external interface, rebuilt from the DB at boot /
ipv6-reapply. Prefix-only steps (ndppd rules, container peer script,
/112 block routes) are skipped in pool mode.
Co-authored-by: CommandCodeBot <noreply@commandcode.ai>
ipv6_mode (none|prefix|pool) is immutable (snapshot-enforced) — switching
modes would renumber every container. ipv6_pool holds bare global /128
addresses; each is validated (global, non-ULA, no prefix length, no
duplicates). FillAuto reads VPSMGR_IPV6_MODE / VPSMGR_IPV6_POOL from the
installer.
Co-authored-by: CommandCodeBot <noreply@commandcode.ai>
users gains ipv6_address (NULL = no v6). A UNIQUE index guarantees one
address is never handed to two users, and the reservation is written in
the same transaction as the user row. The address dies with the row, so
deleting a user releases it for reuse automatically.
Co-authored-by: CommandCodeBot <noreply@commandcode.ai>
The 0755 world-readable scheme from 2980e3a only touched the installer
scripts; ensureVPSUser still ran the old cross-link block on every
`vps install` (40-panel.sh), chmodding /etc/traefik and the dynamic dir
back to 0750 after 30-traefik.sh had set 0755 — so the fix never stuck
until a manual chmod 755.
Drop the vps<->traefik group cross-link and 0750 from ensureVPSUser:
with a world-readable dynamic dir, traefik reads the YAMLs as its own
user and vps writes as owner; no group link is needed. 30-traefik.sh now
chowns the dynamic dir to vps:vps (matching ensureVPSUser) instead of
root:traefik, so the service unit keeps running as unprivileged traefik.
Co-authored-by: CommandCodeBot <noreply@commandcode.ai>
uninstall.sh deleted the vps/traefik users but not their same-named
primary groups, so a reinstall hit useradd exit 9 (G_NAME_IN_USE) when
creating the user. uninstall.sh now groupdels the groups alongside the
users, and ensureVPSUser also adopts a leftover group via --gid so an
already-broken host heals on reinstall.
Co-authored-by: CommandCodeBot <noreply@commandcode.ai>
Keep the v1.0.0 codebase unchanged (main.go untouched). Two installer
script fixes on top:
- scripts/30-traefik.sh: make /etc/traefik and the dynamic dir 0755
(world-readable) instead of relying on the vps<->traefik group
cross-link, so traefik can always read the dynamic dir regardless of
install order or group membership. vps still owns the dir and writes
as the owner.
- scripts/40-panel.sh: fix the prebuilt-download EXIT trap to not
reference a function-local variable under `set -u` (unbound variable
skipped cleanup).
Co-authored-by: CommandCodeBot <noreply@commandcode.ai>
The loop-file pool is sparse — creating it costs no disk, only actual
container data grows it. The "leave 3 GiB free" guard was a worst-case
worry that blocked real installs (a 23 GiB-free host shrank its pool
instead of just working), and the percentage ceiling already keeps the
host from being fully starved. Remove the check; pool = 90% (or 80%)
of free space, no reserve.
Co-authored-by: CommandCodeBot <noreply@commandcode.ai>
A 30 GiB disk (23 GiB free) failed install: pool = 90% of free = 20.7
GiB, leaving 2.3 GiB — under the 3 GiB reserve, so 10-incus.sh died
with "too little free space". Any disk with < 30 GiB free was
uninstallable.
Now the pool is shrunk in 512 MiB steps (down to a 1 GiB floor) until
the 3 GiB host reserve holds, instead of aborting. A small pool beats
no pool. Verified on the 30 GiB Ubuntu 24.04 test host: 19.5 GiB pool
created, install completes, full test suite passes.
Co-authored-by: CommandCodeBot <noreply@commandcode.ai>
The distro package was pointless: Debian 12's golang-go (1.19) can't
parse go.mod's `toolchain` directive anyway, so the official go.dev
tarball was always the fallback. Now we go straight to go.dev when no
usable go (>= 1.21) exists — one less apt install, one less failure
path.
And once the panel binary is produced, the go toolchain is dead weight
(a couple hundred MB on a small host) along with the module/build
caches including the auto-downloaded go1.26.5 toolchain. Track whether
the installer brought the toolchain itself (GO_FROM_OFFICIAL) and, if
so, remove /usr/local/go + GOMODCACHE + GOCACHE right after bin/vps is
in place. A pre-existing host go is left untouched.
Co-authored-by: CommandCodeBot <noreply@commandcode.ai>
2 GiB floor was more than a small VPS needs. Create a swap file the
size of RAM (1:1) with a 1 GiB floor: 1G box gets 1G, 2G box gets 2G,
still plenty for --local-build's go build, less disk eaten.
Co-authored-by: CommandCodeBot <noreply@commandcode.ai>
Debian 12 ships golang-go 1.19, which cannot parse go.mod's `toolchain
go1.26.5` directive (unknown directive) — so `install.sh --local-build`
failed on Debian 12 before the panel was ever built. ensure_go only
checked `command -v go` and accepted the broken 1.19.
ensure_go now:
- accepts an existing go only if it is >= 1.21 (the minimum for
toolchain auto-switch that go.mod relies on)
- otherwise installs the distro package, and if that is still too old,
downloads the official Go 1.26.x tarball from go.dev into
/usr/local/go and puts it first on PATH for the installer
Verified on the Debian 12 test host (previously failed with go1.19).
Co-authored-by: CommandCodeBot <noreply@commandcode.ai>
The swap recommendation was half of RAM, so a 1 GiB host got a 512M
swap file. `--local-build` compiles the panel (go build) and runs Incus
+ ZFS in the same memory; on the Debian 13 1G test host the build OOMs
and only an explicitly-added 2G swap got it through. Floor the swap
file at 2 GiB so the interactive default (and the non-interactive
auto-create) is enough for small boxes.
Co-authored-by: CommandCodeBot <noreply@commandcode.ai>
ndppd's init script starts the daemon with bare `-d -p $PIDFILE` — no
-c flag — so it reads the DEFAULT path /etc/ndppd.conf. The panel has
rendered its config to /etc/vpsmgr/ndppd.conf since the P0-1 rework, so
every container create failed at WireIPv6 with "Failed to load
configuration file '/etc/ndppd.conf'". (Found on the Ubuntu 24.04 test
host; would break every fresh install.)
- Maintain a root-owned symlink /etc/ndppd.conf -> /etc/vpsmgr/ndppd.conf
via two new pinned sudoers commands (/bin/ln -sf, /bin/rm -f) — no
wildcards, same privilege model as the rest of the whitelist.
- writeNDPPD links before restarting the daemon, and unlinks when the
last container's rule is removed; uninstall.sh drops the link too.
Also fix the vps-nft.service unit: the batch starts with
`delete table inet vpsmgr`, so the pre-apply `nft -c` check failed on a
missing table (fresh install, or a host after `nft flush ruleset`). The
table must exist BEFORE the check — `nft add table` is idempotent.
Co-authored-by: CommandCodeBot <noreply@commandcode.ai>
Three install-path bugs surfaced by the Ubuntu 24.04 test host:
- 40-panel.sh: replacing the running vps binary failed with ETXTBSY on
adopted installs (vps.service active) and the failure was ignored —
the install "succeeded" with the OLD binary. Stop the service before
cp and die if the copy fails.
- 40-panel.sh: `tee /etc/vpsmgr/.last-install.out` failed on a fresh
host because /etc/vpsmgr does not exist yet (it is created by
`vps install`). mkdir it first.
- 30-traefik.sh: chown vps:vps on /etc/traefik/dynamic ran before the
vps user existed (40-panel creates it), erroring with "invalid user"
and aborting the step under set -e. Skip when the user is absent;
ensureVPSUser re-chowns the dir on every install.
Co-authored-by: CommandCodeBot <noreply@commandcode.ai>
Trigger the pre-zpool cache/kernel cleanup earlier (free < 25 GiB). On
small VPS hosts the kernel comes from the provider template and stale
kernel rollback is not a concern, so the extra headroom is worth it.
Co-authored-by: CommandCodeBot <noreply@commandcode.ai>
The pre-zpool cleanup now triggers earlier (free < 20 GiB), leaving more
headroom for the pool loop file, Incus image store and the root
filesystem before a sparse pool is carved out.
Co-authored-by: CommandCodeBot <noreply@commandcode.ai>
The pre-zpool space reclaim kept the two newest kernels (running + one
fallback). Kernels are large (~200 MiB each), so drop to keeping ONLY
the running kernel — with two guards:
- If a NEWER kernel than the running one is already installed (upgrade
pending a reboot), keep that too: purging it would make the next
reboot boot an OLDER kernel than the one currently running.
- The linux-image-<arch> meta package (no version, holds no payload) is
excluded from the purge; deleting it would silently stop apt from
pulling future kernel upgrades.
Verified on this host: running 6.12.101 kept, 6.12.95 purged, meta
package untouched.
Co-authored-by: CommandCodeBot <noreply@commandcode.ai>
- The domains page header only had Overview; it now carries the same
Audit log button as the overview page.
- The overview procs column showed "<procs> / 4096" (the pids.max cap);
the cap is a fixed implementation detail, not a quota the operator
manages, so show just the live process count.
Co-authored-by: CommandCodeBot <noreply@commandcode.ai>
Review P0-1 remaining item: the sudoers whitelist allowed
`/sbin/ip -6 route/neigh/addr ... *` — sudo's wildcard matches ANY
arguments, so the unprivileged panel process could run ip with arbitrary
routes/addresses/neighbors (network control, and with ip's option syntax,
close to arbitrary root commands).
The panel now goes through a restricted subcommand of the (root-owned)
vps binary itself:
- New `vps ip6 <op> <addr-or-cidr> <iface>` hidden command runs as root
via sudo but validates EVERY argument before exec'ing ip: the operation
must be one of route-add/addr-add/route-del/neigh-del-proxy, the
address/CIDR must parse as global-scope IPv6, and the interface must be
a legal Linux name (same rule as config). Injection attempts like
"incusbr0;id" or IPv4 "1.2.3.4/24" are rejected.
- su.IP6() is the only entry point; all four ipv6.go call sites migrated.
- sudoers now carries `/usr/local/bin/vps ip6 *` and the eight bare
ip -6 wildcards are gone. cmdInstall re-asserts root ownership of the
binary so the helper itself cannot be tampered with by the panel user.
- isExistsErr() also recognizes "already assigned" — surfaced now that
addr-add errors are checked instead of swallowed.
Verified live as the vps user: legal ops go through, injection and bare
`ip -6` are both refused by the helper / sudoers.
Also: legacy immutable snapshots that stored empty lxd.pool/lxd.bridge
(the pre-fix code always wrote "") are treated as "never snapshotted"
instead of flagging every existing install as drifted on upgrade.
Co-authored-by: CommandCodeBot <noreply@commandcode.ai>
- Admin user table now shows the persistent lifecycle status (creating /
reinstalling / failed) next to the container state, so a crashed
Add/Reinstall is visible to the operator instead of looking like a
healthy user (follow-up to the P1-2/P1-3 lifecycle work).
- 40-panel.sh's prebuilt-download scratch dir only had explicit rm on the
success/known-failure paths; an exit between cp and chmod would leak
it. An EXIT trap now guarantees cleanup (rm -rf of a missing dir is a
harmless no-op on the existing explicit paths).
Co-authored-by: CommandCodeBot <noreply@commandcode.ai>
Security review finding P0-1 (part 1 — the configuration-trust chain):
The panel daemon (unprivileged 'vps') generates the nftables ruleset and
ndppd config that root later consumes. Full isolation would need a
root-owned rule generator (a much larger refactor); this change narrows
the surface materially:
- fw.Reload now runs `nft -c -f` (check mode) BEFORE applying, so a
syntax/semantic error in a panel-generated file never reaches the live
ruleset — combined with the existing delete-table atomic batch, a bad
generation leaves the previous rules running instead of a partial or
garbage state. The check is a new pinned sudoers whitelist entry.
- vps-nft.service (root oneshot at boot) now validates with nft -c before
applying for the same reason.
- /etc/ndppd.conf moved to /etc/vpsmgr/ndppd.conf: the panel daemon
generates this file, so keeping it in the root-owned /etc zone meant
chowning a root-zone file to the panel user — the "unprivileged user
writes a root-consumed file" problem with extra steps. It now lives in
the panel's own writable dir; uninstall cleans the new path.
Remaining P0-1 surface (documented in the review file): the sudoers
whitelist's ip -6 ... * wildcards still let the panel issue arbitrary
route/neigh/addr operations. Tightening that to validated parameters is
deferred — it needs either a root helper or parameter pinning.
Co-authored-by: CommandCodeBot <noreply@commandcode.ai>
Security review finding P2-10 (operator decision: clean them up):
uninstall.sh never removed /etc/sudoers.d/vps or the vps/traefik system
users — a plain uninstall left a root-privileged sudoers file (nft / ip /
systemctl) permanently usable by a leftover account. Both are now removed
in every mode; a reinstall recreates user + whitelist, so adoption of
/etc/vpsmgr is unaffected while the residual root-equivalent access is
gone.
Co-authored-by: CommandCodeBot <noreply@commandcode.ai>
Security review finding P2-9 + the extra requirement in the review doc:
- P2-9: fixed /tmp paths were used by install steps — a local attacker
could pre-create the file (symlink follow on cat/tee/curl -o) and
/tmp/vpsmgr-install.out carried the one-time admin password in a world
guessable location. 10-incus.sh preseed now uses mktemp; 30-traefik.sh
downloads into a private mktemp dir with an EXIT trap; 40-panel.sh
writes the install transcript to /etc/vpsmgr/.last-install.out (0600,
root-only) instead of /tmp, and install.sh reads it from there.
- Extra: before sizing a loop-file zpool, 10-incus.sh now reclaims space
when / has less than 15 GiB free: apt clean, apt list purge, autoremove,
journal vacuum (3d), /tmp + /var/tmp, stale kernels (keep 2 newest,
linux-image-* only), pip/go build caches — then re-checks free space and
refuses to build a pool that would leave < 3 GiB free worst-case.
Co-authored-by: CommandCodeBot <noreply@commandcode.ai>
Security review findings P2-2 + P2-3:
- P2-2: SetupIPv6Bridge swallowed the ip route add / addr add /
enableForwarding errors (comment said "idempotent EEXIST fine", but a
REAL failure was equally silent — the install reported success with no
IPv6). Added an isExistsErr() discriminator: "already exists" is the
no-op case, every other failure now aborts setup. UnwireIPv6 stopped
returning void; its error is checked in Del and in Add's cleanup, where
a failed NDP proxy removal now keeps the user row marked 'failed'
instead of silently dropping it.
- P2-3: ApplyBandwidth treated ANY counter decrease as a container
restart and added the post-reset value. A late, lower sample from a
concurrent sampler (panel daemon + CLI, no shared lock) was therefore
counted twice. The sampler now reads the container's init PID
(Incus state 'pid') with each sample and stores it (schema v3,
bandwidth.last_pid): same PID + lower counter = out-of-order old
sample, dropped; PID changed = genuine restart, post-reset value
counted. Tests cover normal growth, out-of-order drop, restart with
new PID, rollover, and cascade delete.
Verified live: the running /etc/vpsmgr DB migrated to v3 with last_pid.
Co-authored-by: CommandCodeBot <noreply@commandcode.ai>
Security review findings P1-5, P1-7, P2-11, P2-12:
- P1-5: SetDisk / EnsureEth0Options / EnsureNicRateLimit each did a
GET -> modify -> PATCH of the whole devices map; two concurrent calls
clobbered each other's keys (rate limit vs disk size vs isolation
options). Client now holds a per-instance mutex for the entire
read-modify-write. The map entry is kept after use so a waiting
goroutine blocks on the same mutex.
- P1-7: Del removed the traefik YAMLs AFTER the container and DB row, so
a crash mid-delete left a proxy config pointing at an IP that a new
tenant could later reuse. Domain files are now removed FIRST — a
failed removal aborts the delete (retryable) instead of completing a
delete with a dangling proxy config.
- P2-11: cmdServe now runs SyncAllDomains at every panel start, so any
DB/file drift from a crash is repaired by the next boot instead of
persisting until a manual `vps install`.
- P2-12: AddDomain/DelDomain/SetDomainProtocol/SyncAllDomains now share
one domainMu (opMu is not taken by them, and Del's own domain-file
removal deliberately does not take domainMu, so there is no lock-order
deadlock). Traefik.WriteDomain's temp file is now PID-unique so two
writers cannot race on the same .tmp path.
Co-authored-by: CommandCodeBot <noreply@commandcode.ai>
Security review findings P1-9, P1-2, P1-3, P1-4:
- P1-9: migrate() was CREATE TABLE IF NOT EXISTS only — no version
tracking, so a kept /etc/vpsmgr from an older install could never
gain new columns. Added a schema_migrations table, versioned
transactional migrations, downgrade protection (refuse to open a DB
newer than the binary), and v2 adding users.status. Legacy databases
upgrade in place; tested with a hand-built v1 database and idempotent
reopen.
- P1-2: Add's cleanup deleted the DB row even when the container delete
failed, silently leaking an orphan whose IP/idx NextFreeIdx could hand
out again. Cleanup now keeps the row marked 'failed' whenever the
container or firewall cleanup did not fully succeed; only a clean
rollback deletes it. Add writes the row as 'creating' up front and
flips to 'ready' only once container + plumbing + nft all exist.
- P1-3: Reinstall destroyed the old container with no persisted state,
so a mid-reinstall crash left a DB row pretending the container was
fine. Reinstall now marks 'reinstalling' before the delete, rolls the
half-built container back and marks 'failed' on any failure, and
returns to 'ready' only after the new container is provisioned and
wired.
- P1-4: Reinstall kept the in-memory throttled flag, so an over-quota
user's new container silently lost its rate limit. It now clears the
flag after a successful reinstall; the next enforce pass re-evaluates
from the actual Incus NIC state.
Verified live: the running /etc/vpsmgr DB (v1 schema) migrated to v2
with the status column and recorded version 2; existing users are
'ready'.
Co-authored-by: CommandCodeBot <noreply@commandcode.ai>
Security review finding P2-4:
- net.ext_if was only checked non-empty, then interpolated straight into
nft (oifname) and ip ... dev commands — special characters could
change rule semantics. It now must match a Linux interface name
(1-15 chars, alphanumerics/_.-, never starting with '-'; kernel
IFNAMSIZ limit).
- panel.public_ip accepted any net.ParseIP value including IPv6, but the
generated DNAT rules use `ip daddr` (IPv4-only) — an IPv6 value
silently produced an invalid rule set. It now requires IPv4 (or
AUTO/empty). panel.display_ip stays IP-agnostic: it is only displayed
to users, never used in rules.
Container names were already constrained by ValidateName (letters/digits/
hyphens, no path chars), and net.subnet/net.gateway are immutable with
CIDR validation, so the nft generation surface is now fully validated.
Co-authored-by: CommandCodeBot <noreply@commandcode.ai>
Security review finding P1-6:
Exec decided success by "stdout EOF" and treated non-empty stderr as
failure — so `sh -c 'exit 1'` (no output, non-zero exit) was reported
as success, and Provision/repair scripts could silently half-fail.
Reading the Incus server source confirmed the exit code is NOT sent on
the websocket control channel (that channel is client→server only:
signals/window-resize). The server records it via
op.ExtendMetadata({"return": status}), so Exec now GETs the operation
after stdout closes and treats metadata.return != 0 as failure, folding
stderr (or stdout if stderr is empty) into the error message.
Verified live against the real Incus daemon: exit 0 ok, `exit 1` with
no output now errors, `exit 3` reports the stderr text, stdin piping
still works.
Co-authored-by: CommandCodeBot <noreply@commandcode.ai>
Security review finding P1-8:
- The SQLite file was created with default umask (commonly 0644), so any
local user could read the DB — and the DB held raw session tokens,
making a read directly replayable. Open() now creates/chmods the DB to
0600 (plus existing -wal/-shm companions) and sessions store only
SHA-256 hashes of the 256-bit random tokens; the raw token exists only
in the cookie. Callers are unchanged: CreateSession returns the raw
token for the cookie, SessionUser/DeleteSession hash it internally.
Co-authored-by: CommandCodeBot <noreply@commandcode.ai>
Security review findings P2-6 + P2-7:
- The sudoers whitelist listed `systemctl enable traefik.service`, but
the code calls `enable --now traefik.service` (and `disable --now`),
and `is-active vps.service` vs the real `is-active --quiet`. sudo
matches argv exactly, so every one of those calls was rejected at
runtime: traefik could never be started by the panel and `vps config
set` could never restart the panel. The whitelist now mirrors the
actual invocations.
- Removed dead whitelist entries with no code call (start/stop/is-active
traefik.service, route replace, neigh add proxy, addr replace/del) —
narrower surface, nothing functional lost.
- ensureVPSUser/ensureSudoers swallowed every error and even skipped
installing the whitelist on failure, leaving a panel that looked
healthy but failed all privileged ops. Both now return errors and
abort `vps install`.
- scripts/00-check.sh now installs sudo (with visudo) as a hard
dependency.
Co-authored-by: CommandCodeBot <noreply@commandcode.ai>
Security review findings P1-10 + P2-1:
- http.Server had no Read/Write/Idle/Header timeouts and no
MaxHeaderBytes: slowloris or oversized requests could exhaust a small
host's fds/goroutines. Add 5s header, 15s read, 60s write, 60s idle
and a 1 MiB header cap.
- No handler capped the request body: all POSTs now go through
http.MaxBytesReader (1 MiB) in requirePost, and both login handlers
cap their body before ParseForm. The largest legit form (init script,
64 KiB) fits comfortably.
- PoolUsage swallowed Incus errors as (-1, nil), so Add proceeded when
capacity could not be determined. It now returns the error, and Add's
existing error path refuses to create — fail closed.
Co-authored-by: CommandCodeBot <noreply@commandcode.ai>
The immutable snapshot field list referenced lxd.pool / lxd.bridge,
which no registry field matches — FieldValue returned "" for them, so
pool/bridge drift was never detected or blocked by `vps install`.
- Rename the keys to incus.pool / incus.bridge in ImmutableFields.
- VerifyImmutable maps legacy lxd.* keys onto incus.* so pre-rename
snapshots are still verified against the historical baseline instead
of silently skipping the check.
- Add per-field tests: snapshot round-trip includes the incus fields,
pool/bridge drift is caught, legacy lxd.* snapshots pass when equal
and fail on drift.
Co-authored-by: CommandCodeBot <noreply@commandcode.ai>
