merge: review fixes

.gitignore

@@ -8,3 +8,35 @@ target/
 
 # MSVC Windows builds of rustc generate these, which store debugging information
 *.pdb
+
+# the cargo home CI keeps inside the project so it lands in the cache
+.cargo/
+
+# where the completions build writes when it is not given a directory
+completions/
+
+# os
+.DS_Store
+
+# editors
+.vscode
+.idea
+.ignore
+
+# ai
+CLAUDE.local.md
+.claude
+
+# languages
+.python-version
+
+# nix
+.nix-venv
+.envrc
+.direnv/
+
+# project-local
+TODO.md
+# database dumps, which this tool exists to keep out of a repository
+dumps
+*.partial

.gitlab-ci.yml

@@ -5,30 +5,38 @@ default:
       files:
         - Cargo.lock
     paths:
+      # no .cargo/bin here: the audit job runs what is in it, and this cache is
+      # shared by every branch and merge request, so a pipeline could otherwise
+      # leave behind the binary a later scheduled audit executes
       - .cargo/registry
-      # so the audit job does not rebuild cargo-audit on every schedule
-      - .cargo/bin
       - target
 
 variables:
   # keep the registry inside the project so it lands in the cache
   CARGO_HOME: $CI_PROJECT_DIR/.cargo
   CARGO_TERM_COLOR: always
+  # pinned, rather than whatever version exists on the night this runs
+  CARGO_AUDIT_VERSION: "0.21.2"
 
 # without this a push to a branch with an open merge request runs twice
 workflow:
   rules:
+    # first, because a scheduled pipeline sets CI_COMMIT_BRANCH as well and so
+    # would be matched by the branch rule below, or vetoed by the one above it
+    - if: $CI_PIPELINE_SOURCE == "schedule"
     - if: $CI_PIPELINE_SOURCE == "merge_request_event"
     - if: $CI_COMMIT_BRANCH && $CI_OPEN_MERGE_REQUESTS
       when: never
     - if: $CI_COMMIT_BRANCH
-    - if: $CI_PIPELINE_SOURCE == "schedule"
 
 check:
   script:
     - rustup component add rustfmt clippy
     - cargo fmt --check
-    - cargo clippy --all-targets -- -D warnings
+    # --locked on the first cargo invocation too: without it clippy resolves the
+    # dependencies and rewrites a Cargo.lock that has drifted from Cargo.toml,
+    # and the --locked commands after it then pass against what it just wrote
+    - cargo clippy --locked --all-targets -- -D warnings
     - cargo test --locked
     - cargo build --release --locked
   rules:
@@ -39,19 +47,33 @@ check:
 # runs only from a pipeline schedule, so set one up in the project settings
 audit:
   script:
-    - cargo install cargo-audit --locked
+    - cargo install cargo-audit --version $CARGO_AUDIT_VERSION --locked
     - cargo audit
     # reports dependencies that have drifted behind, without changing the lockfile
     - cargo update --dry-run
+  # its own cache, so the audit tool is not rebuilt on every schedule while
+  # still being written only by this schedule-only job. the pinned version is
+  # part of the key, so a bump fetches rather than reusing the old binary
+  cache:
+    key: audit-tools-$CARGO_AUDIT_VERSION
+    paths:
+      - .cargo/bin
+      - .cargo/registry
   rules:
     - if: $CI_PIPELINE_SOURCE == "schedule"
-  allow_failure: true
 
 # the msrv declared in Cargo.toml, so it fails when something needs a newer rustc
 msrv:
   image: rust:1.85
   script:
     - cargo build --locked
+  # its own key: artifacts built by another rustc are of no use to this job, and
+  # sharing one only has the two toolchains taking turns overwriting it
+  cache:
+    key: msrv-$CI_COMMIT_REF_SLUG
+    paths:
+      - .cargo/registry
+      - target
   rules:
     - if: $CI_PIPELINE_SOURCE == "schedule"
       when: never

README.md

@@ -95,15 +95,21 @@ named anything:
 - **postgres**: the service whose image is a postgres flavour, matching
   `postg`, `timescale`, `pgvector` or `citus`
 - **django**: the service that both builds an image and sets
-  `DJANGO_SETTINGS_MODULE`. A worker sharing the same build and `env_file`
+  `DJANGO_SETTINGS_MODULE`. Where more than one does — a worker beside the web
-  matches too, so the one publishing ports wins
+  service, say — the one publishing ports wins
 
 If nothing matches, or two candidates cannot be told apart, `ahab` says so and
-lists the services it looked at rather than guessing.
+lists the services it looked at rather than guessing. Two services that both
+build and set the settings module are ambiguous unless exactly one of them
+publishes ports.
 
 `POSTGRES_USER` and `POSTGRES_DB` are read off the detected postgres service, so
 `dropdb`, `createdb`, `pg_restore` and `pg_dump` use the role and database the
-project declares, falling back to `db` when it declares neither.
+project declares, falling back to `db` when it declares neither. Both have to
+be names: `ahab` hands them to those tools as a role and a database, and libpq
+reads a database name holding an `=` or a url as a whole connection string,
+which would send a dump to whatever server it names. A value that could be read
+as something other than a name is refused rather than passed on.
 
 ## django
 
