Box config: make /etc/lager writable by the box user, and fail loudly when a render fails

CHANGELOG.md

@@ -2,6 +2,62 @@
 
 All notable changes to the Lager platform are documented here. For detailed release notes, see [docs.lagerdata.com](https://docs.lagerdata.com).
 
+## [0.31.11] - 2026-07-13
+
+### Fixed
+
+- **`lager box config apply` no longer reports success while applying nothing.**
+  `apply` runs `start_box.sh` on the box as the login user, and its four
+  box-config renderers *create* files in `/etc/lager` (`box_config.docker.sh`,
+  `user_requirements.txt`, `cargo_packages.txt`, `npm_packages.txt`). Creating a
+  file needs write permission on the **directory**, and `lager install` left
+  `/etc/lager` owned by the container user only (`33:33`, mode `755`) — so every
+  render failed with `EACCES`. Renders are soft-failed by design (the container
+  must always come up), which turned this into a silent no-op: the install steps
+  read files that were never written, so `apply` skipped them, stamped the
+  applied-hash, and printed "Applied box config". Every `pip`/`cargo`/`npm`
+  package, mount, volume, and env var added through `lager box config` was
+  quietly dropped on any box whose last provisioning step was an install.
+  `/etc/lager` is now `33:<box-user-group>` mode `2775` (setgid), so the
+  container (owner) and `start_box.sh` (group) can both write it.
+
+- **`/etc/lager` is no longer world-writable.** `lager update` granted the box
+  user write access by running `chmod 777` on the directory, which also gave it
+  to every other local account — enough to replace `box_config.json`,
+  `saved_nets.json`, or the org secrets. It now gets the same owner/group/setgid
+  treatment as above, which is what the two writers actually need and nothing
+  more. This is also why the breakage above was invisible for so long: an
+  updated box ended up world-writable and rendered fine, while a freshly
+  *installed* box was owner-only and silently applied nothing — the two paths
+  disagreed.
+
+- **A box-config render failure is now loud, and no longer poisons the retry.**
+  A failed render printed a one-line warning and a raw Python traceback. It now
+  reports which file could not be written, why, and how to repair it.
+  `start_box.sh` exits `3` — a new code meaning "the container is up, but the
+  config on disk was not applied to it" — and `apply` neither stamps the
+  applied-hash nor rolls back on it. Not stamping the hash is the load-bearing
+  half: it previously did, so the retry after a fix short-circuited on "Config
+  unchanged since last apply; skipping restart" and the config could never be
+  applied. Rolling back is wrong here because the container is healthy and the
+  fault is environmental — restoring the previous config and re-bouncing would
+  fail identically. A genuine bounce failure (container possibly down) still
+  rolls back exactly as before. A failed in-container `pip`/`cargo`/`npm`
+  install now exits `3` for the same reason — those steps run *after*
+  `docker run`, so they already reported "container is up but ... may be
+  incomplete" while exiting `1` and triggering a rollback of a healthy
+  container.
+
+- **An uncaught exception in a script run inside the box container no longer
+  buries the real error.** `lager_excepthook` read `LAGER_HOST_MODULE_FOLDER`
+  with `os.environ[...]` in order to rewrite host paths out of tracebacks. That
+  variable is set by `lager python`; anything else that execs a script into the
+  container leaves it unset, so the exception *handler* raised `KeyError` —
+  Python then printed "Error in sys.excepthook:" followed by a traceback from
+  inside lager, and only after that the user's actual exception. A one-line
+  `ModuleNotFoundError` arrived buried under a stack trace pointing at lager
+  internals. With no host paths to rewrite, the traceback is now printed
+  unchanged.
 ## [0.31.10] - 2026-07-13
 
 ### Added

box/lager/box_config/config.py

@@ -884,3 +884,49 @@ def _atomic_write_json(path: str, payload: Any) -> None:
         if os.path.exists(tmp):
             os.unlink(tmp)
         raise
+
+
+class RenderWriteError(Exception):
+    """A rendered box-config artifact could not be written to disk."""
+
+
+def _current_user() -> str:
+    try:
+        import getpass
+
+        return getpass.getuser()
+    except Exception:
+        return f"uid {os.getuid()}"
+
+
+def write_atomic(path: str, body: str) -> None:
+    """Atomically write `body` to `path`, raising RenderWriteError on failure.
+
+    Shared by the four render_*.py scripts, which start_box.sh runs on the box
+    HOST as the box's login user. Creating the tmp file needs write permission
+    on the *directory*, so a /etc/lager that only the container user (uid 33)
+    can write makes every render fail — silently, because start_box.sh soft-fails
+    renders to guarantee the container still comes up. The result was a box whose
+    box_config.json said one thing and whose container ran another, with the CLI
+    reporting a successful apply. Turn the raw OSError into something an operator
+    can act on.
+    """
+    _ensure_dir(path)
+    tmp = f"{path}.tmp"
+    try:
+        with open(tmp, "w", encoding="utf-8") as f:
+            f.write(body)
+        os.replace(tmp, path)
+    except OSError as e:
+        if os.path.exists(tmp):
+            try:
+                os.unlink(tmp)
+            except OSError:
+                pass
+        d = os.path.dirname(path) or "."
+        raise RenderWriteError(
+            f"cannot write {path}: {e.strerror or e}\n"
+            f"  {d} must be writable by the user start_box.sh runs as "
+            f"({_current_user()}). Run `lager update --box <BOX>` to repair "
+            f"the permissions on {d}."
+        ) from e

box/lager/box_config/render_cargo_packages.py

@@ -33,17 +33,10 @@
 )
 
 
