root/Jyotisha
2
0
Fork 0
Code Issues Pull Requests 1 Actions 1 Packages Projects Releases Wiki Activity

fix: pin staging backup directory

Browse Source
This commit is contained in:
Jesse_Chen
2026-07-20 23:27:18 +08:00
parent 2bf5594730
commit 130904a54d
3 changed files with 105 additions and 15 deletions
Download Patch File Download Diff File Expand all files Collapse all files
.superpowers/sdd/task-3-report.md
+20 -1
View File
@@ -5,7 +5,7 @@
- Added `deploy/backup-staging-postgres.sh DATABASE_ENV_FILE BACKUP_DIRECTORY`.
- Validates the private database environment before reading required values without sourcing or executing the env file.
- Refuses backup destinations with disk usage at or above 70%, creates the explicit destination with mode `0700`, writes encrypted custom PostgreSQL dumps with mode `0600`, and keeps only the newest three exact staging dump names.
- Uses a PID-scoped `.partial` file, `pipefail`, an exit cleanup trap, and same-directory rename so failed dump/encryption pipelines leave no completed partial archive and successful archives are published atomically.
- Uses a PID-scoped `.partial` file, `pipefail`, an exit cleanup trap, and same-directory hard-link/no-clobber publication so failed dump/encryption pipelines leave no completed partial archive and successful archives are published atomically.
- Emits only final path/count on success; credentials use environment variables and never command-line arguments.
## TDD evidence
@@ -36,6 +36,25 @@ cd frontend && npm run lint exit 0
git diff --check exit 0
```
## TOCTOU follow-up (2026-07-20)
- Before `mkdir`/`cd`, the script walks all existing absolute-target ancestors, rejects symlinks, requires current-user-or-root ownership, and rejects group/world-writable modes. The unsafe-parent regression uses `realpathSync` for the macOS temporary root, so its nested symlink and `0777` parent are the components actually reached by validation.
- After creation it enters the directory with `cd -P`, verifies the canonical path and directory identity, and keeps the working directory pinned. Disk check, lock, partial/final publication, inventory, `find`, and rotation all use `.` or relative filenames; success still prints the canonical absolute archive path.
```text
RED:
cd frontend && npm run test:db
FAIL rejects unsafe writable backup parents before creating the target
The old script created the target and then failed only at the disk check, rather than rejecting the unsafe ancestor.
GREEN:
bash -n deploy/backup-staging-postgres.sh exit 0
cd frontend && npm run test:db 17 passed, 0 failed
cd frontend && npm test 493 passed, 0 failed
cd frontend && npm run lint exit 0
git diff --check exit 0
```
## Self-review and caveats
- Reviewed the final diff for secret exposure, output scope, filename filtering, rotation boundaries, portable shell options, and atomic/cleanup behavior; no task-scope finding remained.
deploy/backup-staging-postgres.sh
+58 -13
View File
@@ -20,6 +20,31 @@ reject_backup_directory() {
exit 1
}
reject_unsafe_backup_directory_ancestor() {
echo "backup directory ancestor must be owned by the current user or root and not group/world-writable" >&2
exit 1
}
stat_owner_and_mode() {
local path="$1"
if stat -f '%u %Lp' "$path" >/dev/null 2>&1; then
stat -f '%u %Lp' "$path"
else
stat -c '%u %a' "$path"
fi
}
directory_identity() {
local path="$1"
if stat -f '%d:%i' "$path" >/dev/null 2>&1; then
stat -f '%d:%i' "$path"
else
stat -c '%d:%i' "$path"
fi
}
if [ "$BACKUP_DIRECTORY_INPUT" = "/" ] || [[ "$BACKUP_DIRECTORY_INPUT" != /* ]] || [[ "$BACKUP_DIRECTORY_INPUT" == */ ]] || [[ "$BACKUP_DIRECTORY_INPUT" == *"//"* ]]; then
reject_backup_directory
fi
@@ -30,6 +55,7 @@ if [ "${#backup_directory_components[@]}" -eq 0 ]; then
fi
backup_directory_component_path=""
CURRENT_UID="$(id -u)"
for backup_directory_component in "${backup_directory_components[@]}"; do
if [ -z "$backup_directory_component" ] || [ "$backup_directory_component" = "." ] || [ "$backup_directory_component" = ".." ]; then
reject_backup_directory
@@ -38,6 +64,18 @@ for backup_directory_component in "${backup_directory_components[@]}"; do
if [ -L "$backup_directory_component_path" ]; then
reject_backup_directory
fi
if [ -e "$backup_directory_component_path" ]; then
if [ ! -d "$backup_directory_component_path" ]; then
reject_backup_directory
fi
read -r backup_directory_owner backup_directory_mode <<< "$(stat_owner_and_mode "$backup_directory_component_path")"
if [ "$backup_directory_owner" != "$CURRENT_UID" ] && [ "$backup_directory_owner" != "0" ]; then
reject_unsafe_backup_directory_ancestor
fi
if (( (10#${backup_directory_mode: -2:1} & 2) != 0 || (10#${backup_directory_mode: -1} & 2) != 0 )); then
reject_unsafe_backup_directory_ancestor
fi
fi
done
"$VALIDATOR" "$DATABASE_ENV_FILE" >/dev/null
@@ -60,13 +98,18 @@ STAGING_BACKUP_ENCRYPTION_KEY="$(read_environment_value STAGING_BACKUP_ENCRYPTIO
export STAGING_BACKUP_ENCRYPTION_KEY
mkdir -p "$BACKUP_DIRECTORY_INPUT"
BACKUP_DIRECTORY="$(cd "$BACKUP_DIRECTORY_INPUT" && pwd -P)"
cd -P "$BACKUP_DIRECTORY_INPUT"
BACKUP_DIRECTORY="$(pwd -P)"
if [ "$BACKUP_DIRECTORY" != "$BACKUP_DIRECTORY_INPUT" ] || [ "$BACKUP_DIRECTORY" = "/" ]; then
reject_backup_directory
fi
chmod 0700 "$BACKUP_DIRECTORY"
BACKUP_DIRECTORY_IDENTITY="$(directory_identity .)"
if [ "$(directory_identity "$BACKUP_DIRECTORY_INPUT")" != "$BACKUP_DIRECTORY_IDENTITY" ]; then
reject_backup_directory
fi
chmod 0700 .
DISK_USAGE="$(df -Pk "$BACKUP_DIRECTORY" | awk 'NR == 2 { gsub(/%/, "", $5); print $5 }')"
DISK_USAGE="$(df -Pk . | awk 'NR == 2 { gsub(/%/, "", $5); print $5 }')"
if ! [[ "$DISK_USAGE" =~ ^[0-9]+$ ]] || [ "$DISK_USAGE" -ge 70 ]; then
echo "backup directory disk usage must be below 70 percent" >&2
exit 1
@@ -79,9 +122,9 @@ if ! [[ "$BACKUP_TIMESTAMP" =~ ^[0-9]{8}T[0-9]{6}Z$ ]]; then
fi
FILE_NAME="jyotisha-staging-${BACKUP_TIMESTAMP}.dump.enc"
FINAL_FILE="$BACKUP_DIRECTORY/$FILE_NAME"
PARTIAL_FILE="$BACKUP_DIRECTORY/.${FILE_NAME}.$$.partial"
LOCK_DIRECTORY="$BACKUP_DIRECTORY/.${FILE_NAME}.lock"
FINAL_FILE="$FILE_NAME"
PARTIAL_FILE=".${FILE_NAME}.$$.partial"
LOCK_DIRECTORY=".${FILE_NAME}.lock"
LOCK_ACQUIRED=0
if [ -e "$FINAL_FILE" ] || [ -L "$FINAL_FILE" ]; then
@@ -110,10 +153,12 @@ LOCK_ACQUIRED=1
: > "$PARTIAL_FILE"
chmod 0600 "$PARTIAL_FILE"
cd "$REPOSITORY_ROOT"
docker compose -p "${COMPOSE_PROJECT_NAME:-jyotisha-staging}" \
-f deploy/docker-compose.postgres.yml exec -T postgres \
pg_dump -U "$POSTGRES_USER" -d "$POSTGRES_DB" --format=custom --no-owner |
(
cd "$REPOSITORY_ROOT"
docker compose -p "${COMPOSE_PROJECT_NAME:-jyotisha-staging}" \
-f deploy/docker-compose.postgres.yml exec -T postgres \
pg_dump -U "$POSTGRES_USER" -d "$POSTGRES_DB" --format=custom --no-owner
) |
openssl enc -aes-256-cbc -salt -pbkdf2 \
-pass env:STAGING_BACKUP_ENCRYPTION_KEY > "$PARTIAL_FILE"
@@ -126,7 +171,7 @@ rm -f "$PARTIAL_FILE"
PARTIAL_FILE=""
completed=()
if ! completed_paths="$(find "$BACKUP_DIRECTORY" -maxdepth 1 -type f -name 'jyotisha-staging-*.dump.enc' -print | LC_ALL=C sort)"; then
if ! completed_paths="$(find . -maxdepth 1 -type f -name 'jyotisha-staging-*.dump.enc' -print | LC_ALL=C sort)"; then
echo "failed to enumerate completed backups" >&2
exit 1
fi
@@ -139,8 +184,8 @@ done <<< "$completed_paths"
if [ "${#completed[@]}" -gt 3 ]; then
for ((index = 0; index < ${#completed[@]} - 3; index += 1)); do
rm -f "$BACKUP_DIRECTORY/${completed[$index]}"
rm -f "${completed[$index]}"
done
fi
printf 'path=%s count=%s\n' "$FINAL_FILE" "$(( ${#completed[@]} > 3 ? 3 : ${#completed[@]} ))"
printf 'path=%s count=%s\n' "$BACKUP_DIRECTORY/$FINAL_FILE" "$(( ${#completed[@]} > 3 ? 3 : ${#completed[@]} ))"
frontend/tests/database-backup.test.ts
+27 -1
View File
@@ -2,6 +2,7 @@ import assert from "node:assert/strict";
import { spawn, spawnSync } from "node:child_process";
import {
chmodSync,
existsSync,
mkdirSync,
mkdtempSync,
readdirSync,
@@ -227,7 +228,7 @@ test("staging backups are encrypted, atomic, private, and retain the newest thre
});
test("rejects destructive backup directory aliases and symlink components before mutation", () => {
const root = mkdtempSync(join(tmpdir(), "jyotisha-backup-boundary-"));
const root = canonicalTemporaryDirectory("jyotisha-backup-boundary-");
const environmentFile = createDatabaseEnvironment();
const target = join(root, "target");
const sentinel = join(target, "sentinel.txt");
@@ -271,6 +272,31 @@ test("rejects destructive backup directory aliases and symlink components before
}
});
test("rejects unsafe writable backup parents before creating the target", () => {
const root = canonicalTemporaryDirectory("jyotisha-backup-unsafe-parent-");
const environmentFile = createDatabaseEnvironment();
const unsafeParent = join(root, "unsafe-parent");
const target = join(unsafeParent, "backup");
const sentinel = join(unsafeParent, "sentinel.txt");
try {
mkdirSync(unsafeParent, { mode: 0o700 });
writeFileSync(sentinel, "must remain untouched");
chmodSync(unsafeParent, 0o777);
const result = runBackup(environmentFile.file, target, process.env, root);
assert.notEqual(result.status, 0);
assert.match(result.stderr, /backup directory ancestor must be owned by the current user or root and not group\/world-writable/);
assert.equal(existsSync(target), false);
assert.equal(statSync(unsafeParent).mode & 0o777, 0o777);
assert.equal(readFileSync(sentinel, "utf8"), "must remain untouched");
} finally {
rmSync(root, { force: true, recursive: true });
rmSync(environmentFile.directory, { force: true, recursive: true });
}
});
test("same-second backups publish once without overwriting the completed archive", async () => {
const environmentFile = createDatabaseEnvironment();
const backupDirectory = canonicalTemporaryDirectory("jyotisha-backup-collision-");