Say so when the marker directory is there but unwritable #85

Merged
branderson merged 2 commits from notify-dir-check into main 2026-08-05 22:03:20 -07:00
Collaborator

Summary generated by LLM agent

Independent of the CI stack — branched from main, touches
docker/entrypoint.sh, bin/dejima, runtime.bats and static.bats.

This is the fix for the failure you hit earlier: notifications stopped
arriving, sessions vanished from dejima-attach, and the notify hook reported
EACCES — three symptoms with no obvious shared cause and no obvious moment
they started.

What causes it

bin/dejima creates the marker directory per launch and removes it on exit,
so a container that is restarted rather than relaunched has no bind source
left. Docker's answer to a missing bind source is not to refuse — it creates
one, root-owned. The directory then exists, which is all the notify hook's
[ -d ] guard asks, so the guard passes and the write fails instead of the
hook exiting quietly the way it does when remote access simply isn't
configured. The session's registry entry went with the same directory, which
is the half that explains dejima-attach.

Where the check goes

entrypoint.sh, because that is the only place that runs on a container
restart — bin/dejima is not involved in one, so a check there would have
missed this entirely. A launcher-side check is added too, for the different
case of a stale root-owned leaf that mkdir -p happily succeeds against.

Reported, not repaired. Chowning the directory would restore
notifications while the session stayed absent from the registry — which
bin/dejima writes host-side and never mounts into the container — leaving a
session that looks fixed with half the feature still broken. The message names
relaunching instead.

Opt-in stays opt-in

Both checks sit inside the "was this configured at all" guard, and there is a
test for that rather than only for the warning. Remote access is off by
default, so for almost every session this must say nothing — a warning about
an unconfigured optional feature would fire on every start, and one that fires
on every start is one nobody reads.

bin/dejima warns and continues rather than refusing. The state-dir check
just above it does refuse, but that one is about a symlink handing a session an
attacker-chosen path; this is only about an optional feature working, and
blocking a launch over undeliverable notifications costs more than it saves.

Tests

runtime.bats covers both directions against real containers — an unwritable
marker directory is reported and left alone, and an ordinary session says
nothing. static.bats pins the guard's placement, that nothing chowns the
directory, and that the launcher does not exit.

The runtime.bats half was not run locally; a sandbox has no Docker.

**Summary generated by LLM agent** Independent of the CI stack — branched from `main`, touches `docker/entrypoint.sh`, `bin/dejima`, `runtime.bats` and `static.bats`. This is the fix for the failure you hit earlier: notifications stopped arriving, sessions vanished from `dejima-attach`, and the notify hook reported EACCES — three symptoms with no obvious shared cause and no obvious moment they started. ## What causes it `bin/dejima` creates the marker directory per launch and removes it on exit, so a container that is **restarted** rather than relaunched has no bind source left. Docker's answer to a missing bind source is not to refuse — it creates one, root-owned. The directory then exists, which is all the notify hook's `[ -d ]` guard asks, so the guard passes and the write fails instead of the hook exiting quietly the way it does when remote access simply isn't configured. The session's registry entry went with the same directory, which is the half that explains `dejima-attach`. ## Where the check goes `entrypoint.sh`, because that is the only place that runs on a container restart — `bin/dejima` is not involved in one, so a check there would have missed this entirely. A launcher-side check is added too, for the different case of a stale root-owned leaf that `mkdir -p` happily succeeds against. **Reported, not repaired.** Chowning the directory would restore notifications while the session stayed absent from the registry — which `bin/dejima` writes host-side and never mounts into the container — leaving a session that looks fixed with half the feature still broken. The message names relaunching instead. ## Opt-in stays opt-in Both checks sit inside the "was this configured at all" guard, and there is a test for *that* rather than only for the warning. Remote access is off by default, so for almost every session this must say nothing — a warning about an unconfigured optional feature would fire on every start, and one that fires on every start is one nobody reads. `bin/dejima` warns and continues rather than refusing. The state-dir check just above it does refuse, but that one is about a symlink handing a session an attacker-chosen path; this is only about an optional feature working, and blocking a launch over undeliverable notifications costs more than it saves. ## Tests `runtime.bats` covers both directions against real containers — an unwritable marker directory is reported and left alone, and an ordinary session says nothing. `static.bats` pins the guard's placement, that nothing chowns the directory, and that the launcher does not exit. The `runtime.bats` half was not run locally; a sandbox has no Docker.
dejima-agent added 2 commits 2026-08-05 21:53:12 -07:00
Restarting a sandbox container rather than relaunching it silently breaks
remote access, in a way that presents as three unrelated symptoms with no
obvious moment they started: notifications stop arriving, the session is
missing from dejima-attach, and the notify hook reports EACCES.