-def _write(path: str, body: str) -> None:
-    tmp = f"{path}.tmp"
-    d = os.path.dirname(path)
-    if d and not os.path.isdir(d):
-        os.makedirs(d, exist_ok=True)
-    with open(tmp, "w", encoding="utf-8") as f:
-        f.write(body)
-    os.replace(tmp, path)
+_write = cfg.write_atomic
 
 
-def main(argv: list) -> int:
+def _render(argv: list) -> int:
     if len(argv) < 3:
         print("usage: render_cargo_packages.py <box_config.json> <out_path>", file=sys.stderr)
         return 2
@@ -71,5 +64,16 @@ def main(argv: list) -> int:
     return 0
 
 
+def main(argv: list) -> int:
+    # A write failure here used to surface as a raw traceback that start_box.sh
+    # swallowed into a one-line warning, leaving the container running the old
+    # package set while the CLI reported a successful apply.
+    try:
+        return _render(argv)
+    except cfg.RenderWriteError as e:
+        print(f"[ERROR] {e}", file=sys.stderr)
+        return 1
+
+
 if __name__ == "__main__":
     sys.exit(main(sys.argv))

box/lager/box_config/render_docker_args.py

@@ -81,17 +81,10 @@ def _empty_body() -> str:
     )
 
 
-def _atomic_write(path: str, body: str) -> None:
-    d = os.path.dirname(path)
-    if d and not os.path.isdir(d):
-        os.makedirs(d, exist_ok=True)
-    tmp = f"{path}.tmp"
-    with open(tmp, "w", encoding="utf-8") as f:
-        f.write(body)
-    os.replace(tmp, path)
+_atomic_write = cfg.write_atomic
 
 
-def main(argv: list) -> int:
+def _render(argv: list) -> int:
     if len(argv) < 3:
         print("usage: render_docker_args.py <box_config.json> <out.sh>", file=sys.stderr)
         return 2
@@ -120,5 +113,17 @@ def main(argv: list) -> int:
     return 0
 
 
+def main(argv: list) -> int:
+    # A write failure here used to surface as a raw traceback that start_box.sh
+    # swallowed into a one-line warning, so the container came up with none of
+    # the box_config mounts, volumes, or env vars while the CLI reported a
+    # successful apply.
+    try:
+        return _render(argv)
+    except cfg.RenderWriteError as e:
+        print(f"[ERROR] {e}", file=sys.stderr)
+        return 1
+
+
 if __name__ == "__main__":
     sys.exit(main(sys.argv))