@@ -138,6 +144,10 @@ ahab postgres import <path>          # drop, create, then restore
 ahab postgres psql <args>            # psql in the database container
 ```
 
+A dump is written `0600` and under a name of its own until it is complete, then
+renamed over the target: a cluster dump carries every role's password hash, and
+the default `0644` would hand it to anyone else with an account on the machine.
+
 `psql` passes its arguments through and only asks docker for a terminal when it
 has one to hand over, so both of these work:
 
@@ -155,9 +165,14 @@ A whole cluster dump from `pg_dumpall` is recognised by its header and handled
 differently again: it creates its own databases and carries role statements, so
 the database is dropped but not recreated, the dump goes to `psql` connected to
 `postgres`, and it runs without `ON_ERROR_STOP` because roles that already exist
-report errors that are expected. Gzipped dumps are decompressed on the way in,
+report errors that are expected. Any error it does not expect is reported and
-whichever of the three they hold. A cluster that has never held the database
+fails the import, since psql without `ON_ERROR_STOP` exits 0 having applied
-yet is a valid target either way, so importing into a fresh one works.
+only part of the dump. Gzipped dumps are decompressed on the way in, whichever
+of the three they hold. A cluster that has never held the database yet is a
+valid target either way, so importing into a fresh one works.
+
+`import` stops the project before it starts, so an import that fails leaves it
+stopped and says so: `docker compose up` brings it back.
 
 ## link
 
@@ -200,10 +215,24 @@ there is nothing to undo for a path whose symlink is gone.
 end with a NUL, and the two paths of a symlink leading elsewhere are separated
 by one as well, the way `git status -z` reports a rename.
 
+`check` looks at tracked paths too, since git tracks symlinks and one can lead
+out of the repository without appearing in any untracked listing. `add` cannot
+externalize a tracked path, so all `check` can do is report it, under the code
+`T>`.
+
+A symlink already leading out of the repository is not something `add` will
+take: moving the link would move the pointer and leave the contents where they
+are, so the store would hold a way back out while `check`, seeing a link into
+the store, called the repository clean.
+
 The store lives under
 `${XDG_DATA_HOME:-$HOME/.local/share}/ahab/<host>/<owner>/<repo>/`, derived
 from the git `origin` remote, or `_local/<checkout>` when there is no remote to
-name it after.
+name it after. Its directories are created `0700`: it exists to hold what
+should not be readable from the repository, and on a shared machine the default
+`0755` would leave that to whoever else has an account. A store path derived
+from a remote, or a checkout name, that needed characters replacing carries a
+short fingerprint of the original, so two of them cannot land on one directory.
 
 ## configuration
 

build.rs

@@ -25,6 +25,14 @@ fn install_dir(shell: Shell) -> Option<(PathBuf, bool)> {
 }
 
 fn main() -> Result<(), Error> {
+    // naming any rerun-if condition replaces cargo's default of re-running the
+    // script whenever anything in the package changed, so everything that
+    // shapes a completion script has to be named here. without them an
+    // installed script goes stale against the binary it completes
+    println!("cargo::rerun-if-changed=build.rs");
+    println!("cargo::rerun-if-changed=src/cli");
+    println!("cargo::rerun-if-changed=Cargo.toml");
+
     println!("cargo::rerun-if-env-changed=SHELL_COMPLETIONS_DIR");
     for shell in Shell::value_variants() {
         println!(
@@ -40,7 +48,18 @@ fn main() -> Result<(), Error> {
             continue;
         };
 
-        fs::create_dir_all(&dir)?;
+        // a build script's working directory is the package root, so a relative
+        // dir would quietly install into the source tree and one holding `..`
+        // somewhere else again. only a path that says where it means is taken
+        if requested && !dir.is_absolute() {
+            return Err(Error::other(format!(
+                "the completions directory must be an absolute path, not {}",
+                dir.display()
+            )));
+        }
+
+        fs::create_dir_all(&dir)
+            .map_err(|e| Error::other(format!("creating {}: {e}", dir.display())))?;
         let path = generate_to(*shell, &mut cmd, env!("CARGO_PKG_NAME"), &dir)?;
 
         if requested {

src/cli/django.rs

@@ -9,7 +9,7 @@ pub enum Django {
     /// Start a bash session in a fresh django container
     Bash,
 
-    /// Prepare empty management command 'command' in app 'app'
+    /// Prepare an empty management command NAME in the app at APP
     MakeCommand { app: PathBuf, name: String },
 
     /// Run Django's manage.py makemigrations

src/cli/link.rs

@@ -20,6 +20,10 @@ pub enum Link {
 
     /// Move paths in the store back into the repository
     Restore {
+        // said here rather than checked at runtime, so a mistake in the
+        // arguments is reported as one, with the usage and the exit code clap
+        // gives every other argument error
+        #[arg(required_unless_present = "all", conflicts_with = "all")]
         paths: Vec<PathBuf>,
 
         /// Restore every path this repository has in the store

src/cmd/argv.rs

@@ -1,7 +1,8 @@
 use std::{
+    ffi::{OsStr, OsString},
     fmt::Display,
     fs::File,
-    os::unix::process::CommandExt,
+    os::unix::process::{CommandExt, ExitStatusExt},
     path::Path,
     process::{Command, ExitStatus, Output, Stdio},
 };
@@ -9,10 +10,17 @@ use std::{
 use anyhow::{Context, Result, anyhow};
 
 use crate::ctx::Ctx;
+use crate::output::write_err;
 
 // a program and its arguments: the only thing in ahab that knows what argv looks like
 #[derive(Default, Clone)]
-pub struct Argv(Vec<String>);
+pub struct Argv(Vec<OsString>);
+
+// a dry run spawned nothing, so there is no status; nothing ran and nothing
+// failed, which is what every caller checking one of these wants to hear
+fn planned() -> ExitStatus {
+    ExitStatus::from_raw(0)
+}
 
 impl Display for Argv {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
@@ -21,42 +29,57 @@ impl Display for Argv {
 }
 
 impl Argv {
-    pub fn new(program: &str) -> Self {
+    pub fn new(program: impl AsRef<OsStr>) -> Self {
-        Self(vec![program.to_string()])
+        Self(vec![program.as_ref().to_owned()])
     }
 
-    pub fn arg(mut self, arg: impl AsRef<str>) -> Self {
+    pub fn arg(mut self, arg: impl AsRef<OsStr>) -> Self {
-        self.0.push(arg.as_ref().to_string());
+        self.0.push(arg.as_ref().to_owned());
         self
     }
 
     pub fn args<I, S>(mut self, args: I) -> Self
     where
         I: IntoIterator<Item = S>,
-        S: AsRef<str>,
+        S: AsRef<OsStr>,
     {
         self.0
-            .extend(args.into_iter().map(|arg| arg.as_ref().to_string()));
+            .extend(args.into_iter().map(|arg| arg.as_ref().to_owned()));
         self
     }
 
     // a long option and the value it takes
-    pub fn flag(self, name: &str, value: impl AsRef<str>) -> Self {
+    pub fn flag(self, name: &str, value: impl AsRef<OsStr>) -> Self {
         self.arg(name).arg(value)
     }
 
-    pub fn program(&self) -> &str {
+    // for a message that names the tool being run, so lossy is what is wanted
-        self.0.first().map(String::as_str).unwrap_or_default()
+    pub fn program(&self) -> String {
+        self.0
+            .first()
+            .map(|word| word.to_string_lossy().into_owned())
+            .unwrap_or_default()
     }
 
-    pub fn words(&self) -> &[String] {
+    pub fn words(&self) -> &[OsString] {
         &self.0
     }
 
-    // one word list for `sh -c`, quoted so a value with a space survives the shell
+    // one word list for `sh -c`, quoted so a value with a space survives the
+    // shell. lossy: this is for reading, and for scripts built only out of the
+    // ascii and compose-derived words that reach a pipeline
     pub fn quoted(&self) -> String {
-        // a nul byte is the only thing shlex refuses, and no argv can hold one
+        let words: Vec<String> = self
-        shlex::try_join(self.0.iter().map(String::as_str)).unwrap_or_default()
+            .0
+            .iter()
+            .map(|word| word.to_string_lossy().into_owned())
+            .collect();
+
+        // a nul byte is the only thing shlex refuses, and no argv can hold one,
+        // so this only happens for a word that could never have been run. it
+        // still has to be readable: an empty string would name no command at
+        // all in the very message explaining what went wrong
+        shlex::try_join(words.iter().map(String::as_str)).unwrap_or_else(|_| words.join(" "))
     }
 
     pub fn run(&self, ctx: &Ctx) -> Result<()> {
@@ -71,6 +94,14 @@ impl Argv {
 
     // reading changes nothing, so a dry run answers the question for real
     pub fn capture(&self, ctx: &Ctx) -> Result<String> {
+        let stdout = self.capture_bytes(ctx)?;
+
+        String::from_utf8(stdout).with_context(|| format!("reading the output of `{self}`"))
+    }
+
+    // git's -z listings are raw bytes: a path on unix is bytes, not text, and
+    // one that is not utf-8 would otherwise fail the whole listing it appears in
+    pub fn capture_bytes(&self, ctx: &Ctx) -> Result<Vec<u8>> {
         let out = self.command(ctx)?.output()?;
 
         if !out.status.success() {
@@ -84,7 +115,7 @@ impl Argv {
             });
         }
 
-        Ok(String::from_utf8(out.stdout)?)
+        Ok(out.stdout)
     }
 
     // replaces this process, so the command's exit code and signals become ours
@@ -110,10 +141,18 @@ impl Argv {
 
     // the status rather than an error, for callers with something better to say
     pub fn status(&self, ctx: &Ctx) -> Result<ExitStatus> {
+        if self.skipped(ctx) {
+            return Ok(planned());
+        }
+
         Ok(self.command(ctx)?.spawn()?.wait()?)
     }
 
     pub fn stdin_from(&self, ctx: &Ctx, input: &Path) -> Result<ExitStatus> {
+        if self.skipped(ctx) {
+            return Ok(planned());
+        }
+
         let stdin = self.opened(input)?;
 
         Ok(self.command(ctx)?.stdin(stdin).spawn()?.wait()?)
@@ -121,24 +160,41 @@ impl Argv {
 
     // both streams held back, for callers that read the command's complaints
     pub fn stdin_from_captured(&self, ctx: &Ctx, input: &Path) -> Result<Output> {
+        if self.skipped(ctx) {
+            return Ok(Output {
+                status: planned(),
+                stdout: Vec::new(),
+                stderr: Vec::new(),
+            });
+        }
+
         let stdin = self.opened(input)?;
 
         Ok(self.command(ctx)?.stdin(stdin).output()?)
     }
 
+    // a probe that reads its input: like capture, it is asking rather than
+    // changing, so a dry run gets the real answer and plans with it
+    pub fn probe_with_stdin(&self, ctx: &Ctx, input: &Path) -> Result<Output> {
+        let stdin = self.opened(input)?;
+
+        Ok(self.command(ctx)?.stdin(stdin).output()?)
+    }
+
+    // a probe: it asks a question rather than changing anything, so a dry run
+    // answers it for real, and the status is the answer instead of an error
+    pub fn probe_status(&self, ctx: &Ctx) -> Result<ExitStatus> {
+        Ok(self.command(ctx)?.stdout(Stdio::null()).spawn()?.wait()?)
+    }
+
     // whether it succeeded, without failure being an error
     pub fn quietly_succeeds(&self, ctx: &Ctx) -> Result<bool> {
-        Ok(self
+        Ok(self.probe_status(ctx)?.success())
-            .command(ctx)?
-            .stdout(Stdio::null())
-            .spawn()?
-            .wait()?
-            .success())
     }
 
     fn command(&self, ctx: &Ctx) -> Result<Command> {
         if ctx.verbose {
-            eprintln!("running `{self}`");
+            write_err(format_args!("running `{self}`"));
         }
 
         let (program, rest) = self.0.split_first().context("empty command")?;
@@ -150,7 +206,8 @@ impl Argv {
 
     fn skipped(&self, ctx: &Ctx) -> bool {
         if ctx.dry_run {
-            eprintln!("would run `{self}`");
+            // the plan is what a dry run was asked for, so --quiet keeps it
+            write_err(format_args!("would run `{self}`"));
             return true;
         }
 

src/cmd/compose.rs

@@ -15,6 +15,7 @@ fn compose(quiet: bool) -> Argv {
 pub struct Run {
     service: String,
     inner: Argv,
+    quiet: bool,
 }
 
 impl Run {
@@ -22,13 +23,21 @@ impl Run {
         Self {
             service: service.to_string(),
             inner,
+            quiet: false,
         }
     }
+
+    // compose narrates the container it creates before handing over, which is
+    // progress along the way rather than what was asked for
+    pub fn quiet(mut self, quiet: bool) -> Self {
+        self.quiet = quiet;
+        self
+    }
 }
 
 impl Cmd for Run {
     fn argv(&self) -> Argv {
-        compose(false)
+        compose(self.quiet)
             .arg("run")
             .arg("--rm")
             .arg(&self.service)

src/cmd/docker.rs

@@ -1,3 +1,6 @@
+use std::ffi::OsString;
+use std::path::Path;
+
 use super::{Argv, Cmd};
 
 // docker exec, around a command that runs inside the container
@@ -47,24 +50,25 @@ impl Cmd for Exec {
     }
 }
 
-// docker cp, in either direction
+// docker cp, in either direction. the local side stays an OsString: it is a
+// path the user named, and a lossy one would copy something else
 pub struct Cp {
-    from: String,
+    from: OsString,
-    to: String,
+    to: OsString,
 }
 
 impl Cp {
-    pub fn into_container(local: &str, container: &str, remote: &str) -> Self {
+    pub fn into_container(local: &Path, container: &str, remote: &str) -> Self {
         Self {
-            from: local.to_string(),
+            from: local.as_os_str().to_owned(),
-            to: format!("{container}:{remote}"),
+            to: OsString::from(format!("{container}:{remote}")),
         }
     }
 
-    pub fn out_of_container(container: &str, remote: &str, local: &str) -> Self {
+    pub fn out_of_container(container: &str, remote: &str, local: &Path) -> Self {
         Self {
-            from: format!("{container}:{remote}"),
+            from: OsString::from(format!("{container}:{remote}")),
-            to: local.to_string(),
+            to: local.as_os_str().to_owned(),
         }
     }
 }

src/cmd/git.rs

@@ -1,10 +1,22 @@
+use std::ffi::OsString;
 use std::path::Path;
 
 use super::{Argv, Cmd};
 
 // -C has no long form; it runs git as though from that directory
 fn git(root: &Path) -> Argv {
-    Argv::new("git").arg("-C").arg(root.to_string_lossy())
+    Argv::new("git").arg("-C").arg(root)
+}
+
+// `--` stops git reading a leading dash as an option, but pathspec magic is
+// read after it too, so a file actually named `:(exclude)x` or `:!x` would ask
+// about a different set of paths than the one in hand. built as an OsString: a
+// path is bytes, and a lossy one asks git about a name nothing on disk has
+fn literal(path: &Path) -> OsString {
+    let mut spec = OsString::from(":(literal)");
+    spec.push(path);
+
+    spec
 }
 
 // the root of the repository the working directory is in
@@ -31,7 +43,7 @@ pub struct LsFiles<'a> {
     root: &'a Path,
     tracked: bool,
     ignored: bool,
-    pathspecs: Vec<String>,
+    pathspecs: Vec<OsString>,
 }
 
 impl<'a> LsFiles<'a> {
@@ -64,7 +76,7 @@ impl<'a> LsFiles<'a> {
     pub fn limited_to<P: AsRef<Path>>(mut self, pathspecs: &[P]) -> Self {
         self.pathspecs = pathspecs
             .iter()
-            .map(|path| path.as_ref().to_string_lossy().to_string())
+            .map(|path| literal(path.as_ref()))
             .collect();
         self
     }
@@ -72,15 +84,14 @@ impl<'a> LsFiles<'a> {
 
 impl Cmd for LsFiles<'_> {
     fn argv(&self) -> Argv {
-        let mut argv = git(self.root).arg("ls-files");
+        // -z has no long form; it separates the paths with NUL, which is the
+        // only separator a filename cannot contain
+        let mut argv = git(self.root).arg("ls-files").arg("-z");
 
         if self.tracked {
             argv = argv.arg("--cached");
         } else {
-            // -z has no long form; it separates the paths with NUL, which is the
-            // only separator a filename cannot contain
             argv = argv
-                .arg("-z")
                 .arg("--others")
                 .arg("--exclude-standard")
                 .arg("--directory")
@@ -113,7 +124,7 @@ impl Cmd for CheckIgnore<'_> {
             .arg("check-ignore")
             .arg("--quiet")
             .arg("--")
-            .arg(self.path.to_string_lossy())
+            .arg(self.path)
     }
 }
 
@@ -134,7 +145,7 @@ mod tests {
         assert_eq!(
             argv.quoted(),
             "git -C /repo ls-files -z --others --exclude-standard --directory \
-             --no-empty-directory --ignored -- 'a b'"
+             --no-empty-directory --ignored -- ':(literal)a b'",
         );
     }
 
@@ -144,7 +155,25 @@ mod tests {
             .limited_to(&[PathBuf::from(".env")])
             .argv();
 
-        assert_eq!(argv.quoted(), "git -C /repo ls-files --cached -- .env");
+        assert_eq!(
+            argv.quoted(),
+            "git -C /repo ls-files -z --cached -- ':(literal).env'"
+        );
+    }
+
+    #[test]
+    fn a_pathspec_is_asked_for_literally_however_it_is_spelled() {
+        // git reads magic after `--` too, so these names must not become one
+        for name in [":(exclude)secret", ":!secret", ":(glob)**"] {
+            let argv = LsFiles::tracked(Path::new("/repo"))
+                .limited_to(&[PathBuf::from(name)])
+                .argv();
+
+            assert_eq!(
+                argv.words().last().unwrap(),
+                format!(":(literal){name}").as_str()
+            );
+        }
     }
 
     #[test]

src/cmd/mod.rs

@@ -55,6 +55,10 @@ pub trait Cmd {
         self.argv().capture(ctx)
     }
 
+    fn capture_bytes(&self, ctx: &Ctx) -> Result<Vec<u8>> {
+        self.argv().capture_bytes(ctx)
+    }
+
     fn replace(&self, ctx: &Ctx) -> Result<()> {
         self.argv().replace(ctx)
     }
@@ -76,7 +80,15 @@ pub trait Cmd {
         self.argv().stdin_from_captured(ctx, input)
     }
 
+    fn probe_with_stdin(&self, ctx: &Ctx, input: &Path) -> Result<Output> {
+        self.argv().probe_with_stdin(ctx, input)
+    }
+
     fn quietly_succeeds(&self, ctx: &Ctx) -> Result<bool> {
         self.argv().quietly_succeeds(ctx)
     }
+
+    fn probe_status(&self, ctx: &Ctx) -> Result<ExitStatus> {
+        self.argv().probe_status(ctx)
+    }
 }

src/cmd/postgres.rs

@@ -25,6 +25,9 @@ impl Cmd for DropDb<'_> {
         Argv::new("dropdb")
             .flag("--username", self.username)
             .arg("--if-exists")
+            // a database name is a positional, and getopt would read one
+            // beginning with a dash as an option instead
+            .arg("--")
             .arg(self.dbname)
     }
 }
@@ -42,6 +45,7 @@ impl Cmd for CreateDb<'_> {
             .flag("--username", self.username)
             .flag("--encoding", "utf8")
             .flag("--template", "template0")
+            .arg("--")
             .arg(self.dbname)
     }
 }
@@ -76,7 +80,7 @@ impl Cmd for PgRestore<'_> {
             .flag("--dbname", self.dbname);
 
         match self.from {
-            Some(path) => argv.arg(path),
+            Some(path) => argv.arg("--").arg(path),
             None => argv,
         }
     }
@@ -179,6 +183,7 @@ impl Cmd for PgDump<'_> {
             Some(path) => argv.flag("--file", path),
             None => argv,
         }
+        .arg("--")
         .arg(self.dbname)
     }
 }
@@ -208,7 +213,7 @@ mod tests {
             }
             .argv()
             .quoted(),
-            "dropdb --username u --if-exists db"
+            "dropdb --username u --if-exists -- db"
         );
     }
 
@@ -216,11 +221,11 @@ mod tests {
     fn a_directory_dump_names_the_file_it_writes() {
         assert_eq!(
             PgDump::new("u", "db", "d").to("/tmp/dump").argv().quoted(),
-            "pg_dump --username u --format d --file /tmp/dump db"
+            "pg_dump --username u --format d --file /tmp/dump -- db"
         );
         assert_eq!(
             PgDump::new("u", "db", "c").argv().quoted(),
-            "pg_dump --username u --format c db"
+            "pg_dump --username u --format c -- db"
         );
     }
 

src/commands/completions.rs

@@ -27,15 +27,24 @@ fn script(shell: Shell) -> Vec<u8> {
 
 #[cfg(test)]
 mod tests {
-    use super::{Shell, script};
+    use super::{Ahab, Shell, script};
-    use clap::ValueEnum;
+    use clap::{CommandFactory, ValueEnum};
 
     #[test]
     fn every_shell_gets_a_script_covering_the_subcommands() {
+        // asked of the parser rather than listed here, which is a list that
+        // silently stops covering the newest command
+        let subcommands: Vec<String> = Ahab::command()
+            .get_subcommands()
+            .map(|sub| sub.get_name().to_string())
+            .collect();
+
+        assert!(subcommands.len() > 1, "{subcommands:?}");
+
         for shell in Shell::value_variants() {
             let out = String::from_utf8(script(*shell)).expect("script is utf8");
 
-            for subcommand in ["django", "postgres", "link", "completions"] {
+            for subcommand in &subcommands {
                 assert!(
                     out.contains(subcommand),
                     "{shell:?} script never mentions {subcommand}"

src/commands/django.rs

@@ -1,4 +1,4 @@
-use std::path::{Path, PathBuf};
+use std::path::Path;
 
 use anyhow::{Result, anyhow};
 
@@ -16,9 +16,20 @@ class Command(BaseCommand):
 
 "#;
 
-pub fn make_command(ctx: &Ctx, app: &PathBuf, name: &str) -> Result<()> {
+pub fn make_command(ctx: &Ctx, app: &Path, name: &str) -> Result<()> {
     let app_name = app.to_string_lossy();
-    let app_dir = Path::new(&app);
+    let app_dir = app;
+
+    // it becomes `<name>.py` under the app, and django imports it by this name,
+    // so anything that is not an identifier would land the file somewhere else
+    // or somewhere django will never look for it
+    if !is_module_name(name) {
+        return Err(anyhow!(
+            "{name:?} is not a usable command name; \
+             django imports it as a python module, so it can hold only \
+             letters, digits and underscores"
+        ));
+    }
 
     if !app_dir.is_dir() {
         return Err(anyhow!("directory {app_name} does not exist"));
@@ -54,16 +65,30 @@ pub fn make_command(ctx: &Ctx, app: &PathBuf, name: &str) -> Result<()> {
     Ok(())
 }
 
+fn is_module_name(name: &str) -> bool {
+    !name.is_empty()
+        && !name.starts_with(|c: char| c.is_ascii_digit())
+        && name.chars().all(|c| c.is_ascii_alphanumeric() || c == '_')
+}
+
 pub fn bash(ctx: &Ctx) -> Result<()> {
-    Bash.in_service(&service(ctx)?).replace(ctx)
+    Bash.in_service(&service(ctx)?)
+        .quiet(ctx.quiet)
+        .replace(ctx)
 }
 
 pub fn run(ctx: &Ctx, rest: &[String]) -> Result<()> {
-    Words::new(rest).in_service(&service(ctx)?).replace(ctx)
+    Words::new(rest)
+        .in_service(&service(ctx)?)
+        .quiet(ctx.quiet)
+        .replace(ctx)
 }
 
 pub fn manage(ctx: &Ctx, rest: &[String]) -> Result<()> {
-    Manage::new(rest).in_service(&service(ctx)?).replace(ctx)
+    Manage::new(rest)
+        .in_service(&service(ctx)?)
+        .quiet(ctx.quiet)
+        .replace(ctx)
 }
 
 // shortcuts
@@ -85,3 +110,20 @@ pub fn shell(ctx: &Ctx) -> Result<()> {
 fn service(ctx: &Ctx) -> Result<String> {
     Project::resolve(ctx)?.django()
 }
+
+#[cfg(test)]
+mod tests {
+    use super::is_module_name;
+
+    #[test]
+    fn a_command_name_has_to_be_a_python_module_name() {
+        for name in ["report", "send_mail", "_private", "sync2"] {
+            assert!(is_module_name(name), "should be usable: {name}");
+        }
+
+        // a path would put the file somewhere other than the app
+        for name in ["../../../etc/cron.d/x", "a/b", "with space", "dash-ed", ""] {
+            assert!(!is_module_name(name), "should be refused: {name}");
+        }
+    }
+}

src/commands/link.rs

@@ -7,14 +7,18 @@ use fs_err::{read_dir, read_link};
 use std::cell::Cell;
 use std::path::{Path, PathBuf};
 
-use anyhow::{Result, anyhow, bail};
+use anyhow::{Context, Result, anyhow, bail};
 
-use self::store::{Repo, ignored, resolve, symlink_metadata_opt, tracked};
+use self::store::{Leads, Repo, ignored, leads, resolve, symlink_metadata_opt, tracked};
 use crate::ctx::Ctx;
-use crate::fsops::{move_path, place_link, prune_empty, remove_file, rename, suffixed};
+use crate::fsops::{
+    ensure_private_parent, move_path, place_link, prune_empty, remove_file, rename, suffixed,
+};
 use crate::output::{line, note, warning};
 
 const BACKUP_SUFFIX: &str = ".ahab-bak";
+// where the link waits while the payload comes back out of the store
+const RESTORING_SUFFIX: &str = ".ahab-restoring";
 
 // move untracked paths out of the repo and symlink them back
 pub fn add(ctx: &Ctx, paths: &[PathBuf], force: bool, store: Option<&Path>) -> Result<()> {
@@ -28,7 +32,7 @@ pub fn add(ctx: &Ctx, paths: &[PathBuf], force: bool, store: Option<&Path>) -> R
     let mut failed = 0;
     for path in paths {
         if let Err(e) = link_one(ctx, &repo, path, force, &report) {
-            note!(ctx, "error: {e:#}");
+            warning!("{e:#}");
             failed += 1;
         }
     }
@@ -44,11 +48,11 @@ pub fn restore(ctx: &Ctx, paths: &[PathBuf], all: bool, store: Option<&Path>) ->
     let repo = Repo::discover(ctx, store)?;
     let report = Report::new(&repo);
 
-    let stored = match (all, paths) {
+    // clap requires one or the other and refuses both, so only the two real
-        (true, []) => stored_paths(&repo, &repo.store)?,
+    // cases are left here
-        (true, _) => bail!("--all restores everything, so it takes no paths"),
+    let stored = match all {
-        (false, []) => bail!("name a path to restore, or pass --all"),
+        true => stored_paths(&repo, &repo.store)?,
-        (false, paths) => paths.iter().cloned().map(|p| (p, Stored::Linked)).collect(),
+        false => paths.iter().cloned().map(|p| (p, Stored::Linked)).collect(),
     };
 
     // only a linked path can be moved back; the store can hold orphans too
@@ -88,7 +92,7 @@ pub fn restore(ctx: &Ctx, paths: &[PathBuf], all: bool, store: Option<&Path>) ->
     let mut failed = 0;
     for path in &linked {
         if let Err(e) = restore_one(ctx, &repo, path, &report) {
-            note!(ctx, "error: {e:#}");
+            warning!("{e:#}");
             failed += 1;
         }
     }
@@ -126,8 +130,27 @@ fn restore_one(ctx: &Ctx, repo: &Repo, path: &Path, report: &Report) -> Result<(
         bail!("{} is missing from the store", rel.display());
     }
 
-    remove_file(ctx, &src)?;
+    // the link is moved aside rather than removed: if the payload cannot come
-    move_path(ctx, &stored, &src)?;
+    // back out of the store, the repository is left pointing at where it still is
+    let aside = suffixed(&src, RESTORING_SUFFIX);
+    if symlink_metadata_opt(&aside)?.is_some() {
+        bail!("{} is in the way; move it aside", aside.display());
+    }
+
+    rename(ctx, &src, &aside)?;
+
+    if let Err(e) = move_path(ctx, &stored, &src) {
+        rename(ctx, &aside, &src).with_context(|| {
+            format!(
+                "could not put the link at {} back after failing to restore it",
+                src.display()
+            )
+        })?;
+
+        return Err(e);
+    }
+
+    remove_file(ctx, &aside)?;
     prune_empty(ctx, stored.parent(), &repo.base);
 
     report.line("restored", &rel);
@@ -158,9 +181,13 @@ fn stored_paths(repo: &Repo, dir: &Path) -> Result<Vec<(PathBuf, Stored)>> {
 
     for entry in entries {
         let stored = entry?.path();
-        let rel = stored
+        let rel = stored.strip_prefix(&repo.store).with_context(|| {
-            .strip_prefix(&repo.store)
+            format!(
-            .expect("walked out of the store");
+                "{} is not under the store {}",
+                stored.display(),
+                repo.store.display()
+            )
+        })?;
         let src = repo.root.join(rel);
 
         let state = match symlink_metadata_opt(&src)? {
@@ -170,8 +197,12 @@ fn stored_paths(repo: &Repo, dir: &Path) -> Result<Vec<(PathBuf, Stored)>> {
             Some(_) => Stored::Taken,
         };
 
+        // a symlink the store happens to hold is a leaf, never a directory to
+        // walk into: is_dir would follow it and list whatever it points at
+        let holds_dir = symlink_metadata_opt(&stored)?.is_some_and(|meta| meta.is_dir());
+
         // a linked directory is one entry; otherwise the paths inside it are
-        if state == Stored::Linked || !stored.is_dir() {
+        if state == Stored::Linked || !holds_dir {
             found.push((src, state));
         } else {
             found.extend(stored_paths(repo, &stored)?);
@@ -249,22 +280,37 @@ fn link_one(ctx: &Ctx, repo: &Repo, path: &Path, force: bool, report: &Report) -
     let rel = repo.relative(&src)?;
     let target = repo.store.join(&rel);
 
-    // a target inside the repo would be readable from the sandbox anyway
+    // a target inside the repo would be readable from the sandbox anyway. the
-    if target.starts_with(&repo.root) {
+    // base is resolved as well as compared: one symlinked into the checkout
+    // passes a prefix test while landing the file straight back inside it
+    if target.starts_with(&repo.root) || matches!(leads(&repo.base, &repo.root), Leads::Inside) {
         return Err(anyhow!(
-            "target {} is inside the repository; point AHAB_LINK_ROOT elsewhere",
+            "the store at {} is inside the repository {}; point --store or \
-            target.display()
+             AHAB_LINK_ROOT somewhere else",
+            repo.base.display(),
+            repo.root.display()
         ));
     }
 
+    stays_in_store(repo, &rel)?;
+    // before anything is moved in, so the tree it lands in is never briefly
+    // readable by anyone else
+    ensure_private_parent(ctx, &repo.base, &target)?;
+
     if tracked(ctx, repo, &rel)? {
         return Err(anyhow!(
             "{} is tracked by git; only untracked or ignored paths can be externalized",
             rel.display()
         ));
     }
-    if !ignored(ctx, repo, &rel) {
+    match ignored(ctx, repo, &rel) {
-        warning!("{} is not gitignored", rel.display());
+        Ok(true) => {}
+        Ok(false) => warning!("{} is not gitignored", rel.display()),
+        // saying "not gitignored" here would be an answer git never gave
+        Err(e) => warning!(
+            "could not tell whether {} is gitignored: {e:#}",
+            rel.display()
+        ),
     }
 
     let src_meta = symlink_metadata_opt(&src)?;
@@ -288,6 +334,19 @@ fn link_one(ctx: &Ctx, repo: &Repo, path: &Path, force: bool, report: &Report) -
 
             // nothing in the store to adopt, so the symlink itself moves out
             if !target_taken {
+                // moving the link moves the pointer and leaves the contents
+                // where they are, so the store would hold a way back out and
+                // check, seeing a link into the store, would call it clean
+                if let Leads::Outside(end) = leads(&src, &repo.root) {
+                    return Err(anyhow!(
+                        "{} is a symlink to {}, outside the repository; \
+                         externalizing it would move the link and leave its \
+                         contents there, so repoint or remove it instead",
+                        rel.display(),
+                        end.display()
+                    ));
+                }
+
                 if !src.exists() {
                     warning!(
                         "{} is a broken symlink to {}",
@@ -295,8 +354,7 @@ fn link_one(ctx: &Ctx, repo: &Repo, path: &Path, force: bool, report: &Report) -
                         dest.display()
                     );
                 }
-                move_path(ctx, &src, &target)?;
+                move_and_link(ctx, &src, &target)?;
-                place_link(ctx, &src, &target)?;
                 report.line("moved", &rel);
                 return Ok(());
             }
@@ -332,8 +390,7 @@ fn link_one(ctx: &Ctx, repo: &Repo, path: &Path, force: bool, report: &Report) -
         }
 
         Some(_) => {
-            move_path(ctx, &src, &target)?;
+            move_and_link(ctx, &src, &target)?;
-            place_link(ctx, &src, &target)?;
             report.line("moved", &rel);
             Ok(())
         }
@@ -355,6 +412,57 @@ fn link_one(ctx: &Ctx, repo: &Repo, path: &Path, force: bool, report: &Report) -
     }
 }
 
+// the two halves have to end up looking like one step: with the payload moved
+// but no link placed, the store holds a path nothing points at and the working
+// tree has lost it altogether, which is the one outcome worse than failing
+fn move_and_link(ctx: &Ctx, src: &Path, target: &Path) -> Result<()> {
+    move_path(ctx, src, target)?;
+
+    if let Err(e) = place_link(ctx, src, target) {
+        move_path(ctx, target, src).with_context(|| {
+            format!(
+                "could not put {} back after failing to link it to {}",
+                src.display(),
+                target.display()
+            )
+        })?;
+
+        return Err(e);
+    }
+
+    Ok(())
+}
+
+// creating the store directories follows any symlink already standing in them,
+// so a store that holds one would take the move somewhere else entirely; only
+// the components at or below the store are examined, since everything above it
+// is outside by definition
+fn stays_in_store(repo: &Repo, rel: &Path) -> Result<()> {
+    let mut path = repo.store.clone();
+
+    for part in rel.components() {
+        path.push(part);
+
+        match symlink_metadata_opt(&path)? {
+            // nothing here yet, so nothing below it can be followed either
+            None => return Ok(()),
+            Some(meta) if meta.is_symlink() => {
+                if let Leads::Outside(end) = leads(&path, &repo.store) {
+                    return Err(anyhow!(
+                        "the store holds {} as a symlink to {}, outside the store; \
+                         refusing to write through it",
+                        path.display(),
+                        end.display()
+                    ));
+                }
+            }
+            Some(_) => {}
+        }
+    }
+
+    Ok(())
+}
+
 fn needs_force(target: &Path) -> anyhow::Error {
     anyhow!(
         "{} already exists; pass --force to link to it",

src/commands/link/check.rs

@@ -1,12 +1,14 @@
 use fs_err::{read_dir, read_link};
+use std::ffi::OsString;
+use std::os::unix::ffi::{OsStrExt, OsStringExt};
 use std::path::{Path, PathBuf};
 
 use anyhow::{Result, anyhow};
 
-use super::store::{Repo, resolve, symlink_metadata_opt};
+use super::store::{Leads, Repo, leads, resolve, symlink_metadata_opt};
 use crate::cmd::{Cmd, LsFiles};
 use crate::ctx::Ctx;
-use crate::output::{line, text};
+use crate::output::{line, text, write_bytes};
 
 // whether anything is outside the store, which main turns into an exit code
 pub fn check(
@@ -23,17 +25,19 @@ pub fn check(
     // git lists untracked and ignored separately
     for (mark, ignored) in [(UNTRACKED, false), (IGNORED, true)] {
         for entry in list_others(ctx, &repo, ignored, &pathspecs)? {
-            let rel = PathBuf::from(entry.trim_end_matches('/'));
-
             // --directory collapses a wholly untracked dir into `dir/`
-            if entry.ends_with('/') {
+            match entry.strip_suffix(b"/") {
-                exposed.extend(walk(&repo, &rel, mark)?.1);
+                Some(dir) => exposed.extend(walk(&repo, &path_from(dir), mark)?.1),
-            } else {
+                None => exposed.extend(classify(&repo, &path_from(&entry), mark)?),
-                exposed.extend(classify(&repo, &rel, mark)?);
             }
         }
     }
 
+    // and says nothing about tracked paths, where a symlink out still counts
+    for entry in list_tracked(ctx, &repo, &pathspecs)? {
+        exposed.extend(classify_tracked(&repo, &path_from(&entry))?);
+    }
+
     exposed.sort_by(|a, b| a.name.cmp(&b.name));
 
     if porcelain || null {
@@ -46,19 +50,36 @@ pub fn check(
 }
 
 fn print_porcelain(exposed: &[Exposed], null: bool) {
-    let end = if null { '\0' } else { '\n' };
+    // -z is the format for scripts that must survive any filename, so its
-    // a filename can hold an arrow but not a NUL, as `git status -z` also assumes
+    // records are written as the bytes a path actually is. a filename can hold
-    let between = if null { "\0" } else { " -> " };
+    // an arrow but not a NUL, as `git status -z` also assumes
+    if null {
+        for item in exposed {
+            let mut record = item.code().into_bytes();
+            record.push(b' ');
+            record.extend_from_slice(item.name.as_os_str().as_bytes());
+
+            if let Some(dest) = &item.dest {
+                record.push(0);
+                record.extend_from_slice(dest.as_os_str().as_bytes());
+            }
+
+            record.push(0);
+            write_bytes(&record);
+        }
+
+        return;
+    }
 
     for item in exposed {
         match &item.dest {
             Some(dest) => text!(
-                "{} {}{between}{}{end}",
+                "{} {} -> {}\n",
                 item.code(),
-                item.name,
+                item.name.display(),
                 dest.display()
             ),
-            None => text!("{} {}{end}", item.code(), item.name),
+            None => text!("{} {}\n", item.code(), item.name.display()),
         }
     }
 }
@@ -98,8 +119,8 @@ fn print_listing(repo: &Repo, exposed: &[Exposed]) {
         line!("\n{heading}\n{hint}");
         for item in items {
             match &item.dest {
-                Some(dest) => line!("\t{} -> {}", item.name, dest.display()),
+                Some(dest) => line!("\t{} -> {}", item.name.display(), dest.display()),
-                None => line!("\t{}", item.name),
+                None => line!("\t{}", item.name.display()),
             }
         }
     }
@@ -123,15 +144,17 @@ impl Section {
 const UNTRACKED: char = '?';
 const IGNORED: char = '!';
 const ELSEWHERE: char = '>';
+// no git equivalent: a tracked path, which only ever appears as a symlink out
+const TRACKED: char = 'T';
 
 struct Exposed {
     mark: char,
-    name: String,
+    name: PathBuf,
     dest: Option<PathBuf>,
 }
 
 impl Exposed {
-    fn content(mark: char, name: String) -> Self {
+    fn content(mark: char, name: PathBuf) -> Self {
         Self {
             mark,
             name,
@@ -151,7 +174,7 @@ impl Exposed {
 
 fn classify(repo: &Repo, rel: &Path, mark: char) -> Result<Option<Exposed>> {
     let src = repo.root.join(rel);
-    let name = rel.display().to_string();
+    let name = rel.to_path_buf();
 
     let Some(meta) = symlink_metadata_opt(&src)? else {
         return Ok(None);
@@ -162,7 +185,16 @@ fn classify(repo: &Repo, rel: &Path, mark: char) -> Result<Option<Exposed>> {
 
     let dest = read_link(&src)?;
     if dest == repo.store.join(rel) {
-        return Ok(None);
+        // it names the store, but what the store holds there can be a symlink of
+        // its own leading straight back out, which is not being held at all
+        return Ok(match leads(&src, &repo.store) {
+            Leads::Inside | Leads::Dangling => None,
+            Leads::Outside(end) => Some(Exposed {
+                mark,
+                name,
+                dest: Some(end),
+            }),
+        });
     }
 
     Ok(Some(Exposed {
@@ -172,6 +204,29 @@ fn classify(repo: &Repo, rel: &Path, mark: char) -> Result<Option<Exposed>> {
     }))
 }
 
+// git tracks symlinks, so one can lead out of the repository without ever
+// showing up in an untracked or ignored listing; `add` cannot externalize it
+// either, so all check can do is say it is there
+fn classify_tracked(repo: &Repo, rel: &Path) -> Result<Option<Exposed>> {
+    let src = repo.root.join(rel);
+
+    let Some(meta) = symlink_metadata_opt(&src)? else {
+        return Ok(None);
+    };
+    if !meta.is_symlink() {
+        return Ok(None);
+    }
+
+    Ok(match leads(&src, &repo.root) {
+        Leads::Inside | Leads::Dangling => None,
+        Leads::Outside(end) => Some(Exposed {
+            mark: TRACKED,
+            name: rel.to_path_buf(),
+            dest: Some(end),
+        }),
+    })
+}
+
 fn walk(repo: &Repo, rel: &Path, mark: char) -> Result<(usize, Vec<Exposed>)> {
     let dir = repo.root.join(rel);
     let mut handled = 0;
@@ -194,10 +249,16 @@ fn walk(repo: &Repo, rel: &Path, mark: char) -> Result<(usize, Vec<Exposed>)> {
         }
     }
 
-    // nothing below is in the store, so collapse to one line
+    // nothing below is in the store, so collapse to one line -- unless some of
-    if handled == 0 && !exposed.is_empty() {
+    // it leads out of the repository, which is a different thing to report and
-        let name = format!("{}/", rel.display());
+    // carries a destination the one line would drop
-        return Ok((0, vec![Exposed::content(mark, name)]));
+    let all_content = exposed.iter().all(|item| item.dest.is_none());
+
+    if handled == 0 && !exposed.is_empty() && all_content {
+        let mut name = rel.as_os_str().to_owned();
+        name.push("/");
+
+        return Ok((0, vec![Exposed::content(mark, PathBuf::from(name))]));
     }
 
     Ok((handled, exposed))
@@ -230,16 +291,31 @@ fn list_others(
     repo: &Repo,
     ignored: bool,
     pathspecs: &[PathBuf],
-) -> Result<Vec<String>> {
+) -> Result<Vec<Vec<u8>>> {
     let mut listing = LsFiles::untracked(&repo.root).limited_to(pathspecs);
     if ignored {
         listing = listing.ignored();
     }
 
-    Ok(listing
+    Ok(split_nul(&listing.capture_bytes(ctx)?))
-        .capture(ctx)?
+}
-        .split('\0')
+
-        .filter(|p| !p.is_empty())
+fn list_tracked(ctx: &Ctx, repo: &Repo, pathspecs: &[PathBuf]) -> Result<Vec<Vec<u8>>> {
-        .map(String::from)
+    let listing = LsFiles::tracked(&repo.root).limited_to(pathspecs);
-        .collect())
+
+    Ok(split_nul(&listing.capture_bytes(ctx)?))
+}
+
+// every listing check reads is asked for with -z, and kept as the bytes git
+// wrote: a path is not obliged to be utf-8, and one that is not used to fail
+// the whole listing rather than the one entry
+fn split_nul(out: &[u8]) -> Vec<Vec<u8>> {
+    out.split(|byte| *byte == 0)
+        .filter(|record| !record.is_empty())
+        .map(<[u8]>::to_vec)
+        .collect()
+}
+
+fn path_from(bytes: &[u8]) -> PathBuf {
+    PathBuf::from(OsString::from_vec(bytes.to_vec()))
 }

src/commands/link/store.rs

@@ -1,7 +1,8 @@
 use fs_err as fs;
 use std::env;
-use std::ffi::OsString;
+use std::ffi::{OsStr, OsString};
-use std::path::{Path, PathBuf};
+use std::os::unix::ffi::{OsStrExt, OsStringExt};
+use std::path::{Component, Path, PathBuf};
 
 use anyhow::{Context, Result, anyhow};
 
@@ -22,10 +23,10 @@ pub(super) struct Repo {
 impl Repo {
     pub(super) fn discover(ctx: &Ctx, store: Option<&Path>) -> Result<Self> {
         let root = git_root(ctx)?;
-        let base = match store {
+        let base = normalized(match store {
             Some(store) => store.to_path_buf(),
             None => store_root()?,
-        };
+        })?;
 
         Ok(Self {
             store: base.join(repo_components(ctx, &root)?),
@@ -68,17 +69,44 @@ pub(super) fn resolve(path: &Path) -> Result<PathBuf> {
     Ok(parent.join(name))
 }
 
-fn git_root(ctx: &Ctx) -> Result<PathBuf> {
+// absolute, with `.` and `..` folded out. the store path is written into every
-    let root = RevParse
+// symlink `add` creates, where a relative one would resolve from the link's own
-        .capture(ctx)
+// directory rather than the working one, and it is compared against the
-        .map_err(|_| anyhow!("not inside a git repository"))?;
+// repository root, which a `..` would slip past
+fn normalized(path: PathBuf) -> Result<PathBuf> {
+    let absolute = std::path::absolute(&path)
+        .with_context(|| format!("resolving absolute path of {}", path.display()))?;
 
-    let root = root.trim().to_string();
+    let mut out = PathBuf::new();
+    for part in absolute.components() {
+        match part {
+            Component::CurDir => {}
+            Component::ParentDir => {
+                out.pop();
+            }
+            part => out.push(part),
+        }
+    }
+
+    Ok(out)
+}
+
+fn git_root(ctx: &Ctx) -> Result<PathBuf> {
+    // bytes, since the checkout can live at a path that is not utf-8, and with
+    // the cause kept: git not being installed and the directory not being a
+    // repository are different problems with the same one-line answer otherwise
+    let root = RevParse
+        .capture_bytes(ctx)
+        .context("asking git for the repository root")?;
+
+    let root = root.strip_suffix(b"\n").unwrap_or(&root);
     if root.is_empty() {
         return Err(anyhow!("git reported an empty repository root"));
     }
 
-    Ok(fs::canonicalize(&root)?)
+    Ok(fs::canonicalize(PathBuf::from(OsString::from_vec(
+        root.to_vec(),
+    )))?)
 }
 
 fn repo_components(ctx: &Ctx, root: &Path) -> Result<PathBuf> {
@@ -95,7 +123,7 @@ fn repo_components(ctx: &Ctx, root: &Path) -> Result<PathBuf> {
     let name = root
         .file_name()
         .ok_or_else(|| anyhow!("cannot derive a store path for {}", root.display()))?;
-    Ok(Path::new(LOCAL_NAMESPACE).join(sanitize(&name.to_string_lossy())))
+    Ok(Path::new(LOCAL_NAMESPACE).join(sanitize_name(name)))
 }
 
 fn git_origin_url(ctx: &Ctx) -> Option<String> {
@@ -135,7 +163,7 @@ fn components_from_remote(url: &str) -> Option<PathBuf> {
     (depth > 0).then_some(components)
 }
 
-fn sanitize(s: &str) -> String {
+fn cleaned(s: &str) -> String {
     let out: String = s
         .chars()
         .map(|c| {
@@ -148,10 +176,51 @@ fn sanitize(s: &str) -> String {
         .collect();
 
     // `.` and `..` are legal characters but not legal components
-    if out.chars().all(|c| c == '.') {
+    match out.chars().all(|c| c == '.') {
-        return "_".repeat(out.len());
+        true => "_".repeat(out.len()),
+        false => out,
     }
-    out
+}
+
+fn sanitize(s: &str) -> String {
+    let out = cleaned(s);
+
+    // every replaced character maps to the same `_`, so `my~api` and `my:api`
+    // would otherwise share one directory with the plain `my_api`. a component
+    // that came through untouched keeps its name, so the common remote keeps
+    // the store path it already has
+    match out == s {
+        true => out,
+        false => format!("{out}-{}", fingerprint(s.as_bytes())),
+    }
+}
+
+// a checkout name is bytes like any other path. a lossy rendering turns every
+// byte it cannot read into the same replacement character, so sanitize would
+// see two different names as one and fingerprint them identically: the bytes
+// themselves are what has to be fingerprinted
+fn sanitize_name(name: &OsStr) -> String {
+    match name.to_str() {
+        Some(text) => sanitize(text),
+        None => format!(
+            "{}-{}",
+            cleaned(&name.to_string_lossy()),
+            fingerprint(name.as_bytes())
+        ),
+    }
+}
+
+// fnv-1a: the store path has to stay put across rust releases, which the hashers
+// in std explicitly do not promise
+fn fingerprint(bytes: &[u8]) -> String {
+    let mut hash: u64 = 0xcbf2_9ce4_8422_2325;
+
+    for byte in bytes {
+        hash ^= u64::from(*byte);
+        hash = hash.wrapping_mul(0x100_0000_01b3);
+    }
+
+    format!("{:08x}", hash as u32)
 }
 
 fn store_root() -> Result<PathBuf> {
@@ -170,17 +239,28 @@ fn non_empty_var(name: &str) -> Option<OsString> {
 }
 
 pub(super) fn tracked(ctx: &Ctx, repo: &Repo, rel: &Path) -> Result<bool> {
+    // bytes: the listing echoes back the path asked about, which is not obliged
+    // to be utf-8, and failing to read it would refuse the path for the wrong
+    // reason rather than answering whether git tracks it
     let listed = LsFiles::tracked(&repo.root)
         .limited_to(&[rel])
-        .capture(ctx)?;
+        .capture_bytes(ctx)?;
 
     Ok(!listed.is_empty())
 }
 
-pub(super) fn ignored(ctx: &Ctx, repo: &Repo, rel: &Path) -> bool {
+// check-ignore says 0 for ignored and 1 for not; anything else means it could
-    CheckIgnore::new(&repo.root, rel)
+// not answer at all, which is not the same as "not ignored". git cannot even be
-        .quietly_succeeds(ctx)
+// asked about a path whose name looks like pathspec magic, since it rejects the
-        .unwrap_or(false)
+// magic rather than the name, and `:(literal)` is not accepted by this command
+pub(super) fn ignored(ctx: &Ctx, repo: &Repo, rel: &Path) -> Result<bool> {
+    let status = CheckIgnore::new(&repo.root, rel).probe_status(ctx)?;
+
+    match status.code() {
+        Some(0) => Ok(true),
+        Some(1) => Ok(false),
+        _ => Err(anyhow!("`git check-ignore` exited with {status}")),
+    }
 }
 
 pub(super) fn symlink_metadata_opt(path: &Path) -> Result<Option<std::fs::Metadata>> {
@@ -192,9 +272,40 @@ pub(super) fn symlink_metadata_opt(path: &Path) -> Result<Option<std::fs::Metada
     }
 }
 
+// where a chain of symlinks actually ends up
+pub(super) enum Leads {
+    // somewhere under the directory it was supposed to stay in
+    Inside,
+    // out of it, at this path
+    Outside(PathBuf),
+    // nowhere: a broken link, or too many hops for the kernel to follow
+    Dangling,
+}
+
+// comparing a link's target against an expected path only says what it claims;
+// this says where following it really arrives, which is what decides whether a
+// path is held by the store or merely points at something that is not
+pub(super) fn leads(path: &Path, root: &Path) -> Leads {
+    let Ok(end) = fs::canonicalize(path) else {
+        return Leads::Dangling;
+    };
+
+    // the root can be reached through a symlink of its own, so resolve it too
+    // rather than comparing a resolved path against an unresolved prefix
+    let root = fs::canonicalize(root).unwrap_or_else(|_| root.to_path_buf());
+
+    match end.starts_with(&root) {
+        true => Leads::Inside,
+        false => Leads::Outside(end),
+    }
+}
+
 #[cfg(test)]
 mod tests {
-    use super::{components_from_remote, sanitize};
+    use super::{components_from_remote, fingerprint, normalized, sanitize, sanitize_name};
+    use std::collections::HashSet;
+    use std::ffi::OsStr;
+    use std::os::unix::ffi::OsStrExt;
     use std::path::PathBuf;
 
     #[test]
@@ -249,9 +360,70 @@ mod tests {
 
     #[test]
     fn sanitize_never_yields_a_traversal() {
-        assert_eq!(sanitize(".."), "__");
+        for name in ["..", ".", "a/b", "../..", "a/../b"] {
-        assert_eq!(sanitize("."), "_");
+            let out = sanitize(name);
-        assert_eq!(sanitize("a/b"), "a_b");
+
+            assert!(!out.contains('/'), "{name} -> {out}");
+            assert!(out != "." && out != "..", "{name} -> {out}");
+        }
+
+        // a name that needed no replacing keeps the store path it already has
         assert_eq!(sanitize(".env"), ".env");
+        assert_eq!(sanitize("afurnik"), "afurnik");
+        assert_eq!(sanitize("git.aflabs.org"), "git.aflabs.org");
+    }
+
+    #[test]
+    fn sanitize_keeps_names_apart_that_replacing_would_collapse() {
+        // every disallowed character maps to `_`, so without the fingerprint
+        // these would all share one store directory with a plain `my_api`
+        let names = ["my~api", "my:api", "my api", "my/api", "my%api"];
+
+        for name in names {
+            assert_ne!(sanitize(name), sanitize("my_api"), "name: {name}");
+        }
+
+        let distinct: HashSet<String> = names.iter().map(|name| sanitize(name)).collect();
+        assert_eq!(distinct.len(), names.len(), "{distinct:?}");
+    }
+
+    #[test]
+    fn a_fingerprint_does_not_drift_with_the_toolchain() {
+        // std's hashers make no such promise, and a moved store loses the files
+        assert_eq!(fingerprint(b""), "84222325");
+        assert_eq!(fingerprint(b"my~api"), fingerprint(b"my~api"));
+        assert_ne!(fingerprint(b"my~api"), fingerprint(b"my:api"));
+    }
+
+    #[test]
+    fn a_checkout_name_that_is_not_utf_8_keeps_its_own_directory() {
+        // both render to the same replacement character, so a fingerprint taken
+        // of the rendering rather than the bytes cannot tell them apart
+        let one = OsStr::from_bytes(b"proj\xe9");
+        let two = OsStr::from_bytes(b"proj\xff");
+
+        assert_ne!(sanitize_name(one), sanitize_name(two));
+        // and a name that is utf-8 is keyed exactly as before
+        assert_eq!(sanitize_name(OsStr::new("afurnik")), "afurnik");
+    }
+
+    #[test]
+    fn a_store_root_is_absolute_with_the_dots_folded_out() {
+        // it is written into every symlink add creates, and compared against the
+        // repository root, so it needs exactly one spelling
+        let cases = [
+            ("/a/b/../c", "/a/c"),
+            ("/a/./b", "/a/b"),
+            ("/a/b/../../c", "/c"),
+            ("/../..", "/"),
+        ];
+
+        for (from, want) in cases {
+            assert_eq!(
+                normalized(PathBuf::from(from)).unwrap(),
+                PathBuf::from(want),
+                "from: {from}"
+            );
+        }
     }
 }

src/commands/postgres.rs

@@ -1,9 +1,8 @@
 mod server;
 mod shape;
 
-use fs_err::File;
 use std::io::{self, IsTerminal, Write};
-use std::path::Path;
+use std::path::{Path, PathBuf};
 use std::process::Stdio;
 
 use anyhow::{Context, Result, bail};
@@ -16,15 +15,29 @@ use crate::cmd::{
     Stop, Up,
 };
 use crate::ctx::Ctx;
-use crate::fsops::{remove_file, rename, suffixed};
+use crate::fsops::{create_private_new, remove_file, rename, suffixed};
-use crate::output::note;
+use crate::output::{note, warning};
 
 // unique per run: docker cp will not copy a directory over an existing path, and
 // quietly leaves whatever was there for pg_restore to read instead
 fn remote_dump() -> String {
-    format!("/tmp/ahab-dump-{}", std::process::id())
+    // the container's /tmp is shared with whatever else runs in it, and a name
+    // that can be worked out in advance is one a symlink can be planted at
+    let spun = std::time::SystemTime::now()
+        .duration_since(std::time::UNIX_EPOCH)
+        .map_or(0, |since| since.subsec_nanos());
+
+    format!("/tmp/ahab-dump-{}-{spun:09}", std::process::id())
 }
 
+// beside the target but unique to this run: a fixed name is a path a second
+// dump would share, and one a symlink can be planted at ahead of time
+fn partial_path(file: &Path) -> PathBuf {
+    suffixed(file, &format!(".{}.partial", std::process::id()))
+}
+
+// pg_dumpall recreates roles the cluster already has, so this is the expected
+// complaint rather than a failure
 fn is_existing_role_error(line: &str) -> bool {
     line.starts_with("ERROR:") && line.contains("role \"") && line.ends_with("already exists")
 }
@@ -39,14 +52,24 @@ fn restore_cluster(ctx: &Ctx, db: &Database, restore: &dyn Cmd, file: &Path) ->
     io::stdout().write_all(&out.stdout).ok();
 
     let mut existing = 0;
+    let mut failed = 0;
+
     for line in String::from_utf8_lossy(&out.stderr).lines() {
         if is_existing_role_error(line) {
             existing += 1;
             continue;
         }
 
+        // this psql runs without ON_ERROR_STOP, so it exits 0 whatever the sql
+        // did: these lines are the only account of what happened, which makes
+        // them the result rather than progress, and --quiet has to keep them
+        if line.starts_with("ERROR:") {
+            failed += 1;
+            warning!("{line}");
+        } else {
             note!(ctx, "{line}");
         }
+    }
 
     if existing > 0 {
         note!(
@@ -59,47 +82,29 @@ fn restore_cluster(ctx: &Ctx, db: &Database, restore: &dyn Cmd, file: &Path) ->
     if !out.status.success() {
         bail!("psql failed, the database is left empty");
     }
+    if failed > 0 {
+        bail!(
+            "psql reported {failed} error{}, so the cluster restored only in part",
+            if failed == 1 { "" } else { "s" }
+        );
+    }
 
     Ok(())
 }
 
-pub fn import(ctx: &Ctx, file: &Path) -> Result<()> {
+// which shape is inside the compression, read through the container's own gunzip
-    let dump = Dump::of(file)?;
+// rather than assuming the host has one. this only reads, so it is a probe and
-    let db = Database::resolve(ctx)?;
+// runs even in a dry run, where the answer decides what the plan says
+fn gzip_kind(ctx: &Ctx, db: &Database, file: &Path) -> Result<Kind> {
+    wait_until_ready(ctx, db)?;
 
-    note!(ctx, "stopping all containers");
-    Stop { quiet: ctx.quiet }.run(ctx)?;
-
-    note!(ctx, "starting db container");
-    Start {
-        service: &db.service,
-        quiet: ctx.quiet,
-    }
-    .run(ctx)?;
-
-    let remote = remote_dump();
-
-    // a directory cannot be streamed, so it is the one shape that gets copied in
-    if matches!(dump, Dump::Directory) {
-        when_ready(
-            ctx,
-            &db,
-            &Cp::into_container(&file.to_string_lossy(), &db.container, &remote),
-        )?;
-    }
-
-    let kind = match &dump {
-        Dump::Directory => Kind::Archive,
-        Dump::Header(header) => Kind::of(header),
-        Dump::Gzip => {
-            wait_until_ready(ctx, &db)?;
     let out = Gunzip
         .pipe(&Head::bytes(HEADER_LEN))
         // head closes the pipe once it has its bytes, which kills gunzip
         .allow_early_close()
         .in_container(&db.container)
         .interactive()
-                .stdin_from_captured(ctx, file)
+        .probe_with_stdin(ctx, file)
         .context("reading the compressed dump's header")?;
 
     // head exits 0 whatever gunzip did, so an empty header is the only
@@ -119,8 +124,68 @@ pub fn import(ctx: &Ctx, file: &Path) -> Result<()> {
         );
     }
 
-            Kind::of(&out.stdout)
+    Ok(Kind::of(&out.stdout))
 }
+
+pub fn import(ctx: &Ctx, file: &Path) -> Result<()> {
+    let dump = Dump::of(file)?;
+    let db = Database::resolve(ctx)?;
+
+    if matches!(dump, Dump::Directory) {
+        local_path_for_docker(file)?;
+    }
+
+    note!(ctx, "stopping all containers");
+    Stop { quiet: ctx.quiet }.run(ctx)?;
+
+    // everything from here runs with the project down, so an error leaves it
+    // that way and the user has no reason to guess as much
+    imported(ctx, &db, &dump, file)
+        .map_err(|e| e.context("the project is left stopped, `docker compose up` starts it again"))
+}
+
+// docker cp reads `container:path`, splitting on the first colon, so a local
+// path holding one is read as a container name and something else entirely
+fn local_path_for_docker(file: &Path) -> Result<()> {
+    match file.to_string_lossy().contains(':') {
+        true => bail!(
+            "{} has a colon in it, which docker cp reads as a container name; \
+             rename it to copy it in or out",
+            file.display()
+        ),
+        false => Ok(()),
+    }
+}
+
+fn imported(ctx: &Ctx, db: &Database, dump: &Dump, file: &Path) -> Result<()> {
+    note!(ctx, "starting db container");
+    Start {
+        service: &db.service,
+        quiet: ctx.quiet,
+    }
+    .run(ctx)?;
+
+    let remote = remote_dump();
+
+    // a directory cannot be streamed, so it is the one shape that gets copied in
+    if matches!(dump, Dump::Directory) {
+        when_ready(ctx, db, &Cp::into_container(file, &db.container, &remote))?;
+    }
+
+    let kind = match &dump {
+        Dump::Directory => Kind::Archive,
+        Dump::Header(header) => Kind::of(header),
+        Dump::Gzip => match gzip_kind(ctx, db, file) {
+            Ok(kind) => kind,
+            // looking inside needs a container to decompress with, and a dry run
+            // is worth printing with the project down, which is when it is most
+            // likely to be asked for
+            Err(e) if ctx.dry_run => {
+                note!(ctx, "planning for a single database dump: {e:#}");
+                Kind::Sql
+            }
+            Err(e) => return Err(e),
+        },
     };
 
     let restore = db.restore_with(kind);
@@ -130,7 +195,7 @@ pub fn import(ctx: &Ctx, file: &Path) -> Result<()> {
 
     when_ready(
         ctx,
-        &db,
+        db,
         &DropDb {
             username: &db.user,
             dbname: &db.name,
@@ -143,7 +208,7 @@ pub fn import(ctx: &Ctx, file: &Path) -> Result<()> {
     if kind != Kind::Cluster {
         when_ready(
             ctx,
-            &db,
+            db,
             &CreateDb {
                 username: &db.user,
                 dbname: &db.name,
@@ -152,23 +217,56 @@ pub fn import(ctx: &Ctx, file: &Path) -> Result<()> {
         )?;
     }
 
-    wait_until_ready(ctx, &db)?;
+    wait_until_ready(ctx, db)?;
+    let restored = restore_dump(ctx, db, dump, kind, file, &remote, &tool, restore);
+
+    // the copy inside the container holds the whole database, and its /tmp
+    // outlives the command: it goes whether or not the restore worked, which is
+    // exactly when it used to be left behind
+    if matches!(dump, Dump::Directory) {
+        let _ = Rm::recursive(&remote).in_container(&db.container).run(ctx);
+    }
+    restored?;
+
+    note!(ctx, "restarting containers");
+    Stop { quiet: ctx.quiet }.run(ctx)?;
+    Up { quiet: ctx.quiet }.run(ctx)?;
+
+    Ok(())
+}
+
+#[allow(clippy::too_many_arguments)]
+fn restore_dump(
+    ctx: &Ctx,
+    db: &Database,
+    dump: &Dump,
+    kind: Kind,
+    file: &Path,
+    remote: &str,
+    tool: &str,
+    restore: Box<dyn Cmd + '_>,
+) -> Result<()> {
     if ctx.dry_run {
         note!(ctx, "would restore with {tool}");
-    } else {
+        return Ok(());
+    }
+
     // a directory dump was copied in whole, so pg_restore reads it from the
     // container; every other shape is fed in on stdin, through gunzip when it
     // arrives compressed
     if matches!(dump, Dump::Directory) {
         let status = PgRestore::new(&db.user, &db.name)
-                .from(&remote)
+            .from(remote)
             .in_container(&db.container)
             .status(ctx)?;
 
         if !status.success() {
             bail!("{tool} failed, the database is left empty");
         }
-        } else {
+
+        return Ok(());
+    }
+
     let restore: Box<dyn Cmd> = match dump {
         Dump::Gzip => Box::new(Gunzip.pipe(&*restore)),
         _ => restore,
@@ -177,8 +275,9 @@ pub fn import(ctx: &Ctx, file: &Path) -> Result<()> {
     if kind == Kind::Cluster {
         // psql's output is read rather than streamed here, to keep the
         // expected role errors out of the way
-                restore_cluster(ctx, &db, &*restore, file)?;
+        return restore_cluster(ctx, db, &*restore, file);
-            } else {
+    }
+
     let status = restore
         .in_container(&db.container)
         .interactive()
@@ -187,17 +286,6 @@ pub fn import(ctx: &Ctx, file: &Path) -> Result<()> {
     if !status.success() {
         bail!("{tool} failed, the database is left empty");
     }
-            }
-        }
-    }
-
-    if matches!(dump, Dump::Directory) {
-        let _ = Rm::recursive(&remote).in_container(&db.container).run(ctx);
-    }
-
-    note!(ctx, "restarting containers");
-    Stop { quiet: ctx.quiet }.run(ctx)?;
-    Up { quiet: ctx.quiet }.run(ctx)?;
 
     Ok(())
 }
@@ -229,12 +317,12 @@ pub fn dump(ctx: &Ctx, file: &Path, format: Format, gzip: bool) -> Result<()> {
 
     // written beside the target and renamed once the dump succeeds, so a failure
     // cannot destroy the dump that is already there
-    let partial = suffixed(file, ".partial");
+    let partial = partial_path(file);
     // a dry run produces no dump, so it must not lay a hand on the target either
     let stdout = if ctx.dry_run {
         Stdio::null()
     } else {
-        Stdio::from(std::fs::File::from(File::create(&partial)?))
+        Stdio::from(create_private_new(&partial)?)
     };
 
     let dumping = dump_command(&db, format);
@@ -269,7 +357,9 @@ fn dump_command(db: &Database, format: Format) -> Box<dyn Cmd + '_> {
 // pg_dump writes a directory format dump itself rather than to stdout, so it lands
 // in the container and comes back with docker cp
 fn dump_directory(ctx: &Ctx, db: &Database, target: &Path) -> Result<()> {
-    if target.exists() {
+    // symlink_metadata rather than exists(), which follows the link and so is
+    // false for a dangling one: docker cp would then write through it
+    if std::fs::symlink_metadata(target).is_ok() {
         bail!(
             "{} already exists; a directory dump will not be written over it",
             target.display()
@@ -284,7 +374,7 @@ fn dump_directory(ctx: &Ctx, db: &Database, target: &Path) -> Result<()> {
         .in_container(&db.container)
         .run(ctx)?;
 
-    let copied = Cp::out_of_container(&db.container, &remote, &target.to_string_lossy()).run(ctx);
+    let copied = Cp::out_of_container(&db.container, &remote, target).run(ctx);
 
     let _ = Rm::recursive(&remote).in_container(&db.container).run(ctx);
 

src/commands/postgres/server.rs

@@ -1,7 +1,7 @@
 use std::thread;
 use std::time::{Duration, Instant};
 
-use anyhow::{Result, anyhow, bail};
+use anyhow::{Result, bail};
 
 use super::shape::Kind;
 use crate::cmd::{Cmd, PgIsReady, PgRestore, Ps, Psql};
@@ -10,6 +10,8 @@ use crate::project::Project;
 
 const READY_TIMEOUT: Duration = Duration::from_secs(60);
 const POLL_INTERVAL: Duration = Duration::from_secs(1);
+// stands in for an id a dry run has no running container to look up
+const PLANNED_CONTAINER: &str = "<db container>";
 
 pub(super) struct Database {
     pub(super) service: String,
@@ -22,13 +24,24 @@ impl Database {
     pub(super) fn resolve(ctx: &Ctx) -> Result<Self> {
         let compose = Project::resolve(ctx)?;
         let service = compose.postgres()?;
-        let (user, name) = compose.postgres_credentials(&service);
+        let (user, name) = compose.postgres_credentials(&service)?;
 
-        let container = Ps::id_of(&service).capture(ctx)?.trim().to_string();
+        let listed = Ps::id_of(&service).capture(ctx)?;
+        let mut ids = listed.lines().map(str::trim).filter(|id| !id.is_empty());
 
-        if container.is_empty() {
+        let container = match (ids.next(), ids.next()) {
-            return Err(anyhow!("service {service} has no running container"));
+            (Some(id), None) => id.to_string(),
-        }
+            // one id per line: a scaled service has several, and the whole
+            // listing would go to docker exec as though it were a single id
+            (Some(_), Some(_)) => bail!(
+                "service {service} has more than one container running; \
+                 scale it to one first"
+            ),
+            // a dry run only prints a plan, and it is worth printing with the
+            // project down, which is when it is most likely to be asked for
+            (None, _) if ctx.dry_run => PLANNED_CONTAINER.to_string(),
+            (None, _) => bail!("service {service} has no running container"),
+        };
 
         Ok(Self {
             service,

src/commands/postgres/shape.rs

@@ -55,14 +55,14 @@ impl Kind {
     }
 }
 
-// pg_dumpall recreates roles the cluster already has, so this is the expected
-
 fn read_header(path: &Path) -> Result<Vec<u8>> {
-    let mut file = File::open(path)?;
+    let file = File::open(path)?;
-    let mut header = vec![0; HEADER_LEN];
+    let mut header = Vec::with_capacity(HEADER_LEN);
 
-    let read = file.read(&mut header)?;
+    // read fills what it can rather than what was asked for, and a header that
-    header.truncate(read);
+    // arrives in more than one piece would lose the cluster marker sitting a few
+    // bytes in, leaving a whole-cluster dump looking like a single database
+    file.take(HEADER_LEN as u64).read_to_end(&mut header)?;
 
     Ok(header)
 }

src/commands/status.rs

@@ -69,7 +69,10 @@ fn role(ctx: &Ctx, role: &str, detected: Result<String>, project: &Project) {
     }
 
     if role == "postgres" {
-        let (user, database) = project.postgres_credentials(&service);
+        // status reports what it cannot work out rather than stopping, so a name
+        // the postgres tools would refuse is something to say, not to fail on
+        match project.postgres_credentials(&service) {
+            Ok((user, database)) => {
                 let source = |key: &str| match project.env(&service, key) {
                     Some(_) => "",
                     None => " (default, nothing in the environment)",
@@ -78,6 +81,9 @@ fn role(ctx: &Ctx, role: &str, detected: Result<String>, project: &Project) {
                 line!("\tuser:      {user}{}", source("POSTGRES_USER"));
                 line!("\tdatabase:  {database}{}", source("POSTGRES_DB"));
             }
+            Err(e) => line!("\tcredentials: {e:#}"),
+        }
+    }
 }
 
 // a container id is 64 characters and only the first few are ever typed

src/fsops.rs

@@ -1,7 +1,8 @@
 use std::io::Write;
+use std::os::unix::fs::{DirBuilderExt, OpenOptionsExt};
 use std::path::{Path, PathBuf};
 
-use anyhow::{Context, Result};
+use anyhow::{Context, Result, anyhow};
 // std::fs with the path and the operation already in the error
 use fs_err as fs;
 use fs_err::os::unix::fs::symlink;
@@ -47,14 +48,65 @@ pub fn place_link(ctx: &Ctx, link_path: &Path, target: &Path) -> Result<()> {
         return Ok(());
     }
 
+    // the parent can be missing when the store holds a path the repository does
+    // not have any more, and symlink would only report the bare ENOENT
+    ensure_parent(ctx, link_path)?;
+
     // symlink under a temp name and rename over the path: the rename is atomic
     let tmp = suffixed(link_path, ".ahab-tmp");
-    let _ = fs::remove_file(&tmp);
+
+    // only ahab's own leftover is cleared away: anything else here belongs to
+    // the project, and silently unlinking it would lose it
+    match fs::symlink_metadata(&tmp) {
+        Ok(meta) if meta.is_symlink() => fs::remove_file(&tmp)?,
+        Ok(_) => {
+            return Err(anyhow!(
+                "{} is in the way and is not a symlink ahab left behind; \
+                 move it aside",
+                tmp.display()
+            ));
+        }
+        Err(e) if e.kind() == std::io::ErrorKind::NotFound => {}
+        Err(e) => return Err(e.into()),
+    }
 
     symlink(target, &tmp)?;
     Ok(fs::rename(&tmp, link_path)?)
 }
 
+// 0600 and only where nothing is yet. a dump is the whole database, and
+// pg_dumpall's is every role's password hash, so it is not the owner's to share
+// by default; create_new also refuses to follow a symlink planted at the path,
+// which File::create would open and truncate
+pub fn create_private_new(path: &Path) -> Result<std::fs::File> {
+    std::fs::OpenOptions::new()
+        .write(true)
+        .create_new(true)
+        .mode(0o600)
+        .open(path)
+        .with_context(|| format!("creating {}", path.display()))
+}
+
+// the store holds what should not be reachable from the repository, so the
+// directories it is kept in are the owner's alone. only the tree at or below
+// `base` is created here; what is above it is the user's own business
+pub fn ensure_private_parent(ctx: &Ctx, base: &Path, target: &Path) -> Result<()> {
+    if ctx.dry_run {
+        return Ok(());
+    }
+
+    let Some(parent) = target.parent().filter(|dir| dir.starts_with(base)) else {
+        return Ok(());
+    };
+
+    // a recursive create applies the mode to every directory it makes
+    std::fs::DirBuilder::new()
+        .recursive(true)
+        .mode(0o700)
+        .create(parent)
+        .with_context(|| format!("creating {}", parent.display()))
+}
+
 pub fn remove_file(ctx: &Ctx, path: &Path) -> Result<()> {
     if ctx.dry_run {
         return Ok(());

src/main.rs

@@ -9,10 +9,10 @@ mod output;
 mod project;
 
 use anyhow::Result;
-use clap::Parser;
+use clap::{CommandFactory, Parser};
 
 use crate::ctx::Ctx;
-use crate::output::note;
+use crate::output::{note, write_err};
 
 // 0 ran with nothing to report, 1 could not finish, 2 bad arguments, 4 found something
 const FINDINGS: u8 = 4;
@@ -32,15 +32,24 @@ fn main() -> ExitCode {
         note!(ctx, "dry run, nothing will be changed");
     }
 
-    match run(&ctx, args.command) {
+    // the command's own verdict first, then whether it managed to say it
+    match run(&ctx, args.command).and_then(|code| output::delivered().map(|()| code)) {
         Ok(code) => code,
         Err(e) => {
-            eprintln!("Error: {e:#}");
+            write_err(format_args!("Error: {e:#}"));
             ExitCode::FAILURE
         }
     }
 }
 
+// reported the way clap reports its own argument errors, which is what makes it
+// exit 2 rather than 1
+fn usage(message: &str) -> ! {
+    cli::Ahab::command()
+        .error(clap::error::ErrorKind::ArgumentConflict, message)
+        .exit()
+}
+
 fn run(ctx: &Ctx, command: cli::Commands) -> Result<ExitCode> {
     let done = ExitCode::SUCCESS;
 
@@ -65,6 +74,16 @@ fn run(ctx: &Ctx, command: cli::Commands) -> Result<ExitCode> {
                 cli::Postgres::Import { path } => commands::postgres::import(ctx, &path),
                 cli::Postgres::Psql { rest } => commands::postgres::psql(ctx, &rest),
                 cli::Postgres::Dump { path, format, gzip } => {
+                    // clap cannot say that a flag conflicts with one value of
+                    // another, and this is still an argument error: it belongs
+                    // with the usage and the exit code the others get
+                    if gzip && format == cli::Format::Directory {
+                        usage(
+                            "a directory dump is a directory of already \
+                             compressed files, not a stream",
+                        );
+                    }
+
                     commands::postgres::dump(ctx, &path, format, gzip)
                 }
             }?;

src/output.rs

@@ -1,18 +1,24 @@
 use std::fmt::Arguments;
 use std::io::{self, Write};
+use std::sync::OnceLock;
+use std::sync::atomic::{AtomicU8, Ordering};
+
+use anyhow::{Result, anyhow};
 
 // progress, on stderr and only when it was asked for
 macro_rules! note {
     ($ctx:expr, $($arg:tt)*) => {
         if !$ctx.quiet {
-            eprintln!($($arg)*)
+            $crate::output::write_err(format_args!($($arg)*))
         }
     };
 }
 
 // not progress: it speaks about the result, so --quiet keeps it
 macro_rules! warning {
-    ($($arg:tt)*) => { eprintln!("warning: {}", format_args!($($arg)*)) };
+    ($($arg:tt)*) => {
+        $crate::output::write_err(format_args!("warning: {}", format_args!($($arg)*)))
+    };
 }
 
 // what a command was asked for, on stdout
@@ -25,19 +31,79 @@ macro_rules! text {
     ($($arg:tt)*) => { $crate::output::write_text(format_args!($($arg)*)) };
 }
 
+// stdout is open, or it is not and why
+const OPEN: u8 = 0;
+// the reader left: nothing is wrong, there is just nowhere to write
+const CLOSED: u8 = 1;
+// the write itself failed, so what was asked for was never delivered
+const FAILED: u8 = 2;
+
+static STDOUT: AtomicU8 = AtomicU8::new(OPEN);
+static REASON: OnceLock<String> = OnceLock::new();
+
 pub(crate) fn write_line(args: Arguments) {
+    if writable() {
         finish(writeln!(io::stdout(), "{args}"));
     }
+}
 
 pub(crate) fn write_text(args: Arguments) {
+    if writable() {
         finish(write!(io::stdout(), "{args}"));
     }
-
-// a reader leaving early ends the pipe; println! would panic instead
-fn finish(written: io::Result<()>) {
-    if written.is_err() {
-        std::process::exit(0);
 }
+
+// a path on unix is bytes, not text, so a record naming one is written as bytes
+pub(crate) fn write_bytes(bytes: &[u8]) {
+    if writable() {
+        finish(io::stdout().write_all(bytes));
+    }
+}
+
+// stderr is commentary about the work, not the work itself. there is nowhere to
+// report that it could not be written, so the error goes nowhere -- eprintln!
+// panics instead, which turns a closed pipe into a crash
+pub(crate) fn write_err(args: Arguments) {
+    let _ = writeln!(io::stderr(), "{args}");
+}
+
+fn writable() -> bool {
+    STDOUT.load(Ordering::Relaxed) == OPEN
+}
+
+// a failed write must not end the process: a command halfway through moving
+// files would leave the rest undone and still report the success it had planned
+// on. writing stops, the command runs to its end, and main asks how it went
+fn finish(written: io::Result<()>) {
+    let Err(e) = written else {
+        return;
+    };
+
+    match e.kind() {
+        io::ErrorKind::BrokenPipe => STDOUT.store(CLOSED, Ordering::Relaxed),
+        _ => {
+            STDOUT.store(FAILED, Ordering::Relaxed);
+            let _ = REASON.set(e.to_string());
+        }
+    }
+}
+
+// whether everything a command was asked for actually reached stdout. the
+// buffered writes text! makes are flushed here rather than by the runtime after
+// main returns, which discards the error and truncates the output in silence
+pub(crate) fn delivered() -> Result<()> {
+    if writable() {
+        finish(io::stdout().flush());
+    }
+
+    if STDOUT.load(Ordering::Relaxed) != FAILED {
+        return Ok(());
+    }
+
+    Err(match REASON.get() {
+        Some(reason) => anyhow!("writing to stdout failed: {reason}"),
+        None => anyhow!("writing to stdout failed"),
+    })
 }
 
 pub(crate) use {line, note, text, warning};

src/project.rs

@@ -47,10 +47,23 @@ impl Project {
 
                 match serving.as_slice() {
                     [only] => Ok(only.to_string()),
+                    // publishing ports is the tie-break, so say which way it
+                    // failed: none of them serving is the opposite complaint
+                    // from all of them serving
+                    [] => Err(anyhow!(
+                        "cannot tell which service runs django, {} all build an image \
+                         and set {DJANGO_SETTINGS_MODULE}, and none publishes ports \
+                         to tell them apart",
+                        several.join(", ")
+                    )),
                     _ => Err(anyhow!(
                         "cannot tell which service runs django, {} all build an image, set \
                          {DJANGO_SETTINGS_MODULE} and publish ports",
-                        several.join(", ")
+                        serving
+                            .iter()
+                            .map(|name| **name)
+                            .collect::<Vec<&str>>()
+                            .join(", ")
                     )),
                 }
             }
@@ -82,13 +95,16 @@ impl Project {
         }
     }
 
-    pub fn postgres_credentials(&self, service: &str) -> (String, String) {
+    pub fn postgres_credentials(&self, service: &str) -> Result<(String, String)> {
-        let service = &self.services[service];
+        let entry = &self.services[service];
 
-        let user = env_var(service, "POSTGRES_USER").unwrap_or_else(|| "db".to_string());
+        let user = env_var(entry, "POSTGRES_USER").unwrap_or_else(|| "db".to_string());
-        let database = env_var(service, "POSTGRES_DB").unwrap_or_else(|| "db".to_string());
+        let database = env_var(entry, "POSTGRES_DB").unwrap_or_else(|| "db".to_string());
 
-        (user, database)
+        Ok((
+            usable_name(service, "POSTGRES_USER", user)?,
+            usable_name(service, "POSTGRES_DB", database)?,
+        ))
     }
 
     pub fn names(&self) -> Vec<&str> {
@@ -128,6 +144,35 @@ fn env_var(service: &Value, key: &str) -> Option<String> {
         .map(str::to_string)
 }
 
+// the compose file chooses these, and every postgres tool ahab runs takes them
+// as a role or database name. libpq reads a name holding `=` or a uri as a whole
+// connection string, so one could send a dump to another server entirely, and a
+// leading dash is read as an option however the argv is quoted
+fn usable_name(service: &str, key: &str, value: String) -> Result<String> {
+    let wrong = if value.is_empty() {
+        Some("is empty")
+    } else if value.starts_with('-') {
+        Some("starts with a dash, which the postgres tools read as an option")
+    } else if value.contains('=') {
+        Some("holds an `=`, which libpq reads as a connection string")
+    } else if value.contains("://") {
+        Some("holds a url, which libpq reads as a connection string")
+    } else if value.chars().any(|c| c.is_whitespace() || c.is_control()) {
+        Some("holds whitespace")
+    } else {
+        None
+    };
+
+    match wrong {
+        None => Ok(value),
+        Some(wrong) => Err(anyhow!(
+            "{key} of service {service} {wrong}: {value:?}. \
+             ahab passes it to psql, pg_dump and dropdb as a name, \
+             so it has to be one"
+        )),
+    }
+}
+
 fn is_postgres_image(image: &str) -> bool {
     let image = image.to_lowercase();
     POSTGRES_IMAGES.iter().any(|kind| image.contains(kind))
@@ -135,13 +180,43 @@ fn is_postgres_image(image: &str) -> bool {
 
 #[cfg(test)]
 mod tests {
-    use super::{Project, is_postgres_image};
+    use super::{Project, is_postgres_image, usable_name};
     use serde_json::json;
 
     fn compose(services: serde_json::Value) -> Project {
         Project { services }
     }
 
+    #[test]
+    fn a_name_the_postgres_tools_would_read_as_something_else_is_refused() {
+        // libpq takes a dbname holding `=` or a url as a whole conninfo string,
+        // so a compose file could otherwise pick the server a dump goes to
+        let refused = [
+            "postgresql://myproject:pw@evil.example.net:5432/loot",
+            "host=evil.example.net dbname=loot",
+            "--file=/var/lib/postgresql/data/pg_hba.conf",
+            "-h evil.example.net",
+            "my db",
+            "",
+        ];
+
+        for value in refused {
+            assert!(
+                usable_name("db", "POSTGRES_DB", value.to_string()).is_err(),
+                "should have been refused: {value:?}"
+            );
+        }
+
+        // and the names real projects actually use are left alone
+        for value in ["myproject_db", "my-db", "db", "app.prod", "DB_2"] {
+            assert_eq!(
+                usable_name("db", "POSTGRES_DB", value.to_string()).unwrap(),
+                value,
+                "should have been accepted: {value:?}"
+            );
+        }
+    }
+
     #[test]
     fn recognises_postgres_flavours() {
         for image in [
@@ -242,7 +317,7 @@ mod tests {
             },
         }));
 
-        let (user, database) = compose.postgres_credentials("db");
+        let (user, database) = compose.postgres_credentials("db").unwrap();
         assert_eq!(
             (user.as_str(), database.as_str()),
             ("myproject", "myproject_db")
@@ -253,7 +328,7 @@ mod tests {
     fn falls_back_to_db_when_the_service_says_nothing() {
         let compose = compose(json!({"db": {"image": "postgres:18"}}));
 
-        let (user, database) = compose.postgres_credentials("db");
+        let (user, database) = compose.postgres_credentials("db").unwrap();
         assert_eq!((user.as_str(), database.as_str()), ("db", "db"));
     }
 }