fix(recovery): retain logical restore archive and executable authority - #284
Conversation
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: trueThanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Caution Review failedAn error occurred during the review process. Please try again later. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
4 similar comments
|
Caution Review failedAn error occurred during the review process. Please try again later. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Caution Review failedAn error occurred during the review process. Please try again later. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Caution Review failedAn error occurred during the review process. Please try again later. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Caution Review failedAn error occurred during the review process. Please try again later. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
ACTIVE Draft — logical restore execution authority
Refs #204, #233, #254, #286, #287.
Protected
mainis exactb84f0c94154043a3473939c01bb6471de5a129ae. This source lane isfeat/postgres-logical-restore-executor-d0a4b30, current exact heada8863278b66b2067e272affe93c185c3566ea449, Draft and mechanically mergeable. It remains an explicit non-destructive child of dependency-root#233@5951b7a4d779903b8924abaef2a387cae50b7f54. Fresh targetedlogical-restorebranch inventory finds no second named branch owning this source/test surface. No source or documentation was written directly to protected main.Implemented candidate boundary
The archive side snapshots caller descriptor authority before use, verifies readable regular-file authority at offset zero, reopens the retained inode through
/proc/self/fdfor an independent child read offset, requires a private one-link bounded non-empty archive, retains the original snapshot through child execution, and revalidates that snapshot against the inspected archive metadata afterward. Caller FD replacement or seeking after the snapshot cannot redirect or move the child archive stream.The executable side retains the selected absolute
pg_restoreinode before child execution. The executable is opened read-only with close-on-exec, no-follow and nonblocking flags and accepted only when it is a root-owned regular file with at least one execute bit and without group/other write, set-user-ID or set-group-ID authority.subprocess.run()keeps the validated caller pathname only asargv[0]but executes/proc/self/fd/<retained-executable-fd>with explicitpass_fds, so final-path replacement after retention cannot substitute different executable bytes. Package-owned executable authority is closed infinally; process-control exceptions remain unmasked.The restricted libpq environment, finite connect/restore timeouts, trusted-source assertion, target-service boundary, shell-free invocation, single transaction, exit-on-error, bounded content-free diagnostics, custom-format seek behavior and post-commit metadata-mismatch warning remain intact.
RED -> narrow repair lineage
Earlier branch lineage covered archive descriptor substitution, shared-offset interference, archive inspection races and cleanup. Exact predecessor head
6ac42d8e1c90a8c993848dd866f0d760ec8c13d2reached terminal success on CI32413879355, Release Acceptance32413879456, Security Scan32413879337, and SAST Semgrep32413879328.Test-first commit
4667329b4eb94c9d1d3e88179229c4ddb77a9221defined the missing executable contract: child execution must be pinned to a retained executable descriptor through/proc/self/fdwith explicit inheritance even if the caller pathname changes after retention. Production commit8a0aa5b070bc42bb78dc018dc8124f26efab4993implemented that retained-inode ownership/mode and execution boundary.Exact-head CI
32424882290then exposed a branch-owned test-harness regression across Python 3.10, 3.12 and 3.14: hosted runners do not provide the tests' illustrative/usr/lib/postgresql/18/bin/pg_restoreor/usr/bin/pg_restorepaths, and the newly retained executable opener now runs before the already-mockedsubprocess.run(). Those legacy mocked-child tests therefore failed before reaching the behavior they were intended to exercise. This is not provider or runner evidence and does not justify weakening executable validation.Repair commit
a8863278b66b2067e272affe93c185c3566ea449keeps production unchanged. Only the three legacy mocked-child logical-restore test modules stub the private retained-executable opener with a real inert descriptor, isolating those tests from host PostgreSQL package inventory. The dedicated executable-authority test module remains unstubbed and now covers retained execution flags, bounded open failures, fstat failure cleanup, non-regular/non-root/non-executable/writable/set-id rejection, and secondary close failures. This preserves the product security contract while restoring unit-test determinism and exercising the new branch surface.Exact-head proof boundary
Fresh exact-head refetch on 2026-08-21 confirms all four repository-local workflows for unchanged
a8863278b66b2067e272affe93c185c3566ea449are terminal-success:32433492834: success;32433492831: success;32433492805: success;32433492854: success;This proves the repository-local quality/security/release layer for this exact contributor head only. Absence of formal review is not approval, and no predecessor result transfers across a future head or base change. Keep this exact head stable rather than manufacturing churn.
Integration and residual proof boundary
#233 must integrate first through live governance. This Draft cannot merge while its dependency root is unintegrated or while qualifying final review/current-base governance is absent. The executable candidate claims Linux
/proc/self/fdinode pinning and the explicit root-owned/mode boundary only. It does not claim package-manager provenance, immutable host roots, cross-platform parity, restore validity, PITR, RPO/RTO, HA/DR, CSAP, SOC 2, or certification.After #233 integrates, reconcile this Draft against resulting protected main and reacquire every exact-final-head/current-base quality, security, coverage, package, provenance, release, review and thread-resolution gate before considering merge.