box/lager/box_config/render_npm_packages.py

@@ -33,17 +33,10 @@
 )
 
 
-def _write(path: str, body: str) -> None:
-    tmp = f"{path}.tmp"
-    d = os.path.dirname(path)
-    if d and not os.path.isdir(d):
-        os.makedirs(d, exist_ok=True)
-    with open(tmp, "w", encoding="utf-8") as f:
-        f.write(body)
-    os.replace(tmp, path)
+_write = cfg.write_atomic
 
 
-def main(argv: list) -> int:
+def _render(argv: list) -> int:
     if len(argv) < 3:
         print("usage: render_npm_packages.py <box_config.json> <out_path>", file=sys.stderr)
         return 2
@@ -71,5 +64,16 @@ def main(argv: list) -> int:
     return 0
 
 
+def main(argv: list) -> int:
+    # A write failure here used to surface as a raw traceback that start_box.sh
+    # swallowed into a one-line warning, leaving the container running the old
+    # package set while the CLI reported a successful apply.
+    try:
+        return _render(argv)
+    except cfg.RenderWriteError as e:
+        print(f"[ERROR] {e}", file=sys.stderr)
+        return 1
+
+
 if __name__ == "__main__":
     sys.exit(main(sys.argv))

box/lager/box_config/render_pip_requirements.py

@@ -32,17 +32,10 @@
 )
 
 
-def _write(path: str, body: str) -> None:
-    tmp = f"{path}.tmp"
-    d = os.path.dirname(path)
-    if d and not os.path.isdir(d):
-        os.makedirs(d, exist_ok=True)
-    with open(tmp, "w", encoding="utf-8") as f:
-        f.write(body)
-    os.replace(tmp, path)
+_write = cfg.write_atomic
 
 
-def main(argv: list[str]) -> int:
+def _render(argv: list[str]) -> int:
     if len(argv) < 3:
         print("usage: render_pip_requirements.py <box_config.json> <out_path>", file=sys.stderr)
         return 2
@@ -70,5 +63,16 @@ def main(argv: list[str]) -> int:
     return 0
 
 
+def main(argv: list[str]) -> int:
+    # A write failure here used to surface as a raw traceback that start_box.sh
+    # swallowed into a one-line warning, leaving the container running the old
+    # package set while the CLI reported a successful apply.
+    try:
+        return _render(argv)
+    except cfg.RenderWriteError as e:
+        print(f"[ERROR] {e}", file=sys.stderr)
+        return 1
+
+
 if __name__ == "__main__":
     sys.exit(main(sys.argv))

box/lager/core.py

@@ -66,8 +66,22 @@ def lager_excepthook(etype, value, tb):
         Custom exception printer so users get nice tracebacks without
         weird temporary filenames and paths
     """
-    module_folder = os.environ['LAGER_HOST_MODULE_FOLDER']
     error_lines = traceback.format_exception(etype, value, tb)
+
+    # LAGER_HOST_MODULE_FOLDER is set by `lager python`, which is what the path
+    # rewriting below is for. Anything else that execs a script into the
+    # container (an external test runner, `docker exec python3 ...`) leaves it
+    # unset — and reading it with [] made the excepthook itself raise KeyError.
+    # Python then prints "Error in sys.excepthook:" plus a traceback from inside
+    # lager, and only then the user's actual exception, so a one-line
+    # ModuleNotFoundError arrived buried under a stack trace pointing at lager
+    # internals. With no host paths to rewrite, print the traceback as-is.
+    module_folder = os.environ.get('LAGER_HOST_MODULE_FOLDER')
+    if module_folder is None:
+        for line in error_lines:
+            print(line, end='')
+        return
+
     if module_folder == '/tmp':
         # We are just running a standalone script
         regex = re.compile(r'\A(\s*File \")tmp[a-zA-Z0-9_-]+.py(\",.*)')