bastille-jail-provisioning: add FreeBSD-adapted script safety patterns

Merge in a checklist-style "provisioning script safety" section, adapted
from wshobson/agents' bash-defensive-patterns skill (fetched directly
from GitHub, MIT licensed) rather than trusted from its catalog summary.

Covers: shell-choice caution (#!/bin/bash isn't guaranteed on FreeBSD;
bash is a package at /usr/local/bin/bash, not base), a contextual
error-handling philosophy instead of a blanket `set -Eeuo pipefail`,
trap-based cleanup/logging for scripts interrupted mid-jail-creation
(without auto-rollback, which can worsen the known destroy/create IP
churn issue), jail-name/IP input validation before destructive `bastille`
commands, and FreeBSD `mktemp`/`sed -i`/`date` syntax differences from
GNU. Deliberately dropped the upstream's interactive `rm -rI` cleanup
pattern (wrong for unattended SSH automation) and did not import
retry/locking/ShellCheck-gate advice that wasn't actually present in the
real upstream files. Cross-references remote-shell-quoting-safety instead
of duplicating its nested-shell-quoting content.

Also documents the /usr/local/bastille vs /www/bastille bastille_prefix
split already present across this fleet's hosts (gringo/staging vs
granja), which the new script-safety guidance assumes readers already
know not to hardcode.

Full worked patterns and code examples live in the new
references/script-safety.md; SKILL.md keeps the load-bearing summary.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

skills/bastille-jail-provisioning/SKILL.md

@@ -265,3 +265,63 @@ exist under that name in the FreeBSD ports tree) — and remember Valkey's
 ACL means any code connecting to it needs an explicit `AUTH` step, not just
 `connect()`; a `connect()`-only implementation will silently fail every
 subsequent command with `NOAUTH` against this fleet's standard Valkey setup.
+
+## Bastille filesystem prefix varies by host — check before hardcoding a path
+
+Don't assume `/usr/local/bastille/jails/<name>/root/...` — that's only
+the convention where `bastille.conf` uses the default `bastille_prefix`
+(confirmed on gringo and staging). A host with a custom
+`bastille_prefix` (e.g. `granja`, where storage was repointed at a
+dedicated `wwwpool` with `bastille_prefix=/www/bastille`) puts jails
+under `/www/bastille/jails/<name>/root/...` instead — writing to the
+`/usr/local/bastille/...` path on that host silently succeeds (creates a
+new, wrong directory) rather than erroring, since nothing stops you from
+writing to an arbitrary path. **Check the host's actual
+`bastille_prefix`** (`grep bastille_prefix /etc/rc.conf` or
+`bastille.conf`) before hardcoding either path in a script.
+
+## Provisioning script safety
+
+Applies to any shell script written to automate part of this
+provisioning flow (not the one-off interactive commands above). Full
+worked patterns, including FreeBSD-specific `mktemp`/`sed`/`date` syntax,
+a trap-based cleanup template, and jail-name/IP input validation, live in
+`references/script-safety.md` — read that before writing a new
+provisioning script. Summary of the load-bearing points:
+
+- **Don't assume `#!/bin/bash` exists.** FreeBSD base ships `/bin/sh`
+  (POSIX-ish ash), not bash — bash is a package that installs to
+  `/usr/local/bin/bash`, never `/bin/bash`, and many freshly cloned jails
+  won't have it installed at all. Default to `#!/bin/sh` and POSIX
+  constructs unless bash-only features are genuinely needed and you've
+  confirmed bash is present on the target.
+- **Error handling is contextual, not a blanket `set -Eeuo pipefail`.**
+  Turn on `set -eu` for the whole script, but explicitly tolerate steps
+  that are allowed to fail (e.g. an already-installed package, an
+  optional service restart) with a commented `|| true` rather than either
+  aborting the whole provisioning run on a non-fatal hiccup or suppressing
+  errors wholesale. `-o pipefail`/`-E` are bash-only and unavailable under
+  `/bin/sh`.
+- **Trap cleanup, don't trap auto-rollback.** A script that dies partway
+  through jail creation should log exactly which step it reached (jail
+  created? DB granted? vhost written?) so a human can clean up — see the
+  create/destroy IP-alias churn gotcha above for why an automatic
+  `bastille destroy` inside a failure-path trap can make things worse, not
+  better.
+- **Validate jail names and IPs before they reach a destructive `bastille`
+  command.** `bastille destroy <name>` has no confirmation prompt.
+  Validate with plain POSIX `case`/`[` pattern matching (works under both
+  `/bin/sh` and bash) before any value — especially one derived from
+  outside the script — is interpolated into a command.
+
+For quoting content through nested shell layers (`ssh` → `bastille cmd`
+→ `su`), use the separate `remote-shell-quoting-safety` skill — that
+problem is not duplicated here.
+
+Provenance: the script-safety patterns above are adapted from
+wshobson/agents' `bash-defensive-patterns` skill
+(https://github.com/wshobson/agents,
+`plugins/shell-scripting/skills/bash-defensive-patterns`, MIT licensed,
+fetched 2026-08-15) — rewritten for FreeBSD/Bastille where upstream
+assumed GNU/Linux; see `references/script-safety.md` for exactly what was
+kept, rewritten, or dropped.

skills/bastille-jail-provisioning/references/script-safety.md

@@ -0,0 +1,241 @@
+# Provisioning script safety — FreeBSD/Bastille worked patterns
+
+Deeper worked examples for the "Provisioning script safety" section of
+`SKILL.md`. Read this when the summary there isn't enough to actually
+write the script.
+
+**Provenance**: adapted from wshobson/agents' `bash-defensive-patterns`
+skill (https://github.com/wshobson/agents,
+`plugins/shell-scripting/skills/bash-defensive-patterns/{SKILL.md,references/details.md}`,
+MIT licensed, fetched 2026-08-15). That upstream skill is written for
+Linux/GNU and knows nothing about FreeBSD base tools, `/usr/local/bin/bash`,
+Bastille, rc.d, ZFS, or PF. Every pattern below was checked against FreeBSD
+15.1-RELEASE behavior and either kept, rewritten, or explicitly dropped —
+see "What was deliberately left out" at the end. Nothing here was copied
+from the upstream examples unchanged.
+
+For shell-quoting-through-multiple-hops (`ssh` → `bastille cmd` → `su`),
+see the `remote-shell-quoting-safety` skill instead — not duplicated here.
+
+## Shell choice: don't assume `#!/bin/bash` exists
+
+FreeBSD base's `/bin/sh` is a modified `ash` (POSIX-ish), not bash. Bash
+is **not** part of FreeBSD base — it's a package (`pkg install bash`)
+that installs to `/usr/local/bin/bash`, **never** `/bin/bash`. A
+`#!/bin/bash` shebang written for a Linux-oriented example fails
+immediately with "No such file or directory" on a bare FreeBSD host or a
+freshly cloned jail — many base jail images don't have bash installed at
+all.
+
+- If bash-only features are genuinely needed (arrays, `[[ ]]`, `mapfile`,
+  process substitution `< <(...)`), use `#!/usr/bin/env bash` **and**
+  verify bash is actually present on the target first —
+  `bastille cmd <jail> which bash` or `pkg info bash` on the host. Don't
+  assume.
+- For anything that runs early in provisioning (before you know what's on
+  the target) or where plain POSIX does the job, prefer `#!/bin/sh` — it's
+  guaranteed present on both the host and every jail, no package
+  dependency. Drop arrays, `[[ ]]`, `mapfile`/`readarray`, and process
+  substitution; use `case`, `$( )`, and POSIX `[ ]` test instead.
+
+## Error-handling philosophy: contextual, not a blanket `set -Eeuo pipefail`
+
+The upstream checklist's rule #1 is "always use strict mode." Treating
+that as an absolute is wrong for provisioning scripts:
+
+- `set -e` (and `set -u`) are genuinely valuable for early validation —
+  bad jail name, IP already taken, missing base release — where any
+  failure should hard-stop before anything destructive happens.
+- Once a script is deep into a multi-step provisioning sequence, some
+  steps are legitimately allowed to fail without aborting the whole run —
+  e.g. `pkg install -y arp-scan` when it may already be installed, or
+  restarting an optional service that doesn't exist on every base image
+  yet. Blanket `-e` turns those into full-script aborts, which is exactly
+  how you get the half-created-jail mess this skill warns about elsewhere.
+- Practical rule for this fleet: turn strict mode on for the whole script
+  (`set -eu`; add `-o pipefail` only if it's genuinely running under
+  bash), but explicitly wrap any step that's allowed to fail —
+  `cmd || true` or `if ! cmd; then log_warn "..."; fi` — with a comment
+  saying *why* it's non-fatal. Silent blanket suppression (`set +e`
+  everywhere) is worse than blanket strict mode; the goal is a script
+  where every deliberately-tolerated failure is visible in a diff, not one
+  with no error handling at all.
+- `-o pipefail` and `-E` (ERR trap inherited into functions) are bash
+  extensions, unavailable under POSIX `/bin/sh`. If the script is
+  `/bin/sh`, restructure instead of relying on them — e.g. write a
+  pipeline's output to a temp file and check that command's own exit
+  status, rather than depending on an upstream stage's exit code surviving
+  a pipe.
+
+## Trap-based cleanup for interrupted provisioning
+
+A half-created jail is a real mess: `bastille create` partway through, a
+DB created but no grant, an nginx vhost written but not enabled, a pf
+table entry added but the jail never started. A script that dies partway
+(Ctrl-C, a dropped SSH session, a later step's genuine failure) should at
+minimum log exactly how far it got — automatically rolling back a jail
+creation from inside a trap is its own risk (see this skill's
+create/destroy-churn IP-alias gotcha; automating teardown under failure
+conditions can make the mess worse, not better).
+
+```sh
+# POSIX-compatible: /bin/sh, no bash-only trap features needed
+PROVISION_LOG="/tmp/provision-${JAIL_NAME}.log"
+CREATED_JAIL=""
+CREATED_DB=""
+CURRENT_STEP=""
+
+cleanup() {
+    status=$?
+    if [ "$status" -ne 0 ]; then
+        {
+            echo "FAILED at step: ${CURRENT_STEP:-unknown} (exit $status)"
+            echo "Partial state: jail=${CREATED_JAIL:-none} db=${CREATED_DB:-none}"
+            echo "Manual cleanup needed before retrying with the same name/IP."
+        } >> "$PROVISION_LOG"
+        echo "Provisioning failed — see $PROVISION_LOG" >&2
+    fi
+}
+trap cleanup EXIT INT TERM
+
+CURRENT_STEP="bastille create"
+bastille create "$JAIL_NAME" 15.1-RELEASE "$JAIL_IP" vtnet0
+CREATED_JAIL="$JAIL_NAME"
+
+CURRENT_STEP="db grant"
+# ... etc, set CURRENT_STEP before each major step
+```
+
+Set `CURRENT_STEP` before each major step so the log names the actual
+failed step, not just "something failed."
+
+## Input validation before jail names/IPs reach a destructive command
+
+Bastille commands take a jail name and/or IP as a bare positional
+argument with no confirmation prompt — `bastille destroy <name>` is
+irreversible. Validate **before** the value is interpolated into any
+`bastille` command, especially if it ever originates from outside the
+script (a ticket description, a generated name, pasted text):
+
+```sh
+validate_jail_name() {
+    name="$1"
+    case "$name" in
+        ''|*[!a-zA-Z0-9_-]*)
+            echo "ERROR: invalid jail name: '$name' (letters, digits, - and _ only)" >&2
+            return 1
+            ;;
+    esac
+    if bastille list all 2>/dev/null | grep -qx "$name"; then
+        echo "NOTE: jail '$name' already exists" >&2
+    fi
+}
+
+validate_ipv4() {
+    ip="$1"
+    case "$ip" in
+        *[!0-9.]*|'') echo "ERROR: not an IPv4 literal: '$ip'" >&2; return 1 ;;
+    esac
+    IFS=. read -r o1 o2 o3 o4 <<EOF
+$ip
+EOF
+    for o in "$o1" "$o2" "$o3" "$o4"; do
+        [ -n "$o" ] && [ "$o" -le 255 ] 2>/dev/null || {
+            echo "ERROR: octet out of range in '$ip'" >&2
+            return 1
+        }
+    done
+}
+```
+
+This is intentionally plain `case`/`[` pattern matching, not a regex
+call — it works identically under `/bin/sh` and bash, and avoids needing
+`[[ =~ ]]` (bash-only) or GNU `grep -P`. Run `arp-scan` (see the main
+SKILL.md's IP-conflict section) only **after** `validate_ipv4` passes, not
+before.
+
+## Safe temporary files: FreeBSD `mktemp` syntax
+
+FreeBSD's base `mktemp` is BSD mktemp, not GNU coreutils mktemp —
+long options like `--tmpdir=DIR` or `--suffix=` don't exist there.
+
+```sh
+# Portable on FreeBSD and Linux: bare mktemp / mktemp -d, no custom template
+TMPFILE=$(mktemp) || { echo "ERROR: mktemp failed" >&2; exit 1; }
+TMPDIR=$(mktemp -d) || { echo "ERROR: mktemp -d failed" >&2; exit 1; }
+
+# A custom prefix/path needs an explicit trailing run of X's on BOTH
+# platforms — a prefix alone (no X's) is not enough on either:
+TMPFILE=$(mktemp "/tmp/provision-${JAIL_NAME}.XXXXXX") || exit 1
+```
+
+Clean up with a trap (see above), and always quote and use `--`:
+`rm -rf -- "$TMPDIR"` — the `--` guards against a temp path that could
+otherwise be parsed as an option if it somehow started with `-`.
+
+## `sed -i` and `date`: don't copy GNU-only invocations into jail.conf/pf.conf edits
+
+Two gotchas that bite specifically because provisioning steps edit
+`jail.conf` and `pf.conf` in place:
+
+- **`sed -i`**: FreeBSD/BSD sed requires an explicit (possibly empty)
+  backup-suffix argument as its own token:
+  `sed -i '' -e 's/old/new/' file`. GNU sed's `-i` only takes the suffix
+  glued to the flag (`-i.bak` or bare `-i`) — passing `-i ''` as two
+  separate arguments to GNU sed does **not** mean "no backup"; it's
+  parsed differently and can silently do the wrong thing. These two forms
+  are not cross-compatible. Use the BSD form on this fleet's FreeBSD
+  hosts, don't copy a Linux-written example verbatim.
+- **`date`**: FreeBSD date has no GNU-style free-text parsing
+  (`date -d "1 day ago"` doesn't work). Use `-v` adjustments
+  (`date -v-1d +%F`) or `-j -f <informat> <string> +<outformat>` to parse
+  a fixed format. A script that only timestamps a log line
+  (`date +'%Y-%m-%d %H:%M:%S'`, fixed format, no relative math) is fine
+  as-is on both platforms — the trap is only when a script tries to
+  compute a relative date.
+
+## Structured logging (portable as written)
+
+The upstream timestamped-log-level pattern is fine to reuse as-is —
+fixed-format `date`, no relative parsing, identical under FreeBSD and GNU
+date:
+
+```sh
+log_info()  { echo "[$(date +'%Y-%m-%d %H:%M:%S')] INFO: $*" >&2; }
+log_warn()  { echo "[$(date +'%Y-%m-%d %H:%M:%S')] WARN: $*" >&2; }
+log_error() { echo "[$(date +'%Y-%m-%d %H:%M:%S')] ERROR: $*" >&2; }
+```
+
+## What was deliberately left out
+
+- **Arrays, `mapfile`/`readarray`, `[[ ]]`, process substitution
+  (`< <(...)`)** — all bash-only, and bash isn't guaranteed present on a
+  freshly cloned jail. Only reach for them once you've confirmed the
+  target will always run under a real installed bash; default to POSIX
+  `/bin/sh` constructs for anything that has to run early or on an
+  unknown target.
+- **The upstream `rm -rI` "safe cleanup" pattern** — `-I` prompts for
+  interactive confirmation, which is the opposite of what an unattended
+  provisioning script run over SSH (often with no TTY) needs; a script
+  waiting on a prompt that can never be answered just hangs. Validate
+  inputs hard (see above) instead, then remove non-interactively
+  (`rm -rf --`).
+- **Retry/backoff and file-locking patterns** from the general upstream
+  checklist — not reproduced here because the fetched upstream content
+  (`SKILL.md` + `references/details.md`, both fetched directly from
+  GitHub) doesn't actually contain a worked retry or locking pattern
+  specific enough to adapt; those topics only appear as chapter-title-level
+  mentions in secondhand catalog summaries, not in the real file. If a
+  real retry-needing case shows up in provisioning (a flaky `pkg install`
+  in a fresh jail before DNS is confirmed working — see this skill's
+  resolver-failure-after-destroy/create-churn gotcha), write a
+  fleet-specific pattern then, rather than importing a generic one now on
+  spec.
+- **ShellCheck as a hard gate** — worth running locally before landing a
+  new provisioning script (`pkg install shellcheck`; point it at a
+  `/bin/sh` script with `# shellcheck shell=sh` since its default dialect
+  guess is bash), but not made mandatory here since this fleet has no CI
+  wired to enforce it on these scripts yet.
+- **The upstream "always use `[[ ]]`" advice as a blanket rule** — true
+  only once bash is confirmed present; POSIX `[ ]` is required, not just
+  "more portable," for any `/bin/sh` script.