vvvomit/interzone
Watch
1
0
Fork
You've already forked interzone
 0
Code Issues Pull requests Projects Releases 50 Packages Wiki Activity Actions

Fix data-loss, security, hang and UI bugs found in a codebase audit (GitHub #1) #1

Manually merged
vvvomit merged 0 commits from github/pr-1 into main 2026-10-29 22:32:37 +03:30
Conversation 0 Commits - Files changed -
vvvomit commented 2026-10-29 22:23:13 +03:30
Owner

Copied from https://github.com/charl1xcx/interzone/pull/1, by @TheNarratorsFrevor on GitHub. Discuss it there; this copy follows its commits.

An audit of the whole codebase turned up these bugs; each fix is its own commit with the details. Where a bug could be reproduced, a test reproduces it, and it fails before the fix and passes after.

Data loss and safety

  • interzone servers rm with no name deleted config.json (and servers rm ../config too), printing "✓ Forgot". Profile names are now validated in RemoveProfile/SetActive and the CLI.
  • Offline bundles could write outside their folder through a chain of symlinks (l1 -> ., l1/l2 -> .., l2/evil). Extraction now goes through os.Root. Bundles also no longer pick up .interzone-vars-*.json files (admin password) left behind by a killed action.
  • A corrupt config.json was treated as a first run, and the next save replaced every secret. It's now moved aside to config.json.broken-<time>.
  • Secrets were written in place, keeping a 0644 mode if the file already had one (common for hand-made inventories). config.WriteSecret writes atomically (temp, fsync, rename) and always 0600, following symlinks. It's used for config.json, presets, imports, and the inventory's vars.yml/hosts.
  • The base domain was never validated: https://example.org made nested inventory directories, and an imported matrix_domain: ../../tmp/x wrote outside the playbook.
  • Ansible ran with ANSIBLE_HOST_KEY_CHECKING=False. It now uses StrictHostKeyChecking=accept-new, like interzone's own ssh.
  • The register-user password was on the command line (visible in ps). It now goes through a 0600 vars file.
  • The shared ssh ControlPath directory in /tmp is only used if it's owned by the user and mode 0700.
  • Rollback kept one config.before-rollback.json for all servers, and the wizard overwrote the answers even when that backup failed to save. It's now per server and stops on errors.

Hangs and crashes

  • exec hung forever on an output line over 1 MB, and ctrl+c couldn't stop it. Long lines now come in pieces, the pipe is drained, and WaitDelay is set.
  • Server migration could truncate /matrix: send.Wait() closed the pipe the receiver was still reading, and a dead receiver hung the sender. Fixed by reordering the waits.
  • No /etc/os-release crashed the app (ids[0] on an empty slice).
  • Concurrent map iteration/write between background checks and the UI could abort the app. Added Config.Clone(), a deep copy, used by every background command.

UI

  • Results of background work were dropped if you'd left the screen: Mirrors, Review, Server test, Fine-tune, Updates and Manage spun forever. Worse, a user's one-time new password was lost. Nine message types now name their owner screen.
  • pgup always went back, so the deploy log, logs view and Fine-tune never scrolled with it (ctrl+p still goes back). The dead ctrl+k binding in Components is removed, and the cursor can't reach -1. A failed "forget server" now shows its error. Database-shell failures show in red, not green. Rollback failures are reported.

CLI

  • Flags after positional args were ignored: restore dump.sql --yes still asked (and failed under cron), and import PATH --dry-run printed the usage. A small parse helper now accepts both orders, and -- still ends the flags.
  • rollback exits 1 when the self-check finds problems, like apply. restore and rollback reject extra args.
  • -reset -layout bogus moved the folders aside before complaining about the layout. Options are now validated first. interzone -plain check explains that the command goes first, and interzone help shows the options.

vars.yml

  • A one-line Fine-tune value that's a block list or mapping (- a, a: b) produced invalid YAML.
  • A pasted | block in a YAML field wasn't indented.
  • Imports from a vars file not named vars.yml lost their reference copy.
  • Admin or sudo passwords containing {{, {% or {# (which Jinja breaks or silently alters) are flagged in Missing(). mas-cli gets --password=… and -- so a leading - isn't read as an option.

Build, CI and docs

  • make stamped the date as the version (2026.10.29), which outranks every release, so self-update never fired. It now uses git describe. Added make test.
  • CI runs go test -race. Fixed the CI comment that contradicted .forgejo/workflows/README.md.
  • The screenshot scripts look fonts up by name instead of using Arch-only paths, and the README lists fontTools and the fonts. shots.sh no longer uses grep -q under pipefail.
  • Removed unused gradientLines and cPanel.

Not changed (worth a look)

  • Fine-tune overriding matrix_authentication_service_enabled: false isn't reflected in UsesMAS()/Breaks().
  • ParseDefaults cuts a multi-line default at a column-0 comment.
  • A field hidden by a nested ShowIf is still written.
  • Quoted keys in extra vars aren't recognised as duplicates.
  • Secrets passed as extra vars aren't marked !unsafe; only the passwords above are checked.

gofmt, go vet and go test -race ./... pass, and staticcheck only reports two style notes.

🤖 Generated with Claude Code

<!-- github-pr --> Copied from https://github.com/charl1xcx/interzone/pull/1, by @TheNarratorsFrevor on GitHub. Discuss it there; this copy follows its commits. An audit of the whole codebase turned up these bugs; each fix is its own commit with the details. Where a bug could be reproduced, a test reproduces it, and it fails before the fix and passes after. ## Data loss and safety - **`interzone servers rm` with no name deleted `config.json`** (and `servers rm ../config` too), printing "✓ Forgot". Profile names are now validated in `RemoveProfile`/`SetActive` and the CLI. - **Offline bundles could write outside their folder** through a chain of symlinks (`l1 -> .`, `l1/l2 -> ..`, `l2/evil`). Extraction now goes through `os.Root`. Bundles also no longer pick up `.interzone-vars-*.json` files (admin password) left behind by a killed action. - **A corrupt `config.json` was treated as a first run**, and the next save replaced every secret. It's now moved aside to `config.json.broken-<time>`. - **Secrets were written in place, keeping a 0644 mode** if the file already had one (common for hand-made inventories). `config.WriteSecret` writes atomically (temp, fsync, rename) and always 0600, following symlinks. It's used for config.json, presets, imports, and the inventory's `vars.yml`/`hosts`. - **The base domain was never validated**: `https://example.org` made nested inventory directories, and an imported `matrix_domain: ../../tmp/x` wrote outside the playbook. - **Ansible ran with `ANSIBLE_HOST_KEY_CHECKING=False`**. It now uses `StrictHostKeyChecking=accept-new`, like interzone's own ssh. - The **register-user password was on the command line** (visible in `ps`). It now goes through a 0600 vars file. - The shared **ssh ControlPath directory in /tmp** is only used if it's owned by the user and mode 0700. - **Rollback kept one `config.before-rollback.json` for all servers**, and the wizard overwrote the answers even when that backup failed to save. It's now per server and stops on errors. ## Hangs and crashes - **`exec` hung forever on an output line over 1 MB**, and ctrl+c couldn't stop it. Long lines now come in pieces, the pipe is drained, and `WaitDelay` is set. - **Server migration could truncate `/matrix`**: `send.Wait()` closed the pipe the receiver was still reading, and a dead receiver hung the sender. Fixed by reordering the waits. - **No `/etc/os-release` crashed the app** (`ids[0]` on an empty slice). - **Concurrent map iteration/write** between background checks and the UI could abort the app. Added `Config.Clone()`, a deep copy, used by every background command. ## UI - Results of background work were dropped if you'd left the screen: Mirrors, Review, Server test, Fine-tune, Updates and Manage spun forever. Worse, **a user's one-time new password was lost**. Nine message types now name their owner screen. - pgup always went back, so the deploy log, logs view and Fine-tune never scrolled with it (ctrl+p still goes back). The dead ctrl+k binding in Components is removed, and the cursor can't reach -1. A failed "forget server" now shows its error. Database-shell failures show in red, not green. Rollback failures are reported. ## CLI - Flags after positional args were ignored: `restore dump.sql --yes` still asked (and failed under cron), and `import PATH --dry-run` printed the usage. A small `parse` helper now accepts both orders, and `--` still ends the flags. - `rollback` exits 1 when the self-check finds problems, like `apply`. `restore` and `rollback` reject extra args. - `-reset -layout bogus` moved the folders aside before complaining about the layout. Options are now validated first. `interzone -plain check` explains that the command goes first, and `interzone help` shows the options. ## vars.yml - A one-line Fine-tune value that's a block list or mapping (`- a`, `a: b`) produced invalid YAML. - A pasted `|` block in a YAML field wasn't indented. - Imports from a vars file not named `vars.yml` lost their reference copy. - Admin or sudo passwords containing `{{`, `{%` or `{#` (which Jinja breaks or silently alters) are flagged in Missing(). mas-cli gets `--password=…` and `--` so a leading `-` isn't read as an option. ## Build, CI and docs - `make` stamped the date as the version (`2026.10.29`), which outranks every release, so self-update never fired. It now uses `git describe`. Added `make test`. - CI runs `go test -race`. Fixed the CI comment that contradicted `.forgejo/workflows/README.md`. - The screenshot scripts look fonts up by name instead of using Arch-only paths, and the README lists fontTools and the fonts. `shots.sh` no longer uses `grep -q` under pipefail. - Removed unused `gradientLines` and `cPanel`. ## Not changed (worth a look) - Fine-tune overriding `matrix_authentication_service_enabled: false` isn't reflected in `UsesMAS()`/`Breaks()`. - `ParseDefaults` cuts a multi-line default at a column-0 comment. - A field hidden by a nested `ShowIf` is still written. - Quoted keys in extra vars aren't recognised as duplicates. - Secrets passed as extra vars aren't marked `!unsafe`; only the passwords above are checked. `gofmt`, `go vet` and `go test -race ./...` pass, and staticcheck only reports two style notes. 🤖 Generated with [Claude Code](https://claude.com/claude-code) 
ProfilePath maps "" to config.json, and RemoveProfile only refused
"default" and the active server, so 'interzone servers rm' (or
'servers rm ../config') deleted the first server's answers and printed
"✓ Forgot". RemoveProfile and SetActive now refuse anything that isn't a
valid server name, and 'servers use' checks the name the same way.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Go's flag package stops at the first argument, so 'restore dump.sql
--yes' still asked (and failed silently under cron), 'rollback 3
--quiet' wasn't quiet and 'import PATH --dry-run' printed the usage.
Every command now parses with a helper that keeps reading options after
the arguments; "--" still ends them. restore and rollback reject extra
arguments, 'servers rm' without a name prints the usage, and Usage
mentions 'servers rm'.

Rollback also exits 1 when the self-check after it finds problems, the
way apply does, and keeps the answers it replaces in
<server>.json.before-rollback: config.before-rollback.json was shared by
every server, so rolling back one overwrote another's copy.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Like the CLI, it now saves the answers it replaces next to the server's
own file instead of in one shared config.before-rollback.json, and if
that copy can't be written it leaves the answers alone rather than
overwriting the only copy.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
'interzone -reset -layout bogus' moved the config and data folders away
and only then complained about the layout. Options are now checked
first. 'interzone -plain check' says the command goes first instead of
calling it unknown, and 'interzone help' prints the options as -h does.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The default VERSION was the date, which parses as version 2026.10.29
and outranks every release, so a make build never saw an update. It's
now 'git describe', or "dev". 'make test' runs what CI runs, and CI
runs the tests with -race. The CI comment about Forgejo said the
opposite of .forgejo/workflows/README.md.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Saving:
- config.json, presets.json, the imported copies and the inventory's
  vars.yml and hosts are written by config.WriteSecret: to a temp file
  that's synced and renamed into place, so a crash or full disk leaves
  the old file instead of half of the new one, and always 0600. Writing
  in place kept the mode of a file that was already 0644, as a hand-made
  inventory or a restored backup often is. Links are followed, so a
  linked inventory stays linked.
- A config.json that isn't valid JSON is moved aside to
  config.json.broken-<time> before the wizard starts afresh. Before,
  the fresh answers' new secrets overwrote it on the next save, and a
  redeploy locked Synapse out of Postgres.
- Importing over a server whose answers can't be read now stops instead
  of replacing them without a .before-import copy.
- The imported vars file is kept as imported/<name>/vars.yml, which is
  where ImportedVars reads it, whatever it was called.

The base domain:
- It has to be a hostname. It becomes directory names in the
  inventory, so 'https://example.org' made nested directories, and an
  imported matrix_domain of '../../tmp/x' wrote outside the playbook.
  The Server step drops a pasted https:// and trailing slash; Missing()
  and the importer refuse anything else.

vars.yml:
- A one-line Fine-tune value that is a block list or mapping ("- a",
  "a: b") is written under the key rather than after it, which wasn't
  YAML.
- A pasted "|" block in a YAML field is indented like an override's.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
extract checked each entry's path as a string, but not the links earlier
entries had already made: "l1 -> .", then "l1/l2 -> ..", then "l2/evil"
each pass on their own and together wrote evil next to the folder. A
crafted bundle, passed hand to hand during a blackout, could have
written anywhere the user can, ~/.bashrc included. Entries are now
written through an os.Root, which won't follow a link out.

A bundle also no longer takes along the .interzone-vars-*.json files an
admin action leaves in the inventory if it's killed; they hold the
admin's password.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
exec scanned the command's output with a 1 MB line limit. Ansible's
"fatal: ... => {json}" dumps can be longer; the scanner then stopped,
nothing read the pipe again, the command blocked writing to it, and
even ctrl+c only got as far as "aborting…". Long lines now come in
pieces, whatever is left is drained, and Wait gives up on leftover
children after 10 seconds.

serverNeeds indexed the first word of the os-release ID, so a server
(or local machine) without /etc/os-release crashed the whole app when
its mirrors were checked. It now says it doesn't know the OS, as for
any other unknown one.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
copyData waited for the sender first, which closes the pipe the
receiver is still reading: with a slow receiver the last part of the
archive was lost after the old server's services were stopped, leaving
a partial /matrix that also blocked a retry. And if the receiver died
(a full disk), the sender blocked forever. The receiver is now waited
for first; if it failed, the sender is stopped and both servers' last
words are shown. The new inventory is written with WriteSecret.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- Every playbook ran with ANSIBLE_HOST_KEY_CHECKING=False, so anyone in
  the middle could pose as the server and be handed the sudo password
  and every secret in vars.yml. Ansible's ssh now uses
  StrictHostKeyChecking=accept-new, like interzone's own ssh calls: a
  new server is trusted on first sight, a changed key stops the run.
- The ssh connection-sharing folder in /tmp is only used when it's this
  user's own and closed to others; someone else could have made it
  first and planted a socket.
- Registering a user passed the password in --extra-vars, readable with
  ps. It goes through a 0600 vars file, like the other admin actions.
- The restore source and backup destination are passed as JSON, so a
  path with a space no longer breaks them.
- The update preview's git fetch puts "--" before the URL, as every
  other git call does.
- A domain's backup list no longer includes dumps of another domain
  that starts with the same name and a dash (a.org vs a.org-b.net).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The dashboard, mirror, review, users, rooms, tokens and update checks
copied the config with *s.cfg, which still shares the Components,
Values and Overrides maps. They read those maps (VarsYAML, DNSRecords)
while the UI writes them: leaving Settings or saving a Fine-tune value
during a check could abort the app with "concurrent map iteration and
map write". config.Clone copies the maps and slices too, and the deploy
runner and history snapshot use it as well.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- Only some background results were routed to their own screen when
  another one was showing; the rest were dropped by whatever screen was
  up. Leaving Mirrors, Review, the Server step's source test, Fine-tune's
  defaults or Updates mid-check left it spinning for good. Leaving
  Users after "new password" lost the password, shown only once, after
  the server had already changed it, and kept every user action blocked.
  An import finished in the background never switched to the server.
  mirrorsMsg, checksPartMsg, checksMsg, userOpMsg, defaultsMsg,
  changesMsg, importReadMsg, importSavedMsg and servicesMsg now name
  their screen like the others.
- pgup went back from every screen, so the deploy log, the logs view
  and Fine-tune's lists never got to scroll with it. Past the wizard it
  now goes to the screen, as pgdown does; ctrl+p still goes back.
- Components bound ctrl+k to "up", but ctrl+k opens Go-to everywhere
  first. The binding is gone, and the cursor can't go to -1 when a
  search matches nothing.
- Forgetting a server showed no error when it failed: enter() cleared
  it straight after.
- "The database shell ended: exit status 1" was shown in green.
- Unused gradientLines and cPanel removed.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A deploy whose saved answers can't be read, or answers that can't be
saved, made enter on the rollback prompt do nothing at all. The reason
now shows on the status bar.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
render.py and social.py had Arch's font paths built in, so on Debian or
Ubuntu 'shots.sh all' built and started everything and then failed on
the fonts. They now look the fonts up by name under /usr/share/fonts
(or FONT_DIR). The README lists fontTools and the fonts, and mentions
'make test'. shots.sh's shows() no longer pipes into grep -q, which
under pipefail can miss a match when tmux is still writing.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- Ansible runs every value through Jinja, so an admin or sudo password
  with "{{" or "{%" failed the deploy, and one with "{#…#}" was quietly
  shortened, creating the admin with a password other than the one
  saved. Missing() now names them before the deploy starts.
- mas-cli read a new password starting with "-" as an option. The
  password goes as --password=… and the user name after "--".

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
vvvomit manually merged commit ec1f489385 into main 2026-10-29 22:32:37 +03:30
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels
No items
No labels
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
vvvomit/interzone!1
No description provided.