Skip to content

fix: robustness & security audit remediation (2026-07-04) - #10

Open
gliatti wants to merge 6 commits into
mainfrom
fix/audit-2026-07-04
Open

gliatti wants to merge 6 commits into
mainfrom
fix/audit-2026-07-04

Conversation

@gliatti

@gliatti gliatti commented Jul 4, 2026

Copy link
Copy Markdown
Owner

Contexte

Remédiation de l'audit robustesse & sécurité du 2026-07-04 (9 volets : sécurité crypto/storage/TLS, config/CLI/protocole/db/postgres, tests/CI/hygiène, et robustesse backup/restore/expire/archive/verify-stanza/pgbr-info+pipeline). Les deux correctifs prioritaires identifiés dès juin (pagination des listings cloud, validateur de chemin anti-../) sont enfin appliqués.

Fondations

  • Validateur de chemin central (pgbr-storage/src/path_validate.rs) rejetant .. et les chemins absolus, appliqué au worker de stockage distant ; durcissement Posix + fsync du répertoire parent après rename atomique.
  • Écritures atomiques : InfoBackup/InfoArchive::save et Manifest::save_keyed routés par write_with_copy (temp+rename + mirroir .copy + fallback au load).
  • Anti-injection manifest : échappement/rejet des clés INI hostiles (nom de fichier avec \n, [, #, …).
  • Pagination cloud : list() S3/Azure/GCS suit continuation-token / NextMarker / pageToken — plus de troncature silencieuse.

Correctifs par commande

  • backup : block_no par segment de relation, pg_backup_stop garanti en cas d'erreur, chaîne backup-reference transitive complète, prior filtré par db-id, vérification d'existence de l'objet au resume, cible des symlinks enregistrée (Storage::read_link).
  • expire : rétention diff transitive, save de backup.info avant suppression physique, préservation des ranges WAL du layout plat, boundary full time-based, .history conservés.
  • restore : gardes postmaster.pid / PGDATA non vide, tablespace_map non restauré en live (écrit .old), symlinks créés avant la copie avec erreurs dures, fsync données+répertoires, validation des chemins du manifest, nettoyage des fichiers signaux, fuseau PITR, écriture atomique write-and-verify.
  • archive : write_segment atomique, push idempotent (compare avant écrasement), archive-get avec contrôle d'intégrité + écriture atomique fsync vers PG, ordre .ok-avant-suppression du drain.
  • verify/check/stanza : propagation des erreurs archive_mode/archive_command, ordre stanza-delete plus sûr, noms de sonde aléatoires.
  • config : rédaction des valeurs d'options secure dans les erreurs, rejet des clés INI inconnues, allow-range par-flavor.
  • protocol : lecture de ligne bornée et bufferisée (anti-DoS mémoire).
  • pipeline/repo : erreur si backup chiffré sans clé, erreur sur compress-type inconnu au restore, unification repo-get/put sur la chaîne KDF keyed.

Validation

  • Gate : cargo fmt --check + cargo clippy --workspace --all-targets -- -D warnings + cargo test --workspace — vert.
  • Intégration Docker : 12/12 scénarios (01→12) passent. Le scénario 06-tablespaces, qui échouait car le durcissement du restore avait exposé l'absence d'enregistrement des cibles de symlinks côté backup, est corrigé à la racine.

Hors périmètre (follow-ups)

  • Low verrouillés pour compat on-disk : KDF EVP_BytesToKey 1-itération, verify-tls=n.
  • Storage::read_link pour les backends distants/objet (tablespaces en backup distant) — le backup local est couvert.

🤖 Generated with Claude Code

gliatti and others added 2 commits July 4, 2026 13:10
Foundations:
- add central repo-path validator (rejects '..' and absolute paths),
  applied to the remote storage worker; harden Posix and fsync parent
  dir after atomic rename
- route InfoBackup/InfoArchive::save and Manifest::save_keyed through the
  atomic write_with_copy path (temp+rename + .copy mirror + load fallback)
- escape/reject hostile INI keys in the manifest (anti-injection)
- paginate S3/Azure/GCS list() (continuation-token / NextMarker /
  pageToken) so listings are never silently truncated

Command paths:
- backup: per-segment page block_no, backup_stop on error, full
  transitive backup-reference chain, db-id-filtered prior resolution,
  resume repo-object existence check
- expire: transitive diff retention, save before physical delete,
  flat-layout WAL range preservation, time-based boundary full, keep
  .history files
- restore: postmaster.pid / non-empty PGDATA guards, do not restore
  tablespace_map, create symlinks before copy with hard errors, fsync
  data+dirs, manifest path validation, signal-file cleanup, PITR
  timezone, atomic write-and-verify
- archive: atomic write_segment, idempotent push, archive-get integrity
  check + atomic fsync to PG, drain .ok-before-remove ordering
- verify/check/stanza: propagate archive_mode errors, safer
  stanza-delete order, randomized probe names
- config: redact secret option values in errors, reject unknown INI
  keys, per-flavor allow-range
- protocol: bounded, buffered line read (anti-DoS)
- pipeline/repo: error when encrypted without key, error on unknown
  compress-type at restore, unify repo-get/put on keyed KDF

Gate: cargo fmt + clippy --all-targets -D warnings + test --workspace
all pass.

Co-Authored-By: Claude Fable 5 <[email protected]>
Add read_link to the Storage trait (default: unsupported-backend error)
with a Posix implementation, and use it in the backup walk so
ManifestLink entries carry the real destination instead of an empty
string. This completes the tablespace restore path: restore now recreates
tablespace symlinks (and refuses empty destinations honestly). Remote
backends without link-target support fall back to an empty destination,
the prior known-limitation behaviour, kept as a follow-up.

Fixes the 06-tablespaces integration scenario that the audit restore
fixes had exposed. Full suite: 12/12 scenarios pass.

Co-Authored-By: Claude Fable 5 <[email protected]>
@gitguardian

gitguardian Bot commented Jul 4, 2026 •

Copy link
Copy Markdown

️✅ There are no secrets present in this pull request anymore.

If these secrets were true positive and are still valid, we highly recommend you to revoke them.
While these secrets were previously flagged, we no longer have a reference to the
specific commits where they were detected. Once a secret has been leaked into a git
repository, you should consider it compromised, even if it was deleted immediately.
Find here more information about risks.


🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.

gliatti and others added 4 commits July 4, 2026 13:41
The S3 NextContinuationToken and Azure NextMarker in the pagination unit
tests were realistic high-entropy strings that tripped a secret scanner
(false positive: pagination cursors are not credentials). Replace them
with clearly-synthetic low-entropy placeholders exercising the same code
path.

Co-Authored-By: Claude Fable 5 <[email protected]>
Add ignored-matches for the two synthetic cloud pagination tokens (S3
NextContinuationToken, Azure NextMarker) used in list() unit tests. They
live inline in crate src files alongside real signing code, so they
cannot be path-ignored; ignoring the exact values also silences the
occurrences still present in git history (the triggered false-positive
incident) without blinding the scanner to any real secret.

Co-Authored-By: Claude Fable 5 <[email protected]>
…zure guard)

Follow-up from a max-effort multi-agent review of the audit branch:

- storage: add typed StorageError::Unsupported returned by the
  create_symlink / read_link trait defaults. backup's read_link fallback
  now matches Unsupported only, so a genuine local read_link I/O error
  (which maps to Backend/Io) propagates and fails the backup loudly
  instead of silently recording an empty, unrestorable symlink target;
  restore's is_symlink_unsupported matches the variant structurally
  instead of substring-matching the error message.
- archive: write_atomic_std (the parallel drain/prefetch WAL path) now
  fsyncs the parent directory after rename, matching posix
  write_atomic_local — the serial and parallel push paths now have equal
  crash durability. Also correct a stale "size and timeline" comment.
- azure: guard the list() pagination loop against a non-advancing marker
  so a buggy/hostile endpoint echoing the same NextMarker cannot loop
  forever accumulating duplicates (S3 also checks IsTruncated; Azure has
  no such flag).

Gate: fmt + clippy --all-targets -D warnings + test --workspace green;
integration scenario 06 (tablespaces) passes.

Co-Authored-By: Claude Fable 5 <[email protected]>
- verify: resolve repo/backup cipher sub-keys (RepoKeys) so `verify`
  actually decrypts an encrypted repository instead of hashing ciphertext
  and reporting a healthy repo as corrupt. New integration scenario
  13-verify-encrypted validates both a clean verify and corruption
  detection on an encrypted repo.
- remote: add a storage-read-link protocol command + worker handler +
  RemoteStorage/cli client impls, so a backup whose PGDATA is reached over
  SSH/TLS records real symlink (tablespace) targets. Path arg goes through
  the same traversal validator as the other storage-* commands.
- storage: add a shared pagination helper (crate::pagination) and route
  S3/Azure/GCS list() through it (removes three near-identical loops, with
  a single non-advancing-marker guard); resolve the GCS OAuth token per
  page so a long listing cannot die on an expired token.
- storage: export atomic_write_file + fsync_dir from pgbr-storage; archive
  (write_atomic_std) and restore (write_and_verify / fsync_parent_dir) now
  share one crash-safe implementation instead of three hand-rolled copies.
- archive: self-heal an idempotent push against a pre-existing torn/
  undecodable repo object (overwrite) while still erroring on a decodable
  object with different content.
- restore: parse a --target timestamp whose timezone offset is
  space-separated instead of silently dropping it.
- backup: warn (instead of silently skipping) when a [backup:current]
  entry has a missing/non-numeric db-id, so a silent full-promotion is
  visible.
- expire: feed dependent_closure borrowed JSON values instead of deep-
  cloning every retained backup entry.

Gate: fmt + clippy --all-targets -D warnings + test --workspace green.
Integration: 13/13 scenarios pass (incl. new 13-verify-encrypted).

Co-Authored-By: Claude Fable 5 <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant