Recover corrupted notary signature databases

[krassowski]

• Adds a failing test
• Fixes Try to recover/fallback to an in-memory database if nbsignatures fails #428
  • recovers on initialization
  • recovers on runtime?
  • exposes recover method publicly for downstreams to decide when to recover?

[krassowski]

CC @Carreau. FYI I do not have a permission to add a label or request a review on this repo.

[Carreau]

Gave you triage right, ask if you need more. Assigned myself.

[Carreau]

This look s good to me, do you want to work more on this ? Merge as is (and I'll do a release soon) ? Or just hand it over to me ?

[Carreau]

let's try to get that in already, and iterate if necessary.

[Copilot]

Pull request overview

This PR improves resilience of the notebook signature (notary) SQLite database (nbsignatures.db) by detecting corruption early and attempting recovery (with a .bak backup), matching the goals in issue #428 to avoid save failures caused by broken signature DBs.

Changes:

• Adds an integrity check during SQLiteSignatureStore initialization to detect corrupted databases and trigger recovery.
• Implements recovery paths: prefer sqlite3 CLI .recover, with a Python best-effort fallback when the CLI is unavailable or fails.
• Adds a test that corrupts a seeded signatures DB and asserts that at least one signature can be recovered and that a .bak file is created.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
tests/test_sign.py	Adds a corruption helper and a regression test asserting recovery/backup behavior.
nbformat/sign.py	Adds integrity checks and implements recovery logic (CLI + Python fallback) when opening a corrupted signatures DB.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Suppressed comments (4)

nbformat/sign.py:161

• _connect_db calls init_db(db) before verifying integrity via PRAGMA quick_check. If the on-disk DB is corrupted, init_db can write to the file (schema/journal changes) before it gets renamed to .bak, which can reduce the effectiveness of later recovery. Checking integrity immediately after connect and only then initializing the schema avoids mutating a potentially recoverable file.

            db = sqlite3.connect(db_file, **kwargs)
            self.init_db(db)
            self._check_db_integrity(db)

nbformat/sign.py:172

• This warning says the old signatures DB "has been renamed" before the rename is attempted. If the rename fails (e.g., permissions / existing .bak on Windows), the log message is misleading. Consider wording it as an attempt rather than a completed action.

                    (
                        "The signatures database cannot be opened; maybe it is corrupted or encrypted. "
                        "You may need to rerun your notebooks to ensure that they are trusted to run Javascript. "
                        "The old signatures database has been renamed to %s."
                    ),

nbformat/sign.py:177

• Path.rename(old_db_location) can fail on Windows if the destination .bak file already exists. That would force an unnecessary fallback to the in-memory DB and skip recovery. Using Path.replace provides consistent overwrite semantics across platforms.

                try:
                    Path(db_file).rename(old_db_location)
                    db = self.recover(old_db_location, db_file)

nbformat/sign.py:230

• subprocess.run(..., text=True) decodes stdout using the locale encoding with strict error handling. On some systems/locales or if sqlite3 emits non-decodable bytes, this can raise UnicodeDecodeError (not caught here), preventing recovery and bypassing the intended fallback path. Setting an explicit encoding with a non-strict error handler makes this robust.

            recovered_sql = subprocess.run(  # noqa: S603
                [sqlite_cli, "-batch", "--", old_db_location, ".recover"],
                check=False,
                capture_output=True,
                text=True,