Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "etr"
version = "0.8.0"
version = "0.8.1"
edition = "2024"
description = "A Rust implementation of Eternal Terminal (et)"
license = "GPL-3.0-only"
Expand Down
33 changes: 33 additions & 0 deletions NOTES.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,39 @@ the link drops. This project uses **QUIC** (via the `quinn` crate) for the tran
layer, which provides reliable, ordered, multiplexed streams with congestion control
and TLS 1.3 built-in.

## Current state: v0.8.1 — the completions helper could overwrite the binary it was asked to read

Tooling only; no runtime change (145 tests, unchanged).
`scripts/install_completions.py` → **template v3**.

- **The defect, found in `rusticprofile` and propagated here because the file is vendored
byte-identically.** There it destroyed a working binary on a host taking hourly backups — a
3.6 MB executable replaced by a 21 KB bash completion script. etr's copy carried the same bug.
- **The mechanism.** `binaries` are command *names*, and the output path is
`directory / pattern.format(bin=binary)` — but **`Path("/dest") / "/abs/path"` discards the left
operand**. An absolute argument therefore relocates every write out of the completion directory
and onto the path itself, which under `--from-path` is the installed binary.
- **It fails in the worst available order.** `--from-path` runs `[binary]`, so an absolute path
*works for the read* and only breaks the write: generation succeeds and then destroys its own
input, exit 0, nothing printed. And the flag is **called `--from-path`**, which invites precisely
the argument that breaks it — so documenting it would not have prevented it.
- **Fixed by making it unexpressible.** `reject_path_like()` refuses any argument containing a path
separator or resolving absolute, **before any file is written**, and names the correct form
(`install_completions.py etr --from-path`) in the error.
- **Watched failing in both places it is enforced**: neutering the condition fails `--self-test`
(*"rejects an absolute path — expected True, got False"*) and fails `just standard-check`, which
`just check` depends on. Re-running the original accident against a stand-in file now leaves it
**byte-identical** instead of clobbered. A fourth self-test case pins the *property* — joining a
directory with an absolute string yields the absolute string — so the check survives a rewrite of
the guard.
- **etr's own recipes were never at risk**: `install`, `install-tag` and `standard-check` pass
`{{BINS}}`, i.e. bare names. Checked rather than assumed. The exposure is anyone invoking the
helper directly, which is how it happened.
- *Incidentally repaired:* the worktree copy of this file was **CRLF while the index was LF**
(`git ls-files --eol` → `i/lf w/crlf`), stale from before `.gitattributes` landed. Writing the
canonical LF file brings the two back into agreement, which is why this is a 50-line diff rather
than a whole-file rewrite.

## Current state: v0.8.0 — `-4`/`-6` address-family preference

New in v0.8.0 (client + server feature; 112 → 145 tests, one new e2e recipe).
Expand Down
52 changes: 50 additions & 2 deletions scripts/install_completions.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
# Copyright (C) 2026 l1a
"""Install shell completions for one or more binaries. Canonical across repos.

TEMPLATE v2 — vendored verbatim in rusticprofile, retch and etr. Change it here,
TEMPLATE v3 — vendored verbatim in rusticprofile, retch and etr. Change it here,
bump TEMPLATE_VERSION, and propagate in each repo's own PR. `just standard-check`
runs `--self-test` below, so the behavioural invariants are asserted rather than
compared as text: three separate repositories cannot diff each other's files, but
Expand Down Expand Up @@ -56,7 +56,7 @@
import sys
from pathlib import Path

TEMPLATE_VERSION = 2
TEMPLATE_VERSION = 3


def completion_dirs(env, home):
Expand Down Expand Up @@ -102,6 +102,34 @@ def zsh_reads(directory):
return str(directory) in res.stdout.splitlines()


def reject_path_like(binary):
"""Refuse a path where a command NAME belongs. Invariant 4.

`out = directory / pattern.format(bin=binary)` — and `Path("/dest") / "/abs/path"`
DISCARDS the left operand. So an absolute `binary` silently relocates the write out of
the completion directory and onto the path itself, which for `--from-path` is the
installed binary: the helper overwrites the very executable it was asked to read.

Measured 2026-08-24: `install_completions.py ~/.cargo/bin/rusticprofile --from-path`
replaced a 3.6 MB binary with a 21 KB bash completion script, on a host taking hourly
backups. It fails in the worst possible way — `--from-path` runs `[binary]`, so an
absolute path WORKS for the read and only breaks the write. Generation succeeds, then
destroys its own input, and the flag is *called* `--from-path`, which invites exactly
the argument that breaks it.

Made unexpressible rather than documented, on the precedent of refusing a snapshot-set
name beginning with `-`: a note in a docstring would not have stopped it, because the
person passing the path has already read the flag name and concluded it wants one.
"""
if os.sep in binary or (os.altsep and os.altsep in binary) or Path(binary).is_absolute():
raise RuntimeError(
f"`{binary}` looks like a path; this takes a command NAME.\n"
f" Use: install_completions.py {Path(binary).name} --from-path\n"
" (--from-path means 'run the binary as resolved on PATH', not "
"'here is a path to the binary'.)"
)


def generate(binary, shell, out_path, repo_root, from_path=False):
"""Write one completion file, or raise. Invariant 3: a failure is not survivable.

Expand Down Expand Up @@ -173,6 +201,23 @@ def check(name, got, want):
# All six shells present, so a silently dropped one cannot pass.
check("shell count", len(unix), 6)

# Invariant 4: a path where a NAME belongs must be refused, because pathlib would
# otherwise discard the destination directory and write over the binary itself.
def refuses(arg):
try:
reject_path_like(arg)
return False
except RuntimeError:
return True

check("rejects an absolute path", refuses(str(Path.home() / ".cargo/bin/rusticprofile")), True)
check("rejects a relative path", refuses(f"bin{os.sep}rusticprofile"), True)
check("accepts a bare name", refuses("rusticprofile"), False)
# The failure it prevents, stated as the property rather than the mechanism: joining a
# directory with an absolute string must never be how an output path is chosen.
check("pathlib really does discard the left operand",
str(Path("/dest/dir") / "/abs/path"), str(Path("/abs/path")))

if failures:
print(f"self-test FAILED (template v{TEMPLATE_VERSION}):", file=sys.stderr)
print("\n".join(failures), file=sys.stderr)
Expand All @@ -197,6 +242,9 @@ def main(argv):
repo_root = Path(__file__).resolve().parent.parent
dirs = completion_dirs(os.environ, Path.home())

for binary in binaries:
reject_path_like(binary) # invariant 4 — before ANY file is written

for binary in binaries:
for shell, (directory, pattern) in dirs.items():
out = directory / pattern.format(bin=binary)
Expand Down