fix: create staging backup path safely
This commit is contained in:
@@ -36,6 +36,25 @@ cd frontend && npm run lint exit 0
|
||||
git diff --check exit 0
|
||||
```
|
||||
|
||||
## Atomic component-creation follow-up (2026-07-20)
|
||||
|
||||
- The sticky root-owned exception now applies only to ancestors. An existing requested backup leaf is separately required to be current-user owned, non-symlink, and non-group/world-writable before the script can chmod or write it; direct canonical `/tmp` is rejected without calling `chmod` or changing its mode.
|
||||
- The script no longer uses `mkdir -p` for backup paths. It pins the deepest validated existing ancestor, creates each missing component with one plain relative `mkdir`, validates it, and `cd -P`s into it before creating the next. If a concurrent actor creates any component first, including a symlink, the script rejects it without descending through it.
|
||||
|
||||
```text
|
||||
RED:
|
||||
cd frontend && npm run test:db
|
||||
FAIL rejects a direct canonical sticky shared backup directory before chmod
|
||||
FAIL fails safely when a racer inserts a symlink during nested backup path creation
|
||||
|
||||
GREEN:
|
||||
bash -n deploy/backup-staging-postgres.sh exit 0
|
||||
cd frontend && npm run test:db 20 passed, 0 failed
|
||||
cd frontend && npm test 496 passed, 0 failed
|
||||
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.
|
||||
|
||||
@@ -95,6 +95,8 @@ fi
|
||||
backup_directory_component_path=""
|
||||
CURRENT_UID="$(id -u)"
|
||||
FIRST_CREATED_COMPONENT_INDEX=-1
|
||||
DEEPEST_EXISTING_COMPONENT_INDEX=-1
|
||||
DEEPEST_EXISTING_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]}"
|
||||
if [ -z "$backup_directory_component" ] || [ "$backup_directory_component" = "." ] || [ "$backup_directory_component" = ".." ]; then
|
||||
@@ -106,6 +108,8 @@ for ((backup_directory_component_index = 0; backup_directory_component_index < $
|
||||
fi
|
||||
if [ -e "$backup_directory_component_path" ]; then
|
||||
validate_backup_directory_component "$backup_directory_component_path" 0
|
||||
DEEPEST_EXISTING_COMPONENT_INDEX="$backup_directory_component_index"
|
||||
DEEPEST_EXISTING_COMPONENT_PATH="$backup_directory_component_path"
|
||||
elif [ "$FIRST_CREATED_COMPONENT_INDEX" -eq -1 ]; then
|
||||
FIRST_CREATED_COMPONENT_INDEX="$backup_directory_component_index"
|
||||
fi
|
||||
@@ -130,18 +134,24 @@ POSTGRES_USER="$(read_environment_value POSTGRES_USER)"
|
||||
STAGING_BACKUP_ENCRYPTION_KEY="$(read_environment_value STAGING_BACKUP_ENCRYPTION_KEY)"
|
||||
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"
|
||||
if [ "$FIRST_CREATED_COMPONENT_INDEX" -eq -1 ]; then
|
||||
validate_backup_directory_component "$BACKUP_DIRECTORY_INPUT" 1
|
||||
fi
|
||||
|
||||
cd -P "$DEEPEST_EXISTING_COMPONENT_PATH"
|
||||
if [ "$FIRST_CREATED_COMPONENT_INDEX" -ne -1 ]; then
|
||||
for ((backup_directory_component_index = DEEPEST_EXISTING_COMPONENT_INDEX + 1; backup_directory_component_index < ${#backup_directory_components[@]}; backup_directory_component_index += 1)); do
|
||||
backup_directory_component="${backup_directory_components[$backup_directory_component_index]}"
|
||||
if [ -e "$backup_directory_component" ] || [ -L "$backup_directory_component" ]; then
|
||||
reject_backup_directory
|
||||
fi
|
||||
if ! mkdir "$backup_directory_component"; then
|
||||
reject_backup_directory
|
||||
fi
|
||||
validate_backup_directory_component "$backup_directory_component" 1
|
||||
cd -P "$backup_directory_component"
|
||||
done
|
||||
fi
|
||||
BACKUP_DIRECTORY="$(pwd -P)"
|
||||
if [ "$BACKUP_DIRECTORY" != "$BACKUP_DIRECTORY_INPUT" ] || [ "$BACKUP_DIRECTORY" = "/" ]; then
|
||||
reject_backup_directory
|
||||
|
||||
@@ -305,6 +305,71 @@ test("rejects unsafe writable backup parents before creating the target", () =>
|
||||
}
|
||||
});
|
||||
|
||||
test("rejects a direct canonical sticky shared backup directory before chmod", () => {
|
||||
const sharedDirectory = realpathSync("/tmp");
|
||||
const environmentFile = createDatabaseEnvironment();
|
||||
const commandDirectory = mkdtempSync(join(tmpdir(), "jyotisha-backup-direct-shared-command-"));
|
||||
const chmodLog = join(commandDirectory, "chmod.log");
|
||||
const originalMode = statSync(sharedDirectory).mode & 0o777;
|
||||
|
||||
try {
|
||||
writeCommand(commandDirectory, "chmod", "printf '%s' called > \"$CHMOD_LOG\"\nexit 97");
|
||||
const result = runBackup(
|
||||
environmentFile.file,
|
||||
sharedDirectory,
|
||||
backupEnvironment(commandDirectory, { CHMOD_LOG: chmodLog }),
|
||||
);
|
||||
|
||||
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(chmodLog), false);
|
||||
assert.equal(statSync(sharedDirectory).mode & 0o777, originalMode);
|
||||
} finally {
|
||||
rmSync(environmentFile.directory, { force: true, recursive: true });
|
||||
rmSync(commandDirectory, { force: true, recursive: true });
|
||||
}
|
||||
});
|
||||
|
||||
test("fails safely when a racer inserts a symlink during nested backup path creation", () => {
|
||||
const root = canonicalSharedTemporaryDirectory("jyotisha-backup-create-race-");
|
||||
const raceTarget = canonicalTemporaryDirectory("jyotisha-backup-race-target-");
|
||||
const environmentFile = createDatabaseEnvironment();
|
||||
const commandDirectory = mkdtempSync(join(tmpdir(), "jyotisha-backup-create-race-command-"));
|
||||
const target = join(root, "first", "second", "backup");
|
||||
const originalRootMode = statSync(root).mode & 0o777;
|
||||
|
||||
try {
|
||||
safeDiskCommand(commandDirectory);
|
||||
writeCommand(commandDirectory, "docker", "printf '%s' 'race dump payload'");
|
||||
writeCommand(
|
||||
commandDirectory,
|
||||
"mkdir",
|
||||
`if [ "$#" -eq 1 ] && [ "$1" = first ]; then
|
||||
ln -s "$RACE_TARGET" first
|
||||
fi
|
||||
exec /bin/mkdir "$@"`,
|
||||
);
|
||||
const result = runBackup(
|
||||
environmentFile.file,
|
||||
target,
|
||||
backupEnvironment(commandDirectory, {
|
||||
BACKUP_TIMESTAMP: "20260720T060101Z",
|
||||
RACE_TARGET: raceTarget,
|
||||
}),
|
||||
);
|
||||
|
||||
assert.notEqual(result.status, 0);
|
||||
assert.equal(existsSync(join(raceTarget, "second")), false);
|
||||
assert.equal(existsSync(join(raceTarget, "backup")), false);
|
||||
assert.equal(statSync(root).mode & 0o777, originalRootMode);
|
||||
} finally {
|
||||
rmSync(root, { force: true, recursive: true });
|
||||
rmSync(raceTarget, { force: true, recursive: true });
|
||||
rmSync(environmentFile.directory, { force: true, recursive: true });
|
||||
rmSync(commandDirectory, { force: true, recursive: true });
|
||||
}
|
||||
});
|
||||
|
||||
test("creates every absent backup path component privately despite a permissive caller umask", () => {
|
||||
const root = canonicalSharedTemporaryDirectory("jyotisha-backup-umask-");
|
||||
const environmentFile = createDatabaseEnvironment();
|
||||
|
||||
Reference in New Issue
Block a user