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

fix: secure staging backup directory creation

Browse Source
This commit is contained in:
Jesse_Chen
2026-07-20 23:42:15 +08:00
parent 130904a54d
commit 440a56afad
3 changed files with 120 additions and 17 deletions
Download Patch File Download Diff File Expand all files Collapse all files
.superpowers/sdd/task-3-report.md
+20
View File
@@ -36,6 +36,26 @@ cd frontend && npm run lint exit 0
git diff --check exit 0
```
## Secure directory-creation follow-up (2026-07-20)
- `umask 077` now executes at script startup, before any possible `mkdir`. The script records the first absent component, then re-walks every component after `mkdir -p`; all newly created directories must be current-user owned, non-symlink directories without group/world write permission before `cd -P` pins the target.
- Existing root-owned sticky shared ancestors, including canonical `/tmp`, are allowed; ordinary group/world-writable ancestors remain rejected. The shared-temporary-root tests canonicalize `/tmp` with `realpathSync`, so they prove validation reaches their deliberately nested symlink and `0777` parent instead of stopping at the standard sticky ancestor.
```text
RED:
cd frontend && npm run test:db
FAIL rejects destructive backup directory aliases and symlink components before mutation
FAIL creates every absent backup path component privately despite a permissive caller umask
The prior policy rejected canonical /tmp and only set umask after mkdir.
GREEN:
bash -n deploy/backup-staging-postgres.sh exit 0
cd frontend && npm run test:db 18 passed, 0 failed
cd frontend && npm test 494 passed, 0 failed
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.
deploy/backup-staging-postgres.sh
+56 -14
View File
@@ -1,6 +1,7 @@
#!/usr/bin/env bash
set -euo pipefail
set +x
umask 077
if [ "$#" -ne 2 ]; then
echo "usage: backup-staging-postgres.sh DATABASE_ENV_FILE BACKUP_DIRECTORY" >&2
@@ -28,8 +29,8 @@ reject_unsafe_backup_directory_ancestor() {
stat_owner_and_mode() {
local path="$1"
if stat -f '%u %Lp' "$path" >/dev/null 2>&1; then
stat -f '%u %Lp' "$path"
if stat -f '%u %p' "$path" >/dev/null 2>&1; then
stat -f '%u %p' "$path"
else
stat -c '%u %a' "$path"
fi
@@ -45,6 +46,43 @@ directory_identity() {
fi
}
directory_mode_is_group_or_world_writable() {
local mode="$1"
local permissions="${mode: -3}"
(( (10#${permissions:1:1} & 2) != 0 || (10#${permissions:2:1} & 2) != 0 ))
}
directory_mode_is_sticky() {
local mode="$1"
[ "${#mode}" -ge 4 ] && (( (10#${mode: -4:1} & 1) != 0 ))
}
validate_backup_directory_component() {
local path="$1"
local require_private="$2"
local owner
local mode
if [ -L "$path" ] || [ ! -d "$path" ]; then
reject_backup_directory
fi
read -r owner mode <<< "$(stat_owner_and_mode "$path")"
if [ "$require_private" = "1" ]; then
if [ "$owner" != "$CURRENT_UID" ] || directory_mode_is_group_or_world_writable "$mode"; then
reject_unsafe_backup_directory_ancestor
fi
return
fi
if [ "$owner" != "$CURRENT_UID" ] && [ "$owner" != "0" ]; then
reject_unsafe_backup_directory_ancestor
fi
if directory_mode_is_group_or_world_writable "$mode" && ! { [ "$owner" = "0" ] && directory_mode_is_sticky "$mode"; }; then
reject_unsafe_backup_directory_ancestor
fi
}
if [ "$BACKUP_DIRECTORY_INPUT" = "/" ] || [[ "$BACKUP_DIRECTORY_INPUT" != /* ]] || [[ "$BACKUP_DIRECTORY_INPUT" == */ ]] || [[ "$BACKUP_DIRECTORY_INPUT" == *"//"* ]]; then
reject_backup_directory
fi
@@ -56,7 +94,9 @@ fi
backup_directory_component_path=""
CURRENT_UID="$(id -u)"
for backup_directory_component in "${backup_directory_components[@]}"; do
FIRST_CREATED_COMPONENT_INDEX=-1
for ((backup_directory_component_index = 0; backup_directory_component_index < ${#backup_directory_components[@]}; backup_directory_component_index += 1)); do
backup_directory_component="${backup_directory_components[$backup_directory_component_index]}"
if [ -z "$backup_directory_component" ] || [ "$backup_directory_component" = "." ] || [ "$backup_directory_component" = ".." ]; then
reject_backup_directory
fi
@@ -65,16 +105,9 @@ for backup_directory_component in "${backup_directory_components[@]}"; do
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
validate_backup_directory_component "$backup_directory_component_path" 0
elif [ "$FIRST_CREATED_COMPONENT_INDEX" -eq -1 ]; then
FIRST_CREATED_COMPONENT_INDEX="$backup_directory_component_index"
fi
done
@@ -98,6 +131,16 @@ STAGING_BACKUP_ENCRYPTION_KEY="$(read_environment_value STAGING_BACKUP_ENCRYPTIO
export STAGING_BACKUP_ENCRYPTION_KEY
mkdir -p "$BACKUP_DIRECTORY_INPUT"
backup_directory_component_path=""
for ((backup_directory_component_index = 0; backup_directory_component_index < ${#backup_directory_components[@]}; backup_directory_component_index += 1)); do
backup_directory_component="${backup_directory_components[$backup_directory_component_index]}"
backup_directory_component_path="${backup_directory_component_path}/${backup_directory_component}"
if [ "$FIRST_CREATED_COMPONENT_INDEX" -ne -1 ] && [ "$backup_directory_component_index" -ge "$FIRST_CREATED_COMPONENT_INDEX" ]; then
validate_backup_directory_component "$backup_directory_component_path" 1
else
validate_backup_directory_component "$backup_directory_component_path" 0
fi
done
cd -P "$BACKUP_DIRECTORY_INPUT"
BACKUP_DIRECTORY="$(pwd -P)"
if [ "$BACKUP_DIRECTORY" != "$BACKUP_DIRECTORY_INPUT" ] || [ "$BACKUP_DIRECTORY" = "/" ]; then
@@ -144,7 +187,6 @@ cleanup_partial() {
}
trap cleanup_partial EXIT HUP INT TERM
umask 077
if ! mkdir "$LOCK_DIRECTORY"; then
echo "backup destination is already being created" >&2
exit 1
frontend/tests/database-backup.test.ts
+44 -3
View File
@@ -54,6 +54,10 @@ function canonicalTemporaryDirectory(prefix: string): string {
return realpathSync(mkdtempSync(join(tmpdir(), prefix)));
}
function canonicalSharedTemporaryDirectory(prefix: string): string {
return realpathSync(mkdtempSync(join("/tmp", prefix)));
}
function listDumpArchive(fixture: ReturnType<typeof startPostgresFixture>, dump: Buffer): void {
const hostRestore = spawnSync("pg_restore", ["--list"], {
input: dump,
@@ -119,8 +123,12 @@ function runBackup(
backupDirectory: string,
environment: NodeJS.ProcessEnv,
cwd = repositoryRoot,
callerUmask?: string,
) {
return spawnSync("bash", [backupScript, databaseEnvFile, backupDirectory], {
const args = callerUmask
? ["-c", 'umask "$1"; shift; exec bash "$@"', "backup-umask", callerUmask, backupScript, databaseEnvFile, backupDirectory]
: [backupScript, databaseEnvFile, backupDirectory];
return spawnSync("bash", args, {
cwd,
encoding: "utf8",
env: environment,
@@ -228,7 +236,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 = canonicalTemporaryDirectory("jyotisha-backup-boundary-");
const root = canonicalSharedTemporaryDirectory("jyotisha-backup-boundary-");
const environmentFile = createDatabaseEnvironment();
const target = join(root, "target");
const sentinel = join(target, "sentinel.txt");
@@ -273,7 +281,7 @@ 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 root = canonicalSharedTemporaryDirectory("jyotisha-backup-unsafe-parent-");
const environmentFile = createDatabaseEnvironment();
const unsafeParent = join(root, "unsafe-parent");
const target = join(unsafeParent, "backup");
@@ -297,6 +305,39 @@ test("rejects unsafe writable backup parents before creating the target", () =>
}
});
test("creates every absent backup path component privately despite a permissive caller umask", () => {
const root = canonicalSharedTemporaryDirectory("jyotisha-backup-umask-");
const environmentFile = createDatabaseEnvironment();
const commandDirectory = mkdtempSync(join(tmpdir(), "jyotisha-backup-umask-command-"));
const components = ["first", "second", "backup"];
const target = join(root, ...components);
try {
safeDiskCommand(commandDirectory);
writeCommand(commandDirectory, "docker", "printf '%s' 'umask dump payload'");
const result = runBackup(
environmentFile.file,
target,
backupEnvironment(commandDirectory, { BACKUP_TIMESTAMP: "20260720T050101Z" }),
repositoryRoot,
"000",
);
assert.equal(result.status, 0, result.stderr);
let createdPath = root;
for (const component of components) {
createdPath = join(createdPath, component);
const created = statSync(createdPath);
assert.equal(created.uid, process.getuid?.());
assert.equal(created.mode & 0o777, 0o700);
}
} finally {
rmSync(root, { force: true, recursive: true });
rmSync(environmentFile.directory, { force: true, recursive: true });
rmSync(commandDirectory, { 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-");