Skip to content

Release v0.12.0 - #474

Merged
onel merged 18 commits into
mainfrom
release/0.12.0
Sep 11, 2026
Merged

onel merged 18 commits into
mainfrom
release/0.12.0

Conversation

@onel

@onel onel commented Sep 11, 2026

Copy link
Copy Markdown
Collaborator

Cuts v0.12.0. VERSION is 0.12.0, so merging this tags v0.12.0, creates the GitHub Release, and builds + publishes the hosted cloud image plus the brain and ui images on ghcr (docs/dev/contributing.md # Release model).

The branch is dev with main merged back in, per # Cutting a release, step by step. The merge was clean this time: main only carried the 0.11.1 release commits, which dev already had. VERSION reads 0.12.0. make check and make check-web are green on the merged branch.

This PR was held open while three defects the review found were fixed on dev (#476, #481, #480). They are listed under the headlines.

Eleven commits since v0.11.1. The headline is SSH. A malmo box can now give a single account shell access, from the dashboard, on both profiles.

  • SSH has a settings screen, so a person can actually use it (Give SSH a settings screen so a user can turn it on themselves #483). The API and the images shipped in this release with no way to reach them. Settings now carries an SSH screen: one switch that turns shell access on for your own account, then an Authentication methods pair, the malmo password and the public key, drawn as two cards of the same shape because both are locks the account must pass and both are demanded together when both are on. The screen reads key_required from the brain rather than checking the profile itself, so the method the box makes mandatory shows a switch that cannot be moved: the password on the appliance, the key on hosted. Keys arrive by paste or .pub file. The hosted box owner gets the password switch disabled, because their box password was made during the portal handshake and thrown away, so turning it on would have sshd ask for a string nobody has ever seen.
  • Per-account SSH, off by default (Per-account SSH: the decision and the brain-to-host seam #464). SSH is a per-account switch, not a box-wide one. The profile sets the required factor: a public key on hosted, the malmo password on the appliance. Each profile offers the other factor as an optional second lock, and then sshd demands both. Port 22 is open only while at least one account has SSH on. Turn the last one off and the daemon stops. The host side renders the whole sshd drop-in from the enabled set every time, writes keys before config, tests the config before reloading, and reloads rather than restarts so live sessions survive.
  • SSH ships in both images, and the boot test proves the port opens and closes (SSH in the images: package it, scope it, and prove the port opens and closes #470). The hosted image carries openssh-server installed but stopped, and the appliance gets the nftables rule that keeps :22 to the private ranges. Host keys are deleted at image build and generated on the box, so boxes from one image cannot share them. A new ssh boot in the cloud lane watches the whole path on a real box: :22 shut at boot, a key-less enable refused, a real key login, a password-only attempt refused, the port closing again, and a deleted account leaving no key behind.
  • Hosted owners can confirm privileged actions (Let a hosted owner confirm privileged actions through the portal #472). Destructive admin actions sit behind a five-minute window that used to need a password. A hosted owner has no password to type, so every one of those actions was unreachable for them. The box now mints a one-time challenge and the portal round-trip confirms it. Box users the owner created still get the password prompt, because they do have one.
  • The hosted tenant roster is closed (Close the public login picker on hosted boxes #462). GET /api/v1/auth/users returns 404 on hosted. It drew the login picker on the appliance, where the dashboard answers inside the house. On a public box it was a list of real Linux usernames that any scanner could read, and nothing on hosted called it.
  • The release process is written down, and dev stops drifting (Write down how a release is cut, take notes from the release PR, and stop dev drifting #453). Release notes now come from this PR body instead of a raw commit list. A new workflow opens a main -> dev sync PR after each release.
  • make check stops failing on local image-build leftovers (Vet and test only what is tracked, not local build leftovers #454). The gate now derives its package list from tracked files, so a gitignored vendored tree can no longer turn it red.
  • Two gap-ledger entries for openmausbot (catalog gaps: record two openmausbot entries (into dev) #466).

Three fixes found while cutting this release. The review on this PR read the whole accumulated diff and found three real defects in code that had already merged to dev. All three are fixed here rather than shipped and followed up.

What was tested

Known gaps

  • The portal half of the hosted confirm step lives in the private cloud repo and is not in this release. Until it deploys, the box side is inert and behaves exactly as before.
  • The SSH screen is typechecked and built but has not been clicked in a browser, and web-ui has no test runner. Its hosted branches in particular need a brain on the hosted profile to see.
  • The appliance lane does not exercise the SSH account toggle, because enabling an account cuts the test harness's own connection. The daemon lifecycle is proved in the cloud lane instead.

onel added 12 commits September 8, 2026 18:33
Backflow: merge main into dev after v0.11.1
GET /api/v1/auth/users lists every account on the box as {id, username}
and sits in publicPaths, so it answers with no session. That is right on
the appliance: it draws the user-list login screen before a credential
exists, and AUTH.md accepts the roster being visible because the reader
is already on the LAN or the mesh.

A hosted box has no such perimeter. The dashboard answers on the public
internet at <box-id>.malmo.network, so the same payload is a tenant
roster any scanner can read, and it hands over exact Linux usernames.
The box-id labels needed to find it are already public in certificate
transparency logs.

The route also had no caller there. App.vue bounces an unauthenticated
hosted visitor to the portal, so Login.vue never renders and the owner
returns through the SSO handshake, which mints the session itself.

authUsers now returns 404 on the hosted profile, before touching the
store. 404 rather than 403, mirroring ssoLanding on the appliance: the
route does not exist on that profile, and a 403 would confirm there is a
roster worth hiding. The check lives in the handler, not publicPaths,
because the middleware must still let it through on the appliance.
The comment sat directly above type loginPickerUser, so godoc read it as
that struct's documentation and the handler was left undocumented. The
placement predates this change, but the block is much longer now, so the
mis-attribution matters more. The struct gets its own short note on why
its field set stays narrow.
Close the public login picker on hosted boxes
* catalog gaps: record two openmausbot entries

Both come from the OpenMausBot import (store #98), and both are second or
third instances of a class rather than new mechanisms.

no-public-url: the app runs two inbound listeners and malmo routes one
port. The main server has a unix-socket seam the catalog compose relays
onto; the webhook receiver is started with no host and has no socket, so
there is nothing to reach. openclaw hit the same wall in July for its
inbound webhook channels and recorded it only on the store side, so this
is the class's first ledger entry and its second app.

host-only-admin: the app installs its agent CLIs from Settings, but only
accepts a plain npm one-liner. Six of thirteen agents install by piping a
shell script into bash, so they need a terminal on the machine, which no
box user has. Third app in the class after openclaw and pocket-id.

Worth stating in the second entry: the sandbox was not the blocker.
npm install -g into a path the app owns worked first time under
cap_drop ALL as an arbitrary uid, with no sudo.

* Fix the review finding: cite the right specs, and drop a stale claim

Greptile flagged the APP_ISOLATION.md citation on the host-only-admin entry.
It was right, and the underlying claim was worse than the citation.

"The hosted profile gives the owner no shell at all" stopped being true on
2026-09-09, one day before this entry: os #463 turned hosted SSH on per
account with a mandatory public key (ENVIRONMENT.md # Access & files,
AUTH.md # Device access, DECISIONS.md 2026-09-09). An admin is in sudo, so
a determined box owner could docker exec into an app and run the six
installers by hand. The entry now says so.

That does not weaken the gap, it sharpens it. What is missing is the
product surface: nothing in the dashboard or the store runs a command
inside an app, and the escape hatch is admin-only, outside anything the
store models, and lost on reinstall. APP_ISOLATION.md is still the right
cite for that shape (# Trust tiers: escape means Tier-2 curation or the
admin's own docker run over SSH, never an app-store action), so it stays
alongside the two specs that actually document the SSH posture.

The pocket-id entry above carries the same stale sentence, and is left
alone here: it is a merged record of a different run.
* Per-account SSH: the decision and the brain-to-host seam

SSH existed only on paper. It was specified for the appliance in AUTH.md
and BUILD.md, locked off for hosted in ENVIRONMENT.md, and there was no
SSH code anywhere in the tree.

The mandatory auth factor is now set by the environment profile: a public
key on hosted, the malmo password on the appliance. Each profile offers
the other factor as an optional second lock, rendered as
AuthenticationMethods publickey,password so sshd demands both. It is
never an alternative, because an account's security is set by its weakest
accepted method.

The one-password rule was a decision about a LAN, not about passwords. It
holds on the appliance, where nftables keeps :22 to RFC1918 and the mesh.
On a public box it would turn a household password into an
internet-facing credential, and into a second unguarded door to the same
credential the brain's login throttling guards, which sshd knows nothing
about.

Port 22 is open only while at least one account has SSH enabled. That
works because the box is its own perimeter, measured rather than assumed:
the cloud repo's Hetzner create call sets no Firewalls field and that
provider package has no firewall code, so a hosted box has every port
open. No in-guest nftables ruleset was needed, which matters because one
was deferred on purpose in DECISIONS.md 2026-06-19.

Two new host operations carry one account's full desired state, so a
retry after a partial failure converges instead of compounding. The
sshaccess package renders the drop-in whole from the enabled set, writes
keys before the config, validates with sshd -t before reloading, and
reloads rather than restarts so live sessions survive. An empty set
writes DenyUsers *, since omitting AllowUsers means every account to
sshd. Match blocks are written last, since anything after one joins it.

Brain side: two additive tables, self-service routes under /api/v1/me/ssh
because Device access is a My-account panel, elevation-class writes that
audit success and failure, and keys re-serialised from the parsed key so
a paste cannot smuggle in authorized_keys options.

* Fix the review findings: root LPE, unvalidated install, lost writes

Three defects, all in this slice's own new code.

A root privilege-escalation path. Keys were written into
~/.ssh/authorized_keys as root, which is what AUTH.md described. But
~/.ssh is a path the account controls and can replace with a symlink
between any check and any use, so the chown could be redirected onto
/etc and the read-modify-write could be redirected into disclosing a
root-only file. Tightening the checks would only narrow the race, so the
fix removes the user from the path: malmo's keys now live in a root-owned
file at /etc/ssh/malmo-authorized-keys/<user>, and each Match block names
that file and the user's own .ssh/authorized_keys. Keys a user added from
their shell keep working, and malmo never parses or deletes that file.

A rejected config was left installed. writeDropIn claimed to test a
candidate and then move it into place, and actually wrote straight to the
live path and validated afterwards, so a render sshd rejects stayed on
disk and the next start or reload would fail. The candidate is now tested
on its own before it is installed, and the combined config is tested
after, with the previous file restored if that check fails.

Concurrent writes could drop an account. Every call re-renders one
drop-in holding the whole enabled set, and the manager had no lock, so
the second of two concurrent calls would silently revoke the first's
account or stop sshd while someone still had it on. SetAccess is now
serialised, covered by a test that enables four accounts at once.

Also: the elevation-class key delete now audits its 404 and its last-key
guard rejection, which is the same shape as the last-admin guard.

* Fix the brain-side SSH races and the key a deleted user leaves behind

Three writes each read the account's state, decided, committed and pushed
with nothing holding them together, so two overlapping requests could
commit in one order and reach the host in the other. One process-wide
lock now covers the whole sequence.

A failed host push after a key delete kept the row deleted and pointed at
a reconciler that does not exist. The key was live on the host either
way, so dropping the row only hid it: the panel showed the key gone and a
retry would 404. It rolls back now, like the other two writes.

Deleting a user cascaded the brain's rows away but left the account in
sshd's AllowUsers with its key file still on disk, so a later user with
the same name inherited a deleted account's key. deleteUser revokes
first, and only for accounts that were actually enabled -- a disabled one
was never sent to the host, and calling on every delete would fail on a
box with no sshd installed.

* Drop the stale boot-posture bullet BUILD.md # SSH now contradicts

The decisions list still said sshd is enabled at boot with an empty
allowlist, which is the posture the new # SSH section replaced two
hundred lines above it.

* Hold the SSH lock until the deleted account is actually gone

The revoke released the lock before DeleteUser cascaded the account, so a
request that was already elevated could turn SSH back on in between. The
cascade then dropped the brain's rows while the host kept the key file,
which is the re-grant the revoke exists to prevent.

Also qualify the two BUILD.md claims about sshd being installed: that is
the appliance image only, and hosted packaging is #467.

* Take the SSH lock on every user delete, not only the enabled ones

The delete read the account's SSH state and locked only when it came back
enabled. A disabled account ran the whole delete unlocked, so a request
that was already elevated could turn SSH on in the window between the
read and the cascade. The host kept the account and its key file while
the brain's rows went away, and the next user with that name would
inherit it.

The lock is now taken before the read and held until the account is
gone. A write waiting on it finds the user row missing and fails its
foreign key before it can reach the host.
… closes (#470)

* Ship openssh-server on the hosted image, installed and stopped

Hosted SSH is a per-account opt-in with a public key as the mandatory
factor (DECISIONS.md 2026-09-09), so the image has to carry the daemon.
Both mkosi.conf and expected-packages.txt named openssh-server as a
package that must never appear; that sentence was written when a hosted
box had no SSH at all.

The daemon must not run at boot. Debian's postinst enables ssh.service,
and on this profile the daemon's run state is the ONLY control over :22 —
there is no malmo firewall and the provider attaches none — so a box that
booted with sshd running would answer for its whole life. The postinst
drops the wants link and adds a preset a later preset-all cannot override.
ssh.socket is left alone: trixie does not enable it on install, so the
service unit is the whole of the run state.

Two problems the packaging turned up:

Debian generates host keys at install time, which on an image build means
every box provisioned from that image ships the same ones — anyone with
the published image could impersonate a box to its owner's ssh client. The
build deletes them, same reasoning as the empty machine-id.

Debian's sshd-keygen.service cannot replace them: it is
ConditionFirstBoot=yes and only pulled in by ssh.service, and a daemon
that stays stopped until someone opts in usually first starts long after
the first boot. By then the condition is false, the unit is skipped, and
sshd exits with "no hostkeys available". malmo-sshd-keygen.service is the
same job without the condition.

* Ship the appliance SSH firewall rule and hardening drop-in

BUILD.md # SSH specifies both files and no build wrote either. They live
in a new checked-in appliance/ tree at their in-image paths, copied into
the generated mkosi.extra/ by the medium lane's bootstrap. They are
product configuration, not build output — on a real box the malmo .deb
ships them — so they are not generated by the script the way the harness's
own scaffolding is.

The nftables rule default-denies :22 and allows the three private ranges
plus iif lo, in its own table so loading it never touches Docker's rules.
Its loader unit runs on every boot rather than following the daemon, so
there is no window in which the port is reachable from off the LAN. The
mesh interface clause is not here: v1 installs no mesh client, so there is
no interface to name, and the fill-in lands with the mesh.

The hardening drop-in is static and separate from the file host-agent
renders, so PermitRootLogin no and PasswordAuthentication yes hold while
no account is enabled and nothing has been rendered.

That drop-in also settles the harness-lockout question the issue asked to
decide early. sshd takes the first value it obtains for a keyword and
reads sshd_config.d/*.conf in filename order, so malmo-hardening.conf
would have won PermitRootLogin and shut the harness out of its own image;
the harness file is renamed to sort first. The other half is that this
lane does not exercise the account toggle at all — enabling an account
renders an AllowUsers that excludes root, disabling the last one stops
sshd, and either cuts the connection every assertion here runs over. It
asserts the shipped posture instead; the lifecycle is proved in the cloud
lane, which reaches its box over a serial console.

* Prove the SSH daemon lifecycle on a booted box

#464 built SSH from the brain's API down to the rendered sshd config and
said so in its own gaps: no image could run it, and a unit test cannot
show a port closing. A new ssh boot in the cloud lane closes that.

It takes its own overlay and box-id, seeded with the test-portal key so it
can mint a real owner session. Everything else happens inside the guest:
the box makes its own keypair with ssh-keygen and connects to itself, so
nothing crosses the air gap. In order: :22 closed and the unit inactive at
boot with no baked host keys; a key-less enable refused; a key added and
SSH turned on, with the unit active, :22 listening and the drop-in naming
the account; a real ssh -i key login; a password-only attempt refused,
reading sshd's own list of authentications that can continue off the wire
rather than trusting the rendered string; the optional second factor
making that same key insufficient; removing the only key refused; SSH off,
with the unit stopped, :22 closed and the key file gone; and an account
with SSH on deleted, leaving no name in the drop-in and no key behind.

It is in the publish gate for the same reason access and update are: an
image that boots with sshd running has no control over its own port, and
one that cannot stop it again can never close it.

Every SSH write is elevation-class, and a hosted owner's password is
generated by the SSO auto-create and thrown away, so no owner can pass the
re-auth gate at all — #469, outside this slice. The scenario sets a
password as host root, which is the write the brain would make through
host-agent, and re-elevates before each write so the five-minute window
cannot expire mid-scenario under CI's TCG-only QEMU.

* Let IPv6 clients reach :22 on the LAN

In an inet table an `ip saddr` expression compiles to an nfproto==IPv4
test before the address compare, so it can never match an IPv6 packet.
With only the RFC1918 accepts, every IPv6 connection fell past them into
the final drop while the rule still read as if it allowed the LAN — and a
client resolving the box over mDNS commonly gets an AAAA record, so
`ssh malmo.local` from the same room would have hung. Confirmed by reading
what nft hands the kernel, not from the rule's text.

The v6 side allows link-local and unique-local only. Those are the two
ranges that cannot be routed in from outside, so they are the honest
translation of "RFC1918 = the LAN". A globally-routable v6 address stays
refused even from the same LAN: it is reachable from the public internet,
which is the property this rule exists to hold.

That leaves the dual-stack home network whose peers carry only ISP-
delegated global addresses. BUILD.md # SSH has no answer for it — IPv6 has
no RFC1918 — so it goes to NEXT.md rather than getting settled by widening
the rule here. Link-local always exists on a LAN interface, so no path is
shut in the meantime.

* Keep app containers off the box's SSH port

Docker's default address pool is carved out of 172.17.0.0/16 upward, which
sits inside the 172.16.0.0/12 the rule accepted, so every app container had
the household LAN's own reach to :22 — a compromised app could sit in front
of the authentication surface this rule exists to keep it off
(THREAT_MODEL.md, adversary: compromised app at runtime). The perimeter is
meant to be devices on the LAN or paired on the mesh, and a container is
neither.

Dropped by input interface rather than by subnet. The interface is exact:
a packet from a container always arrives on a Docker-managed bridge, since
the veth is enslaved to it. Excluding 172.17.0.0/16 by address instead
would lock out a household that genuinely numbers its LAN there. Same shape
as the hosted metadata block, which separates container traffic from host
traffic by the path it takes rather than the address it carries.

The medium lane asserts the drop and, separately, that it comes before the
RFC1918 accept — placed after, it would never be reached and the rule would
read as protection while being dead code.

Found by Greptile on #470.

* Generate the host keys at boot, not with the daemon

The first boot-proof run failed the ssh scenario at the enable:

  sshaccess: sshd rejected the rendered config:
  sshd: no hostkeys available -- exiting.

host-agent validates the rendered config with `sshd -t` BEFORE it starts
the unit, and `sshd -t` exits when there are no host keys. Deleting the
baked ones (the previous commit, so boxes don't share them) left
malmo-sshd-keygen.service pulled in only by ssh.service — which is to say
the keys arrived with the daemon, after the validation that needed them.
The very first enable could never succeed.

Pulled in by multi-user.target now, so the keys exist before anything can
validate a config. ssh.service is kept in WantedBy too, so a box whose
/etc/ssh was emptied after boot still gets them before the daemon starts.
Generating them on a box that never turns SSH on costs milliseconds and
opens nothing: a key on disk is not a listening port.

The assertion changes with it. "No host keys at boot" was the wrong test —
they have to be there for sshd -t. What it checks now is when they were
written: a key this box generated has an mtime at or after this boot, and
a baked one carries the image build's timestamp, hours or days earlier.

* Match sshd's methods list on what it says, not where it ends

The boot proof got through the enable and a real key login, then failed on
this assertion against a line the failure message printed as identical to
what it wanted. ssh terminates its -v debug lines with CRLF, so the line is
"...can continue: publickey\r" and an anchored `publickey$` can never
match. The earlier local probe grepped for the text unanchored, which
checked sshd's behaviour but never the pattern the assertion actually uses.

Both probes now strip CR and match on content rather than line end, and
they state the property directly instead of by shape. For the password
attempt, what matters is that "password" is ABSENT from the offered
methods: a list that merely contains publickey would also be satisfied by
publickey,password, which is the two-doors posture hosted refuses. For the
second factor, sshd's own "partial success" is the AND in its own words —
the key was accepted and the connection still did not open.

Both blocks were run verbatim against a real sshd before this commit,
which is the step that was missing the first time.

* Fix two rollback bugs in #464's brain-to-host SSH seam

Both found by the review pass on #470. Out of that issue's scope and
folded in on the maintainer's call rather than by drift.

deleteUser revokes the account's SSH on the host before the brain row
goes, because the revoke reads state the delete is about to cascade away.
That is the one place the brain-commits-first rule cannot hold, so every
later failure owes a compensating re-push — and there was none. A delete
that failed after the revoke left the account enabled in the brain and
revoked on the host, with nothing re-reading the host to notice: the user
silently lost SSH while the dashboard still showed it on.

The same path had a second half. The delete cascades ssh_access and
ssh_keys away, so restoring only the user row handed the account back with
its keys destroyed by an operation that reported failure. Both rows are
captured before the delete now and restored with it.

That restore needed AddSSHKey to stop stamping added_at unconditionally.
ListSSHKeys orders by it, so putting keys back would have silently
reordered the user's list after a failure that said nothing happened. A
zero AddedAt still means now, which is what adding a key is; a caller that
supplies one is re-inserting and keeps the original.

sshaccess.SetAccess writes keys before the config on purpose, so an account
is never named in a config a moment before the key that authenticates it
exists. That means the key file is already committed when sshd -t runs, and
writeDropIn's restore put back only the drop-in — leaving a key file for an
account the config no longer names, or an enabled account whose keys had
been replaced by a render that never took effect. The key file is
snapshotted and restored the same way now.

Each fix has tests that fail without it, checked by reverting the fix and
leaving the test in place.

* Keep /run/sshd so SSH can be turned on more than once per boot

The boot proof got through the whole first cycle — enable, real key login,
password refused, second factor, disable, port closed — and failed when a
second account tried to turn SSH on:

  sshaccess: sshd rejected the rendered config: exit status 255:
  Missing privilege separation directory: /run/sshd

Debian's ssh.service declares RuntimeDirectory=sshd, so systemd deletes
/run/sshd when the unit stops. Stopping it is an ordinary event in this
design rather than an administrator shutting a service down, and sshd
refuses to read any config without that directory. host-agent validates
every render with `sshd -t` before starting anything, so after the first
disable no account could turn SSH on again until the box rebooted.

The first enable after a boot works, because /run/sshd is created at boot
by openssh's own tmpfiles rule. That is what hid this: any test that
enables once passes. Only the second enable of a boot shows it, which is
why the ssh boot's second account is now named as the regression test.

Both images carry the drop-in. Preserving the directory costs an empty
0755 dir on a box that never uses SSH; it is not a port and grants
nothing, and the daemon's run state is still the whole of the port
control.
A hosted owner had no password to re-type, so every elevation-class action was unreachable on a hosted box. Their confirm step is now a second portal round-trip: the dashboard mints a one-time challenge, carries it to the portal inside a return path, and the SSO landing redeems it and elevates the session it mints.

No challenge, no elevation — that rule is what keeps a cross-site page from arming the window by driving the portal's open-box GET. The challenge is owner-only to mint, single-use, bound to its user, and spent before the session is elevated. The return path must be a relative path on this box.

Closes #469
Three changes to the release process, all found while cutting v0.11.0.

The Release model section documented what the workflow does automatically but
not what a person does, so the sequence was re-derived from git history each
time. It is now written out, including the merge-back and the rule that a
VERSION conflict always resolves to the new number.

Release notes now come from the release PR's body, with the generated commit
list appended under it. A list of 45 commit subjects is a poor answer to
'should I upgrade?', and the summary was already being written in the PR.
It falls back to generated notes alone when the body is too short, so a
thin release PR cannot produce notes worse than the automatic ones.

sync-dev.yml opens a main -> dev PR whenever main moves ahead. dev and main
drifted apart at every release from 0.8.0 to 0.11.0, and the same conflict was
resolved by hand each time. It opens a PR rather than pushing, because the
0.11.0 sync conflicted on VERSION and a bot picking the wrong side there would
walk the version backwards.
* Vet and test only what is tracked, not local build leftovers

make check was red on a machine that had ever built a cloud image locally.
go vet ./... walks the working tree, so it also walked dev/cloud/mkosi.tools/
— gitignored vendored sample code with an unreachable-code warning in it.

CI never sees those files, because they are not in the repo. So the local
gate failed for something CI is structurally incapable of catching, which is
the fastest way to teach people to ignore a gate.

Derive the package list from tracked .go files instead, the same idiom
fmt-check already uses. It keeps covering packages added later, which a
hardcoded list of top-level directories would not.

* Fail loudly when the package list comes back empty

An empty GOPKGS makes go test fall back to the current directory and exit 0
— a gate that passes having covered nothing. That is the #375 failure shape,
so guard it rather than trust it. Raised in review of this PR.
@greptile-apps

greptile-apps Bot commented Sep 11, 2026

Copy link
Copy Markdown

RetriggerConfidence Score: 5/5

The PR appears safe to merge; the remaining hosted-owner password-factor limitation is non-blocking.

Findings

  1. P1 Appliance password becomes optional
  2. P1 Security Cross-origin challenge minting
  3. P1 SSH has no dashboard
  4. P2 Password State Becomes Stale

Reviews (3) · Last reviewed commit: "Merge remote-tracking branch 'origin/dev..."

Comment on lines +89 to +96
func methods(a account) string {
if a.KeyCount == 0 {
return "password"
}
if a.RequirePassword {
return "publickey,password"
}
return "publickey"

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Appliance password becomes optional

On an appliance, adding a public key while omitting require_password reaches this function with KeyCount > 0 and RequirePassword == false. It then renders AuthenticationMethods publickey, replacing the profile-mandatory password with key-only authentication instead of making the key an optional second lock. Password-based SSH stops working, which can lock users out if they lose the key or connect from a device that only has the password.

Knowledge Base Used: Host-agent platform integration

Comment thread internal/hostagent/sshaccess/sshaccess.go Outdated
Comment thread internal/api/confirm.go Outdated
Comment on lines +39 to +42
// page could arm it silently. The challenge is the thing such a page cannot
// supply: it is minted by an authenticated POST to the box's own API, which a
// cross-origin page cannot make (the JSON content type forces a preflight the
// brain does not answer). A landing that carries no valid challenge signs the

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 security Cross-origin challenge minting

This defense assumes that the brain rejects cross-origin preflight requests, but the API reflects any Origin, permits credentials, and answers preflight with 204. A compromised hosted app under the same malmo.network site can therefore send the owner's SameSite=Lax, host-scoped dashboard cookie to the box apex, read a challenge, and drive the portal navigation that redeems it. Once the elevated session is issued, the same credentialed CORS policy allows elevation-class requests, defeating the intended confirmation boundary. Restrict credentialed CORS to the trusted development origin or enforce same-origin or CSRF checks on challenge minting and privileged mutations.

How this was verified: A same-site hosted app origin is accepted by credentialed CORS, can mint an owner-bound challenge with the dashboard cookie, and that challenge is redeemed by the normal portal SSO return path.

Knowledge Base Used: Access, setup, and system management API


- **Nothing was run against a real sshd or systemd.** `sshaccess` is covered by unit tests with the command runner and the account lookup faked. The daemon lifecycle is the port control, and a unit test cannot show a port closing — the issue's "Done when" asks for that verification and it has **not** been done. It needs the QEMU medium lane or a booted box.
- **No reconcile loop consumes `GET /v1/ssh/state` yet.** The endpoint and the client method exist and are covered, but nothing polls them on the heartbeat, so drift is not surfaced anywhere. The brain re-pushes only when the user acts.
- **No UI.** `web-ui` is untouched, so the feature is unreachable from the dashboard today. The Device access panel is follow-up 1.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 SSH has no dashboard

This release ships the SSH API and image support, but the progress entry confirms that no dashboard control calls /api/v1/me/ssh. Settings routes and navigation are hardcoded rather than generated from OpenAPI, so users cannot add the mandatory hosted key or enable SSH through the advertised Settings → My account → Device access flow. Add the Device access panel and its key and toggle mutations before presenting per-account SSH as a usable release feature.

Knowledge Base Used: Access, setup, and system management API

* Serve no CORS headers from the brain

withCORS wrapped the whole handler chain on both profiles: it reflected
whatever Origin arrived, set Access-Control-Allow-Credentials: true, and
answered every OPTIONS with 204.

On hosted that is a live hole. Apps are <slug>.<box-id>.malmo.network and
the dashboard is <box-id>.malmo.network, and malmo.network is not on the
Public Suffix List, so the two are same-site. The owner's SameSite=Lax
session cookie therefore rides a fetch from any app to the dashboard host,
and the reflected header let the app read the reply -- including the reply
to POST /auth/elevate/challenge.

Two places already asserted the opposite: AUTH.md # Re-authentication says
a cross-origin page cannot make an authenticated JSON POST here, and
confirm.go says the JSON content type forces a preflight the brain does not
answer. The hosted confirm step's cross-site argument rested on that
refusal, which was in fact a 204.

Removed rather than narrowed to an allowlist, because nothing calls this
API cross-origin in any lane. The dashboard fetches relative paths, Caddy
serves it and the brain on one host in production, and the Vite dev server
proxies /api to the brain -- which had already made the middleware's own
stated reason for existing untrue.

Closes #475

* Say what actually protects the challenge: the unreadable reply

Review pointed out that both the spec sentence and the confirm.go comment
described the wrong mechanism, and that the new AUTH.md guarantee repeated
it.

elevateChallenge takes no body and requires no JSON content type, so the
POST is a simple request in CORS terms. A same-site app page can send it
with the owner's cookie and a challenge is minted. The protection was never
that the request cannot be sent -- it is that the reply cannot be read, and
a challenge nobody reads is inert: single-use, bound to its user, and
expiring unspent.

The preflight still matters, for the requests that do trigger one: every
JSON PUT and PATCH on this API. The test comment now says which half covers
which.
* Keep the appliance's mandatory SSH password when a key is added

AUTH.md # Device access gives each profile a mandatory factor and an
optional second one: the appliance takes the malmo password with a key as
the extra lock, hosted takes a key with the password as the extra lock.
Only the hosted row was enforced.

An appliance account that added a key and left require_password unset got
AuthenticationMethods publickey, so the mandatory factor was replaced
rather than supplemented. The field is omitempty, so that is the default
path a client takes, not an odd one.

The fix belongs in the brain. sshaccess renders what it is handed, and
given a key with RequirePassword false its methods() is right to write
publickey alone -- host-agent does not know the profile and must not guess
which factor is mandatory. The brain was handing it a false the appliance
is not allowed to ask for.

setMySSH is the only entry point a caller-supplied value comes in through;
reapplySSH, the deleteUser restore and both rollbacks read the stored row.
So resolving once before the store write makes every downstream push
correct and stops the row describing a posture sshd is not running.

Closes #477

* Name the real reapply function, and test the omitted field

Two review findings.

The placement comment named reapplySSH, which does not exist -- the
stored-row reapply path is syncSSHIfEnabled. The same invented name was in
the progress entry and the index, where it made the argument for the
placement harder to check.

The appliance test said it covered the omitted-field default and then sent
require_password explicitly. Both shapes decode to the same boolean today,
but omitting it is what a panel actually sends, so the two now run as
separate subtests and a later presence-sensitive decoder cannot break the
default path unnoticed.

* Audit what was asked, and give each subtest its own harness

Two review findings.

The audit meta carried the resolved require_password on every path,
including the records written when the request was refused and nothing was
applied. On the appliance that describes a posture the box never took and
hides what the caller actually sent. It now carries the requested value,
which is what every record here is about, and the success record adds
require_password_applied beside it where there is a real applied state.

The two request-shape subtests shared one harness whose host-call slice
accumulates, and the helper read only the latest call -- so a shape that
stopped calling the host could pass on the other's leftover call. Each
subtest gets its own harness and asserts it made exactly one call.
* Put the SSH host state back when a systemd call fails

SetAccess writes key file, then drop-in, then the systemd call. The first
two steps already undid themselves: snapshotKeys restores the key file and
writeDropIn restores the drop-in when the combined sshd -t rejects the
render. The third step had nothing, so a failed enable, disable or reload
returned an error with both files left as written.

The brain reads that error as a host failure and rolls back only its SQLite
row, so the two sides then disagree about who has a shell. It does not stay
a bookkeeping difference: readDropIn treats the rendered file as truth on
the next call, so a stale entry is merged back in rather than overwritten.

A failed first enable left the account in the drop-in with its key on disk
and nothing in the brain -- and the next account to enable SSH starts the
daemon and brings the forgotten one up with it. A failed disable dropped
the entry and the key file while the brain put its row back to enabled.

snapshotDropIn now mirrors snapshotKeys, and undo restores the drop-in,
then the key file, then reconciles the daemon to the set as it was before
the call. The daemon step is not optional: enable --now starts the unit
before a later reload in the same sequence can fail, so restoring the files
alone could leave sshd serving a set that no longer exists on disk.

Closes #479

* Snapshot before the first write, and finish the undo

Two review findings.

The drop-in snapshot sat between the key write and the render, so a
snapshot that failed returned with the new key file already committed and
nothing to undo it. On a key replacement that silently changes which keys
authenticate while the brain rolls its row back. It is now taken before
either write, where a failure costs nothing.

undo returned at the first restore error, though the drop-in and the key
file are separate paths and neither restore depends on the other. It now
attempts both and joins the errors, so one failing does not leave the
other's write live. The daemon is reconciled only if the drop-in actually
went back: reloading against a render that could not be restored would put
the failed change into effect instead of undoing it.

The second half has a test, injected through the runner. The reordering
does not and cannot at this layer -- readDropIn reads the same path first
and fails on anything unreadable, so any test that forces the failure stops
there and would pass without the reorder. Kept because it is free and
strictly better, and the progress entry says so.

* Reconcile the daemon only when every restore succeeded

Review found that gating on the drop-in alone was not enough. If the key
file restore fails while the drop-in goes back cleanly, the restored config
points at the very path still holding this call's new keys, so reloading
sshd would make keys authenticate that SetAccess has already reported as
failed.

Both restores now have to succeed before the daemon is touched.

The test for it was wrong on the first attempt and passed against the
unfixed gate: the runner reset its own marker when the undo reloaded a
second time, hiding the extra reconcile. With that fixed it fails on the
old gate, showing the undo's extra enable and reload.

* Always stop the daemon on rollback, gate only the start

Review found the reconcile was gated in both directions, and stopping
should never have been.

applyDaemon(false) runs systemctl disable --now, which reads neither file,
so no restore failure can make stopping the wrong move. Gating it meant a
failed first enable that had already started the unit left sshd running
with nobody enabled -- and the daemon's run state is what closes :22, which
on hosted is the only control over the port there is.

Starting and reloading stay gated on both files being back, because those
read what is on disk.

The new test fails on the previous gate, showing the run ending at
systemctl enable --now with no stop after it.

* Restore the index row the rebase conflict dropped

The progress index lost the cors-origin-reflection row while resolving a
rebase conflict. The file is rebuilt from dev with this entry's row
appended, so the change is a pure addition again.
* Give SSH a settings screen so a user can turn it on themselves

The per-account SSH opt-in had a full API and no UI, so nobody could use it
without calling the brain by hand. Closes #482.

The screen reads key_required from the server rather than checking the profile
itself, so the rule that decides which factor is mandatory stays in the brain
and the two cannot drift. The hosted box owner is the one rule this screen
owns: their box password was generated during the portal handshake and
discarded, so offering them the password lock would have sshd demand a string
nobody has ever seen.

* Draw SSH as one switch and a pair of authentication methods

The password and the key are the same kind of thing: locks this account must
pass, both demanded together when both are on. Drawing one as a setting and the
other as a list read like a menu of ways in, which is the opposite of what
AuthenticationMethods publickey,password does. They now sit as two cards of the
same shape under one heading, each with its own switch, and the one the box
makes mandatory cannot be moved.

The pair is hidden while SSH is off, except on a hosted box with no key yet,
where the key card is the only way to reach the first key.

Also from the review: add and remove no longer share one error string, so a
refused removal can no longer appear inside the add form and be wiped by
cancelling it. And the key switch now reads the open add form as on, so the
first click moves the thumb instead of only opening the form.

* Record where the SSH nav item diverges from the settings spec

Greptile flagged SSH sitting in the System group while SETTINGS.md files it
under My account. The group already held Notifications, which that table files
the same way, so the divergence is the nav's and #290 is where it gets settled.

* Make the no-em-dash rule explicit in CLAUDE.md

The B1 rule only asked for fewer em-dash pile-ups, which read as a matter of
degree. It is not: none at all, in every kind of text we produce.
// does not guard this, and should not: it cannot tell an owner asking for a
// second lock from anybody else asking for one. This screen is the only place
// the case is handled, which is why it is written down here.
const ownerHasNoPassword = computed(() => isHosted() && isBoxOwner());

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Password State Becomes Stale

A hosted owner can gain a known box password through the admin password-reset flow, but this check always treats every hosted owner as having no password. The SSH screen then disables the optional password factor even though the account can use it and the API accepts it. The owner must call the API directly to enable this extra lock.

Knowledge Base Used: SSH account access

@onel
onel merged commit c0b832e into main Sep 11, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant