diff --git a/ansible/roles/caddy/templates/Caddyfile.j2 b/ansible/roles/caddy/templates/Caddyfile.j2 index f465e3e..51f4b0f 100644 --- a/ansible/roles/caddy/templates/Caddyfile.j2 +++ b/ansible/roles/caddy/templates/Caddyfile.j2 @@ -46,6 +46,12 @@ root * /var/www/html file_server } +{% elif route.handler is defined and route.handler == "redirect" %} +{% if route.extra is defined %} +{% for line in route.extra.split('\n') %} + {{ line }} +{% endfor %} +{% endif %} {% elif route.extra is defined %} {% for line in route.extra.split('\n') %} {{ line }} diff --git a/bin/check-worktree.sh b/bin/check-worktree.sh index cc1b1f4..418345c 100755 --- a/bin/check-worktree.sh +++ b/bin/check-worktree.sh @@ -11,15 +11,12 @@ # # Exit codes: # 0 in a valid worktree (non-main branch in a worktree dir) -# 1 on main branch in the primary checkout (blocked) +# 1 on main branch in the primary checkout (blocked), or not in a git repository # 2 on main branch in a worktree (allowed — worktree branches can be named "main" in theory, but we still warn) set -euo pipefail -SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" -REPO_ROOT="$(cd "$SCRIPT_DIR/.." && pwd)" - -RED='\033[0;31m'; YELLOW='\033[1;33m'; NC='\033[0m' +RED='\033[0;31m'; YELLOW='\033[1;33m'; GREEN='\033[0;32m'; NC='\033[0m' ALLOW_MAIN=0 while [ $# -gt 0 ]; do @@ -39,28 +36,28 @@ while [ $# -gt 0 ]; do shift done -# Determine if we are in the primary checkout -WORKTREE_LIST="$(git worktree list --porcelain 2>/dev/null || true)" -PRIMARY_PATH="$(cd "$REPO_ROOT" && pwd)" -CURRENT_PATH="$(pwd)" - -# Check if current directory IS the primary checkout -IS_PRIMARY=0 -for line in $WORKTREE_LIST; do - case "$line" in - worktree*) - wt_path="${line#worktree }" - if [ "$(cd "$wt_path" 2>/dev/null && pwd)" = "$PRIMARY_PATH" ]; then - IS_PRIMARY=1 - break - fi - ;; - esac -done +# Determine whether we are in the primary checkout. +# +# Do NOT infer this from paths. The primary checkout is the one directory whose +# --git-dir IS the common repository; in a linked worktree --git-dir points at +# /worktrees/, so the two differ. The previous version derived a +# "primary path" from the script's own location and compared it against each +# entry of `git worktree list`, which always matched the worktree the user was +# standing in — so every worktree commit claimed to be the primary checkout +# (#450). +if ! GIT_DIR_PATH="$(git rev-parse --absolute-git-dir 2>/dev/null)"; then + echo -e "${RED}ERROR: not inside a git repository.${NC}" >&2 + exit 1 +fi +GIT_COMMON_DIR_PATH="$(cd "$(git rev-parse --git-common-dir 2>/dev/null)" && pwd)" || { + echo -e "${RED}ERROR: cannot resolve the common git directory.${NC}" >&2 + exit 1 +} -if [ $IS_PRIMARY -eq 0 ]; then - # Also check by path comparison (worktree list can be unreliable with symlinks) - [ "$CURRENT_PATH" = "$PRIMARY_PATH" ] && IS_PRIMARY=1 +if [ "$GIT_DIR_PATH" = "$GIT_COMMON_DIR_PATH" ]; then + IS_PRIMARY=1 +else + IS_PRIMARY=0 fi if [ $IS_PRIMARY -eq 1 ]; then diff --git a/docs/kb/README.md b/docs/kb/README.md index 7c10771..8042fe5 100644 --- a/docs/kb/README.md +++ b/docs/kb/README.md @@ -18,3 +18,10 @@ Knowledge Base template in `.opencode/skills/templates/SKILL.md`. Use the - [`opencode-desktop-window-never-opens.md`](opencode-desktop-window-never-opens.md) — OpenCode desktop is "running" but no window ever appears; a wedged Electron child-process bootstrap plus single-instance lock; kill to recover. +- [`frappe-500-after-restart-db-grant.md`](frappe-500-after-restart-db-grant.md) — + a Frappe site 500s after a container restart because its DB user was granted for + one container IP instead of `%`; diagnose from the backend log, fix the grant. +- [`bitwarden-cli-serves-stale-cache.md`](bitwarden-cli-serves-stale-cache.md) — + `bw list` answers from its local cache even while locked, so "not in the vault" + can be false; `lastSync` is the only freshness signal, and a TTY-less prompt + reads as an empty result. diff --git a/docs/kb/bitwarden-cli-serves-stale-cache.md b/docs/kb/bitwarden-cli-serves-stale-cache.md new file mode 100644 index 0000000..fdc23d8 --- /dev/null +++ b/docs/kb/bitwarden-cli-serves-stale-cache.md @@ -0,0 +1,89 @@ +# `bw` answers from a stale cache while locked — "not in the vault" can be false + +## Symptom + +A search for a credential returns items, so the vault looks readable and current. +Then the answer drawn from it is wrong: the item you were told does not exist turns +out to exist, or every copy of a password tests as rejected while a working copy +was written days ago. + +The signal that was visible but easy to dismiss: `bw status` reported +`"status": "locked"` and a `lastSync` timestamp days older than the change being +investigated — while `bw list items` kept answering normally. + +## Diagnosis + +Ask the vault how fresh its answer is, not just whether it answers: + +``` +bw status | jq -r '.lastSync, .status' +``` + +Compare `lastSync` against the age of the change you are chasing. If the cache is +older than the change, you have no data — not a negative result. + +Force a refresh and watch what happens to reads: + +``` +bw sync # can print "Syncing complete." while the vault stays locked +bw list items # then prompts "? Master password:" — with no TTY this yields + # empty output, which a caller easily reads as "no items" +``` + +Use the repo's own gate, because it validates with a real API call rather than +trusting a status field: + +``` +bin/bwunlock.sh --check # unlocked | locked | missing | permission +``` + +## Cause + +The CLI serves `bw list` and `bw get` from its local encrypted cache +(`~/.config/Bitwarden CLI/data.json`). A locked session does not stop it from +answering — it only stops fresh decryption. Whatever the cache holds is what you +get, and its age is exactly `lastSync`. + +`bw status` reports the lock state of the local vault, which is a different +question from "is the data I just read current". Both can be true at once, so +"locked" looks like cosmetic noise right up until it isn't. + +## Fix + +Refresh the session, then sync, then re-query: + +``` +bin/bwunlock.sh # password popup; writes the cache atomically +bw sync +bw list items --search +``` + +Headless equivalent: + +``` +(umask 077; bw unlock --raw > ~/.cache/opskit/bw-session) +``` + +## Prevention + +- Treat `lastSync` older than the change under investigation as "no data". Report + "I cannot see it in a cache this old", never "it is not in the vault". +- Gate vault reads through `bin/bwunlock.sh --check`, not through `bw status`. +- When verifying a rotated credential, prefer **one** real authentication attempt + over item metadata. A revision date records intent; only the service records + whether the password works. (Check for lockout policy first — some services + rate-limit or fail-lock on bad attempts.) +- An empty result from a command run without a TTY may be a prompt, not an + absence. `bw` prints `? Master password:` to stderr and nothing to stdout. + +## Notes + +- Stale duplicates are the usual aftermath of a rotation: a new item is written, + the old copies keep the dead password, and a reader that resolves by *name* + can land on a stale one. The resulting `401` looks like a broken service. +- Resolve by item **id**, and confirm the id against `revisionDate` before acting. +- `bw delete item ` moves the item to the trash and is recoverable with + `bw restore item `; verify that rather than assuming deletion is permanent. +- The general lesson: a cache that answers confidently is more dangerous than one + that errors, because the failure surfaces as a confident wrong conclusion far + from its cause. diff --git a/docs/kb/frappe-500-after-restart-db-grant.md b/docs/kb/frappe-500-after-restart-db-grant.md new file mode 100644 index 0000000..c1b7c93 --- /dev/null +++ b/docs/kb/frappe-500-after-restart-db-grant.md @@ -0,0 +1,79 @@ +# Frappe site 500s after a container restart — DB grant pinned to a container IP + +## Symptom + +A Frappe/ERPNext site returns a bare `500 Internal Server Error` on its public +hostname after the Docker stack was stopped and started. Other hostnames served by +the same stack (for example the desk login) still redirect normally, so TLS and the +reverse proxy look healthy and the problem appears to be inside the application. + +## Diagnosis + +Read the backend log before touching the database: + +``` +docker logs --tail 25 -backend-1 +``` + +The tell is: + +``` +MySQLdb.OperationalError: (1045, "Access denied for user 'USER'@'192.0.2.7' +(using password: YES)") +``` + +That message is ambiguous — it is returned both for a wrong password and for a +user/host pair that does not exist. Compare the grant's host to the address the +backend actually connects from: + +``` +docker exec -db-1 sh -c \ + 'mariadb -uroot -p"$MYSQL_ROOT_PASSWORD" -e "select user,host,plugin from mysql.user"' +``` + +If the user exists **only** for an address like `192.0.2.9` while the backend now +connects from `192.0.2.7`, the grant is pinned to a container IP. Docker reassigns +addresses when containers are recreated, so the site breaks on the first restart +after the grant was created. + +Confirm the data is intact before changing anything (so the fix is not mistaken for +a restore): check the DB bind mount/volume size, the site database's table count, +and a row count from a core table. + +## Cause + +The MariaDB user was created for a specific container address instead of `%`. The +password is fine; the *host* match is what fails. + +## Fix + +Grant the same credential for `%`, using the password the application already +holds (`site_config.json` → `db_password`, which Frappe reads): + +```sql +CREATE USER IF NOT EXISTS 'USER'@'%' IDENTIFIED BY ''; +ALTER USER 'USER'@'%' IDENTIFIED BY ''; +GRANT ALL PRIVILEGES ON `DBNAME`.* TO 'USER'@'%'; +FLUSH PRIVILEGES; +``` + +Generate that SQL inside the backend container so the secret never crosses the +shell or a chat transcript, copy it into the DB container, apply as root, then +delete the file from both containers and the host. Verify with a real connection as +the site user, not with `mysql -e "select 1"` as root. + +## Prevention + +- Create DB users for `%` unless a static, guaranteed address exists. In a Docker + compose network, "the container's IP" is not a stable identity. +- Before any planned stop/start of a Frappe stack, confirm every site user has a + `%` grant. +- A restart that "broke the site on its own" is nearly always this, or a volume + that was recreated — check which, because only one of the two needs a restore. + +## Notes + +- `docker exec` needs `-i` to accept a heredoc on stdin; without it the script gets + EOF and silently produces nothing. +- Root password env var is image-specific: `MYSQL_ROOT_PASSWORD` on `mariadb:11` + images, not `MARIADB_ROOT_PASSWORD`. diff --git a/tests/test_check_worktree.py b/tests/test_check_worktree.py new file mode 100644 index 0000000..73be13a --- /dev/null +++ b/tests/test_check_worktree.py @@ -0,0 +1,148 @@ +"""Tests for bin/check-worktree.sh — primary-checkout detection (issue #450). + +The script guards the repo rule that the primary checkout stays on `main` and all +development happens in a linked worktree. Its detection used to derive a "primary +path" from the script's own location and compare it against each entry of +`git worktree list`, which always matched the worktree the caller was standing +in: every worktree commit printed "in the primary checkout", and the one case +worth catching (primary checkout left on a feature branch) became +indistinguishable from routine noise. + +These tests build real repositories — the defect is in how git itself is +queried, so a stubbed git would reproduce the bug's assumptions instead of +catching them. +""" + +import os +import subprocess +from pathlib import Path + +ROOT = Path(__file__).resolve().parents[1] +SCRIPT = ROOT / "bin" / "check-worktree.sh" + +GIT = ["git", "-c", "user.name=Test", "-c", "user.email=test@example.invalid"] + + +def git(*args: str, cwd: Path) -> str: + """Run git with identity pinned, so no ambient config can change results.""" + env = dict(os.environ) + env["GIT_CONFIG_NOSYSTEM"] = "1" + env["GIT_TERMINAL_PROMPT"] = "0" + proc = subprocess.run( + [*GIT, *args], cwd=cwd, capture_output=True, text=True, env=env + ) + assert proc.returncode == 0, f"git {' '.join(args)} failed: {proc.stderr}" + return proc.stdout + + +def make_repo(tmp_path: Path, branch: str = "main") -> Path: + repo = tmp_path / "primary" + repo.mkdir() + git("init", f"--initial-branch={branch}", cwd=repo) + git("commit", "--allow-empty", "-m", "initial", cwd=repo) + return repo + + +def add_worktree(repo: Path, path: str, branch: str) -> Path: + wt = repo.parent / path + git("worktree", "add", "-b", branch, str(wt), cwd=repo) + return wt + + +def run(cwd: Path, *flags: str) -> subprocess.CompletedProcess: + return subprocess.run( + ["bash", str(SCRIPT), *flags], cwd=cwd, capture_output=True, text=True + ) + + +def test_worktree_feature_branch_is_not_the_primary_checkout(tmp_path: Path): + """The #450 regression: a worktree commit must not claim to be primary.""" + repo = make_repo(tmp_path) + wt = add_worktree(repo, "wt-feature", "some-feature") + + result = run(wt) + + assert result.returncode == 0 + assert "primary checkout" not in result.stdout + assert "In worktree" in result.stdout + assert "some-feature" in result.stdout + + +def test_worktree_with_space_in_path(tmp_path: Path): + """Path comparison used to word-split; git's own resolution must not care.""" + repo = make_repo(tmp_path) + wt = add_worktree(repo, "wt with space", "spacey") + + result = run(wt) + + assert result.returncode == 0 + assert "primary checkout" not in result.stdout + assert "In worktree" in result.stdout + + +def test_worktree_from_subdirectory(tmp_path: Path): + """Detection must hold from anywhere inside the worktree, not just its root.""" + repo = make_repo(tmp_path) + wt = add_worktree(repo, "wt-sub", "sub-feature") + nested = wt / "deep" / "dir" + nested.mkdir(parents=True) + + result = run(nested) + + assert result.returncode == 0 + assert "primary checkout" not in result.stdout + assert "In worktree" in result.stdout + + +def test_primary_checkout_on_main_is_blocked(tmp_path: Path): + repo = make_repo(tmp_path) + + result = run(repo) + + assert result.returncode == 1 + assert "Refusing to commit on main in the primary checkout" in result.stdout + + +def test_primary_checkout_on_main_allows_explicit_override(tmp_path: Path): + repo = make_repo(tmp_path) + + result = run(repo, "--allow-main") + + assert result.returncode == 0 + assert "Override acknowledged" in result.stdout + + +def test_primary_checkout_on_feature_branch_warns(tmp_path: Path): + """The genuine condition the script exists to flag must still fire.""" + repo = make_repo(tmp_path) + git("checkout", "-b", "stray-feature", cwd=repo) + + result = run(repo) + + assert result.returncode == 0 + assert "in the primary checkout" in result.stdout + assert "stray-feature" in result.stdout + + +def test_worktree_on_main_warns_about_the_worktree(tmp_path: Path): + """A worktree on `main` is a distinct case and must not be called primary.""" + # Git refuses to check out the same branch twice, so the primary sits on a + # different branch and the worktree takes `main`. + repo = make_repo(tmp_path, branch="develop") + wt = add_worktree(repo, "wt-main", "main") + + result = run(wt) + + assert result.returncode == 0 + assert "primary checkout" not in result.stdout + assert "is on branch 'main'" in result.stdout + + +def test_outside_a_git_repository(tmp_path: Path): + outside = tmp_path / "nowhere" + outside.mkdir() + + result = run(outside) + + assert result.returncode == 1 + assert "not inside a git repository" in result.stdout + result.stderr