From 4232c893e821cdee214f70187845d51851e0b76d Mon Sep 17 00:00:00 2001 From: Jesse_Chen Date: Tue, 21 Jul 2026 00:05:59 +0800 Subject: [PATCH] fix: stop backup path prescan at missing component --- .superpowers/sdd/task-3-report.md | 19 +++++++++ deploy/backup-staging-postgres.sh | 13 ++++-- frontend/tests/database-backup.test.ts | 56 ++++++++++++++++++++++++-- 3 files changed, 81 insertions(+), 7 deletions(-) diff --git a/.superpowers/sdd/task-3-report.md b/.superpowers/sdd/task-3-report.md index f17b809c..56b3c725 100644 --- a/.superpowers/sdd/task-3-report.md +++ b/.superpowers/sdd/task-3-report.md @@ -36,6 +36,25 @@ cd frontend && npm run lint exit 0 git diff --check exit 0 ``` +## Pre-scan race follow-up (2026-07-21) + +- Lexical component validation now completes before ancestor scanning. Once scanning finds the first absent component, it stops constructing deeper absolute pathnames immediately; creation continues only from the already pinned deepest existing directory. +- A deterministic `BASH_ENV` DEBUG-hook regression inserts a symlink after the first missing component is recorded. The script rejects it from the pinned parent and creates no `backup` path under the symlink target. Creation uses `./$component` for test, mkdir, validation, and `cd -P`, so legal names beginning with `-` are handled as names rather than options. + +```text +RED: +cd frontend && npm run test:db +FAIL stops absolute pre-scan traversal when a symlink appears after the first missing component +FAIL creates every absent backup path component privately despite a permissive caller umask + +GREEN: +bash -n deploy/backup-staging-postgres.sh exit 0 +cd frontend && npm run test:db 21 passed, 0 failed +cd frontend && npm test 497 passed, 0 failed +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. diff --git a/deploy/backup-staging-postgres.sh b/deploy/backup-staging-postgres.sh index 0e6acc14..75f60e75 100755 --- a/deploy/backup-staging-postgres.sh +++ b/deploy/backup-staging-postgres.sh @@ -102,6 +102,10 @@ for ((backup_directory_component_index = 0; backup_directory_component_index < $ if [ -z "$backup_directory_component" ] || [ "$backup_directory_component" = "." ] || [ "$backup_directory_component" = ".." ]; then reject_backup_directory fi +done + +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 [ -L "$backup_directory_component_path" ]; then reject_backup_directory @@ -112,6 +116,7 @@ for ((backup_directory_component_index = 0; 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" + break fi done @@ -142,14 +147,14 @@ 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 + if [ -e "./$backup_directory_component" ] || [ -L "./$backup_directory_component" ]; then reject_backup_directory fi - if ! mkdir "$backup_directory_component"; then + if ! mkdir "./$backup_directory_component"; then reject_backup_directory fi - validate_backup_directory_component "$backup_directory_component" 1 - cd -P "$backup_directory_component" + validate_backup_directory_component "./$backup_directory_component" 1 + cd -P "./$backup_directory_component" done fi BACKUP_DIRECTORY="$(pwd -P)" diff --git a/frontend/tests/database-backup.test.ts b/frontend/tests/database-backup.test.ts index fc75260b..fae7d3ff 100644 --- a/frontend/tests/database-backup.test.ts +++ b/frontend/tests/database-backup.test.ts @@ -344,8 +344,8 @@ test("fails safely when a racer inserts a symlink during nested backup path crea writeCommand( commandDirectory, "mkdir", - `if [ "$#" -eq 1 ] && [ "$1" = first ]; then - ln -s "$RACE_TARGET" first + `if [ "$#" -eq 1 ] && [ "$1" = ./first ]; then + ln -s "$RACE_TARGET" ./first fi exec /bin/mkdir "$@"`, ); @@ -370,11 +370,61 @@ exec /bin/mkdir "$@"`, } }); +test("stops absolute pre-scan traversal when a symlink appears after the first missing component", () => { + const root = canonicalSharedTemporaryDirectory("jyotisha-backup-pre-scan-race-"); + const raceTarget = canonicalTemporaryDirectory("jyotisha-backup-pre-scan-target-"); + const environmentFile = createDatabaseEnvironment(); + const commandDirectory = mkdtempSync(join(tmpdir(), "jyotisha-backup-pre-scan-command-")); + const preScanHook = join(commandDirectory, "pre-scan-hook.sh"); + const firstComponent = join(root, "first"); + const target = join(firstComponent, "second", "backup"); + const externalBackup = join(raceTarget, "second", "backup"); + const originalRootMode = statSync(root).mode & 0o777; + + try { + mkdirSync(join(raceTarget, "second"), { mode: 0o700 }); + safeDiskCommand(commandDirectory); + writeCommand(commandDirectory, "docker", "printf '%s' 'pre-scan race dump payload'"); + writeFileSync( + preScanHook, + `pre_scan_inject() { + if [ "${"${BASH_COMMAND:-}"}" = 'FIRST_CREATED_COMPONENT_INDEX="$backup_directory_component_index"' ] && [ "${"${backup_directory_component_path:-}"}" = "$PRE_SCAN_FIRST_PATH" ]; then + ln -s "$PRE_SCAN_RACE_TARGET" "$PRE_SCAN_FIRST_PATH" + trap - DEBUG + fi +} +trap pre_scan_inject DEBUG +`, + { mode: 0o700 }, + ); + chmodSync(preScanHook, 0o700); + const result = runBackup( + environmentFile.file, + target, + backupEnvironment(commandDirectory, { + BACKUP_TIMESTAMP: "20260720T060201Z", + BASH_ENV: preScanHook, + PRE_SCAN_FIRST_PATH: firstComponent, + PRE_SCAN_RACE_TARGET: raceTarget, + }), + ); + + assert.notEqual(result.status, 0); + assert.equal(existsSync(externalBackup), 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(); const commandDirectory = mkdtempSync(join(tmpdir(), "jyotisha-backup-umask-command-")); - const components = ["first", "second", "backup"]; + const components = ["first", "-second", "backup"]; const target = join(root, ...components); try {