bin/dejima creates the marker directory per launch and removes it on exit, so
a restarted container has no bind source left. Docker's answer to a missing
bind source is not to refuse - it creates one, root-owned. The directory then
exists, which is all the notify hook's own `[ -d ]` guard asks, so the guard
passes and the write fails instead of the hook exiting quietly the way it does
when remote access simply is not configured. The session's registry entry went
with the same directory, which is the half that explains dejima-attach.

entrypoint.sh now checks it at startup, because that is the only place that
runs on a container restart - bin/dejima is not involved in one, so a check
there would have missed this entirely. A launcher-side check is added too, for
the different case of a stale root-owned leaf that mkdir -p happily succeeds
against, but it is the container-side one that catches the reported failure.

Reported, not repaired. Chowning the directory would restore notifications
while the session stayed absent from the registry - which bin/dejima writes
host-side and never mounts into the container - leaving a session that looks
fixed with half the feature still broken. The message names relaunching
instead.

Both checks are inside the "was this configured at all" guard, and there is a
test for that rather than only for the warning: remote access is opt-in and off
by default, so for almost every session this must say nothing. A warning about
an unconfigured optional feature would fire on every start, and a warning that
fires on every start is one nobody reads.

bin/dejima warns and continues rather than refusing. The state-dir check just
above it does refuse, but that one is about a symlink handing a session an
attacker-chosen path; this is only about an optional feature working, and
blocking a launch over undeliverable notifications costs more than it saves.

runtime.bats covers both directions against real containers - an unwritable
marker directory is reported and left alone, and an ordinary session says
nothing. static.bats pins the guard's placement, that nothing chowns the
directory, and that the launcher does not exit.
test: give the marker-directory tests their own containers
tests / sandbox (config) (pull_request_target) Successful in 6m1s
tests / runner-env (host-docker) (pull_request_target) Successful in 1m26s
tests / sandbox (audit) (pull_request_target) Successful in 9m3s
tests / sandbox (proxy) (pull_request_target) Successful in 2m14s
tests / runner-env (sandbox) (pull_request_target) Successful in 1m26s
tests / sandbox (static) (pull_request_target) Successful in 3m6s
tests / sandbox (providers) (pull_request_target) Successful in 4m13s
tests / sandbox (remote) (pull_request_target) Successful in 5m3s
tests / sandbox (project) (pull_request_target) Successful in 5m55s
tests / host-docker (pull_request_target) Successful in 6m39s
bb2c1f218f
They called container_name(), which is this file's SHARED container - created
in setup_file and used by every dexec test in the run. Starting a second
container under that name is the conflict CI reported, and the `docker rm -f`
at the end of each body would have destroyed the shared one had the name not
collided first. The failure was the only thing that stopped it.

Tests that start their own container here use a distinct prefix and register
it in teardown, which is unconditional; cleanup at the end of a body is not,
because bats runs bodies under errexit and a failed assertion abandons the
rest. Both now do that.

Two things in the first test would also have let it pass for the wrong reason.

It mounted the directory read-only to make it unwritable, which makes the
"reported, not repaired" assertion vacuous - root could not have chowned it
even if entrypoint.sh tried. Mode 555 on an ordinary mount leaves it
repairable from inside the container and still unwritable by the session user,
so that assertion tests something.

And it asserted the directory was still owned by root, which depends on who
ran the suite: mktemp -d follows the caller, so a non-root run would have
failed on ownership having nothing to do with the check. It now asserts what
the check actually reads and what repairing it would change - that the session
user still cannot write there.
dejima-agent force-pushed notify-dir-check from 74a3f3393e to bb2c1f218f 2026-08-05 21:53:12 -07:00 Compare
branderson approved these changes 2026-08-05 22:03:09 -07:00
branderson merged commit 1cb1be5ee8 into main 2026-08-05 22:03:19 -07:00
Sign in to join this conversation.