+ Choose the source, then the destination. Direction matters.
+
+
+
+
+ Semantic lens
+ Compare system roles
+
+
+
+
Choose up to two semantic kinds. One reveals its real traffic; two compare only direct authored relationships.
+
+
Choose a kind to inspect its nodes and touching relationships.
+
+
+
+
+
+
+
+
+
+ Semantic radar
+ Building overview
+
+
+
+
+
+
Click nodeDrag to pan
+
+
Semantic radar needs more MAP space.
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
Atomicity
+
+
+
• The final path holds a complete file or nothing
+
• Work happens in a temp file inside the destination directory
+
• One rename publishes; a crash leaves only namable debris
+
+
+
+
+
+
+
Idempotent retries
+
+
+
• skip treats an existing output as finished work and reads nothing
+
• extract completes a partial set file by file under skip
+
• overwrite replaces atomically for reprocessing pipelines
+
+
+
+
+
+
+
Deterministic failures
+
+
+
• Exit classes 2-6 are permanent; retrying them is waste
+
• Only exit 1 and pod-level errors are worth a retry
+
• The terminal JSON event carries the machine-readable error_code
+
+
+
+
+
+
+
+
+
diff --git a/docs/diagrams/retry-machine.lifecycle.json b/docs/diagrams/retry-machine.lifecycle.json
new file mode 100644
index 0000000..ffec118
--- /dev/null
+++ b/docs/diagrams/retry-machine.lifecycle.json
@@ -0,0 +1,255 @@
+{
+ "schema_version": 1,
+ "diagram_type": "lifecycle",
+ "meta": {
+ "title": "Run Lifecycle and Retry Semantics",
+ "quality_profile": "showcase",
+ "views": [
+ {
+ "id": "happy-run",
+ "label": "One clean run",
+ "focus": [
+ "invoked",
+ "parsed",
+ "existscheck",
+ "publishing",
+ "complete"
+ ],
+ "note": "Follow a run from invocation to the atomically published output."
+ },
+ {
+ "id": "policy-branches",
+ "label": "Existing-output policy",
+ "focus": [
+ "existscheck",
+ "skipped",
+ "refused"
+ ],
+ "note": "PDFOPS_ON_EXISTS decides what an existing output means."
+ },
+ {
+ "id": "crash-retry",
+ "label": "Crash and retry",
+ "focus": [
+ "publishing",
+ "killed",
+ "parsed"
+ ],
+ "note": "A killed run leaves debris the next run removes before writing."
+ }
+ ]
+ },
+ "lanes": [
+ {
+ "id": "main",
+ "label": "Run phases"
+ },
+ {
+ "id": "recovery",
+ "label": "Interruptions"
+ },
+ {
+ "id": "terminal",
+ "label": "Policy outcomes"
+ }
+ ],
+ "states": [
+ {
+ "id": "invoked",
+ "type": "start",
+ "label": "Invoked",
+ "sublabel": "one operation per run",
+ "lane": "main",
+ "col": 0,
+ "step": "01",
+ "tag": "entry"
+ },
+ {
+ "id": "parsed",
+ "type": "active",
+ "label": "Config parsed",
+ "sublabel": "before any file I/O",
+ "lane": "main",
+ "col": 1,
+ "step": "02",
+ "tag": "pure"
+ },
+ {
+ "id": "existscheck",
+ "type": "decision",
+ "label": "Output exists?",
+ "sublabel": "PDFOPS_ON_EXISTS",
+ "lane": "main",
+ "col": 2,
+ "step": "03",
+ "tag": "policy"
+ },
+ {
+ "id": "publishing",
+ "type": "active",
+ "label": "Temp write",
+ "sublabel": "fsync, then one rename",
+ "lane": "main",
+ "col": 3,
+ "step": "04",
+ "tag": "atomic"
+ },
+ {
+ "id": "complete",
+ "type": "success",
+ "label": "Complete",
+ "sublabel": "exit 0, terminal event",
+ "lane": "main",
+ "col": 4,
+ "step": "05",
+ "tag": "done"
+ },
+ {
+ "id": "skipped",
+ "type": "success",
+ "label": "Skipped",
+ "sublabel": "exit 0, reads nothing",
+ "lane": "terminal",
+ "col": 0,
+ "tag": "skip"
+ },
+ {
+ "id": "killed",
+ "type": "failure",
+ "label": "Killed mid-write",
+ "sublabel": "temp debris only",
+ "lane": "recovery",
+ "col": 2,
+ "tag": "retryable"
+ },
+ {
+ "id": "refused",
+ "type": "failure",
+ "label": "OUTPUT_EXISTS",
+ "sublabel": "exit 6, no clobber",
+ "lane": "terminal",
+ "col": 1,
+ "tag": "fail"
+ }
+ ],
+ "transitions": [
+ {
+ "id": "policy-skip",
+ "from": "existscheck",
+ "to": "skipped",
+ "label": "skip: completed prior work",
+ "variant": "emphasis",
+ "route": "drop"
+ },
+ {
+ "id": "policy-fail",
+ "from": "existscheck",
+ "to": "refused",
+ "label": "fail (default)",
+ "variant": "security",
+ "fromSide": "right",
+ "toSide": "top",
+ "via": [
+ [
+ 479,
+ 157
+ ],
+ [
+ 479,
+ 212
+ ],
+ [
+ 556,
+ 212
+ ]
+ ],
+ "labelAt": [
+ 517,
+ 200
+ ]
+ },
+ {
+ "id": "write-killed",
+ "from": "publishing",
+ "to": "killed",
+ "label": "pod lost",
+ "variant": "security",
+ "labelAt": [
+ 672,
+ 246
+ ],
+ "fromSide": "right",
+ "toSide": "top",
+ "via": [
+ [
+ 633,
+ 157
+ ],
+ [
+ 633,
+ 258
+ ],
+ [
+ 710,
+ 258
+ ]
+ ]
+ },
+ {
+ "id": "retry-cleanup",
+ "from": "killed",
+ "to": "parsed",
+ "label": "retry: stale temp removed",
+ "variant": "emphasis",
+ "fromSide": "bottom",
+ "toSide": "top",
+ "via": [
+ [
+ 710,
+ 560
+ ],
+ [
+ 16,
+ 560
+ ],
+ [
+ 16,
+ 60
+ ],
+ [
+ 248,
+ 60
+ ]
+ ]
+ }
+ ],
+ "cards": [
+ {
+ "dot": "emerald",
+ "title": "Atomicity",
+ "items": [
+ "The final path holds a complete file or nothing",
+ "Work happens in a temp file inside the destination directory",
+ "One rename publishes; a crash leaves only namable debris"
+ ]
+ },
+ {
+ "dot": "amber",
+ "title": "Idempotent retries",
+ "items": [
+ "skip treats an existing output as finished work and reads nothing",
+ "extract completes a partial set file by file under skip",
+ "overwrite replaces atomically for reprocessing pipelines"
+ ]
+ },
+ {
+ "dot": "rose",
+ "title": "Deterministic failures",
+ "items": [
+ "Exit classes 2-6 are permanent; retrying them is waste",
+ "Only exit 1 and pod-level errors are worth a retry",
+ "The terminal JSON event carries the machine-readable error_code"
+ ]
+ }
+ ]
+}
\ No newline at end of file
From 935c2c9f8bd41901ada0001c7745ab479903b1ac Mon Sep 17 00:00:00 2001
From: Radoslav Dimitrov <29573973+Radko-D@users.noreply.github.com>
Date: Fri, 4 Sep 2026 08:10:48 +0300
Subject: [PATCH 07/15] chore: add trailing newlines to the diagram data files
The four diagram JSON files were saved without final newlines, so
pre-commit's end-of-file-fixer rewrote them on every full run and left
the tree dirty. Add the newlines once so the hook chain stays clean.
---
docs/diagrams/extract-trust.dataflow.json | 2 +-
docs/diagrams/password-flow.sequence.json | 2 +-
docs/diagrams/pdf-ops.architecture.json | 2 +-
docs/diagrams/retry-machine.lifecycle.json | 2 +-
4 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/docs/diagrams/extract-trust.dataflow.json b/docs/diagrams/extract-trust.dataflow.json
index 22b281f..772ea4d 100644
--- a/docs/diagrams/extract-trust.dataflow.json
+++ b/docs/diagrams/extract-trust.dataflow.json
@@ -213,4 +213,4 @@
]
}
]
-}
\ No newline at end of file
+}
diff --git a/docs/diagrams/password-flow.sequence.json b/docs/diagrams/password-flow.sequence.json
index 7d8a208..ca1b215 100644
--- a/docs/diagrams/password-flow.sequence.json
+++ b/docs/diagrams/password-flow.sequence.json
@@ -223,4 +223,4 @@
]
}
]
-}
\ No newline at end of file
+}
diff --git a/docs/diagrams/pdf-ops.architecture.json b/docs/diagrams/pdf-ops.architecture.json
index 0f15fd3..ef7f196 100644
--- a/docs/diagrams/pdf-ops.architecture.json
+++ b/docs/diagrams/pdf-ops.architecture.json
@@ -408,4 +408,4 @@
]
}
]
-}
\ No newline at end of file
+}
diff --git a/docs/diagrams/retry-machine.lifecycle.json b/docs/diagrams/retry-machine.lifecycle.json
index ffec118..7b965eb 100644
--- a/docs/diagrams/retry-machine.lifecycle.json
+++ b/docs/diagrams/retry-machine.lifecycle.json
@@ -252,4 +252,4 @@
]
}
]
-}
\ No newline at end of file
+}
From 8b0ebd37337e6d4e68f8c186f129aa7f1946d84e Mon Sep 17 00:00:00 2001
From: Radoslav Dimitrov <29573973+Radko-D@users.noreply.github.com>
Date: Fri, 4 Sep 2026 08:14:21 +0300
Subject: [PATCH 08/15] chore: run hooks, CI and every script through one
toolchain
The pre-commit hooks pinned their own ruff and pyright releases, which
had drifted behind uv.lock; the Python tools now run as local hooks
through uv run --locked, so pre-commit, CI and a developer's shell all
use the single pinned version. CI gets a read-only token, per-ref
concurrency, job timeouts, SHA-pinned actions and uv sync --locked.
The bare "scripts" exclude in the ruff config silently covered both
scripts/ and docs/scripts/, leaving the decision validator that CI runs
on every push unlinted; both directories join the ruff and pyright
gates, and the nine findings from the widened rule set (SIM, PTH, PIE,
RET, PERF, FURB, N; ASYNC dropped - there is no async code) are fixed
in place. pyright now reports ignore comments that suppress nothing;
four dead ones removed. Dependabot watches the three pinned surfaces
weekly: the uv lockfile, the actions, the Docker digests.
Recorded as D-026.
---
.github/dependabot.yml | 19 +++++++
.github/workflows/ci.yml | 25 +++++++---
.pre-commit-config.yaml | 28 ++++++++---
docs/DECISIONS.md | 19 +++++--
docs/DESIGN_NOTES.md | 32 ++++++++++++
docs/scripts/_standard_parser.py | 8 +--
docs/scripts/validate_decisions.py | 77 +++++++++--------------------
pyproject.toml | 11 +++--
src/pdf_ops/engine_pikepdf.py | 3 +-
src/pdf_ops/logging_setup.py | 4 +-
src/pdf_ops/output.py | 8 ++-
tests/unit/test_encryption_label.py | 2 +-
tests/unit/test_logging.py | 2 +-
13 files changed, 150 insertions(+), 88 deletions(-)
create mode 100644 .github/dependabot.yml
diff --git a/.github/dependabot.yml b/.github/dependabot.yml
new file mode 100644
index 0000000..8615d4e
--- /dev/null
+++ b/.github/dependabot.yml
@@ -0,0 +1,19 @@
+# Keeps the three pinned surfaces current: the uv lockfile, the SHA-pinned
+# actions in ci.yml, and the digest-pinned images in the Dockerfile.
+version: 2
+updates:
+ - package-ecosystem: uv
+ directory: /
+ schedule:
+ interval: weekly
+ groups:
+ dev-tooling:
+ patterns: [ruff, pyright, pytest, pre-commit]
+ - package-ecosystem: github-actions
+ directory: /
+ schedule:
+ interval: weekly
+ - package-ecosystem: docker
+ directory: /
+ schedule:
+ interval: weekly
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
index d96ba79..df88bbe 100644
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@@ -5,16 +5,26 @@ on:
branches: [main]
pull_request:
+# Least privilege: nothing here writes to the repository.
+permissions:
+ contents: read
+
+# A newer push to the same branch or PR supersedes the run in flight.
+concurrency:
+ group: ${{ github.workflow }}-${{ github.ref }}
+ cancel-in-progress: true
+
jobs:
quality:
runs-on: ubuntu-latest
+ timeout-minutes: 15
steps:
- - uses: actions/checkout@v4
- - uses: astral-sh/setup-uv@v6
+ - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
+ - uses: astral-sh/setup-uv@20cfd1bf945f4377ade1205e4dbc17946fc9a30d # v10.0.1
with:
enable-cache: true
- name: Sync dependencies
- run: uv sync --frozen
+ run: uv sync --locked
- name: Lint
run: uv run ruff check .
- name: Format check
@@ -24,16 +34,17 @@ jobs:
- name: Tests
run: uv run pytest
- name: Validate decision register
- run: python3 docs/scripts/validate_decisions.py docs/DECISIONS.md
+ run: uv run python docs/scripts/validate_decisions.py docs/DECISIONS.md
docker:
runs-on: ubuntu-latest
+ timeout-minutes: 20
steps:
- - uses: actions/checkout@v4
- - uses: astral-sh/setup-uv@v6
+ - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
+ - uses: astral-sh/setup-uv@20cfd1bf945f4377ade1205e4dbc17946fc9a30d # v10.0.1
with:
enable-cache: true
- name: Sync dependencies
- run: uv sync --frozen
+ run: uv sync --locked
- name: Container contract tests (builds the image)
run: uv run pytest -m container
diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
index edec9bc..72bbcba 100644
--- a/.pre-commit-config.yaml
+++ b/.pre-commit-config.yaml
@@ -1,6 +1,9 @@
+# The Python tools run through uv so that pre-commit, CI and a developer's
+# shell all use the single version pinned in uv.lock. Only the generic
+# hygiene hooks come from a remote repo.
repos:
- repo: https://github.com/pre-commit/pre-commit-hooks
- rev: v5.0.0
+ rev: v6.0.0
hooks:
- id: check-yaml
- id: check-toml
@@ -8,14 +11,23 @@ repos:
- id: end-of-file-fixer
- id: check-added-large-files
- - repo: https://github.com/astral-sh/ruff-pre-commit
- rev: v0.14.9
+ - repo: local
hooks:
- id: ruff-check
- args: ["--fix"]
+ name: ruff check
+ entry: uv run --locked ruff check --fix --force-exclude
+ language: system
+ types_or: [python, pyi]
+ require_serial: true
- id: ruff-format
-
- - repo: https://github.com/RobertCraigie/pyright-python
- rev: v1.1.408
- hooks:
+ name: ruff format
+ entry: uv run --locked ruff format --force-exclude
+ language: system
+ types_or: [python, pyi]
+ require_serial: true
- id: pyright
+ name: pyright
+ entry: uv run --locked pyright
+ language: system
+ types_or: [python, pyi]
+ pass_filenames: false
diff --git a/docs/DECISIONS.md b/docs/DECISIONS.md
index 23af109..e93a713 100644
--- a/docs/DECISIONS.md
+++ b/docs/DECISIONS.md
@@ -2,7 +2,7 @@
> **Project:** Containerized PDF operations (merge, extract attachments) for workflow systems
> **Started:** 2026-08-31
-> **Last updated:** 2026-09-03
+> **Last updated:** 2026-09-04
This document is the authoritative register of all architectural decisions for this project. New decisions are appended with the next available `D-###`. See [DECISION_TRACKING_STANDARD.md](DECISION_TRACKING_STANDARD.md) for format, vocabularies, and workflow. CI validation: [`scripts/validate_decisions.py`](scripts/validate_decisions.py).
@@ -37,8 +37,9 @@ This document is the authoritative register of all architectural decisions for t
| [`D-023`](#D-023) | ๐ข | pdf-engine | Engine swapped to pikepdf; pypdf demoted to dev-dependency test oracle | 2026-09-02 | engine_pikepdf.py (qpdf-backed) replaces engine_pypdf.py as the runtime engine, executing the D-002 plan: better large-file and corrupt-input behavior, native AES-256 (R=6 pinned). password_type user/owner/empty reporting survives via qpdf's password-matched flags; qpdf repairs light damage pypdf refused (pinned as behavior; warnings surface as events - at open via OpenedInput.warnings, after lazy reads/writes via collect_warnings); duplicate attachment names preserved by walking /Names/EmbeddedFiles directly with cycle and type guards (hostile shapes degrade or classify as data problems, never exit 1); the merged output is saved through the atomic layer's open temp file, keeping the single-temp cleanup contract; failed-open algorithm labels come from a best-effort raw /Encrypt scan. pypdf stays in the dev group building fixtures and verifying outputs - every test is a cross-library check. | [`DESIGN_NOTES.md section 1`](DESIGN_NOTES.md) | - |
| [`D-024`](#D-024) | ๐ข | security | Secrets stay stdlib, consolidated into one module | 2026-09-03 | All secret handling (Secret wrapper, EnvSecret/FileSecret source refs with resolve()/describe(), Secrets bundle, scrub registration) consolidated into secrets.py; config.py only parses which source is configured. Shelf options evaluated and rejected: pydantic SecretStr (compiled dependency for one masked-repr class), pydantic-settings (config-layer rewrite; secrets_dir expects field-named files in a fixed directory - a different contract from PDFOPS_PASSWORD_FILE=; ValidationError would need retranslation into the error_code taxonomy), scanner-style log redactors (pattern heuristics, weaker than the exact-value field-restricted scrub that avoids the password-oracle problem). | [`DESIGN_NOTES.md section 8`](DESIGN_NOTES.md) | - |
| [`D-025`](#D-025) | ๐ข | container | Hardened runtime image: digest-pinned multi-stage build, read-only rootfs | 2026-09-03 | The image is a two-stage build: a digest-pinned uv stage resolves the lockfile into a self-contained virtualenv with compiled bytecode; the runtime stage (digest-pinned python:3.14-slim) carries only that venv, uninstalls the base image's pip, removes stdlib ensurepip (its bundled wheel would restore pip in one command), and runs as fixed non-root UID 10001. Read-only root filesystem is proven by a container test running the golden merge under --read-only --cap-drop ALL --security-opt no-new-privileges (all writes land in the output mount by design). deploy/argo-example.yaml ships the full posture incl. fsGroup, secret-mounted password, a retry expression covering exit 1 plus pod-level Error nodes (which carry no exit code), and memory sized by the measured input+128MB rule. Distroless bases considered and not taken: pinned slim minus pip reaches most of the value while staying debuggable. | [`DESIGN_NOTES.md section 11`](DESIGN_NOTES.md) | - |
+| [`D-026`](#D-026) | ๐ข | infra | One uv-locked toolchain with SHA-pinned, Dependabot-watched CI | 2026-09-04 | pre-commit runs ruff and pyright as local hooks through uv run --locked, so hooks, CI and a developer's shell all resolve the single version pinned in uv.lock (the remote-hook revs had drifted behind the lock). CI runs with a read-only token, per-ref concurrency, job timeouts and actions pinned to full commit SHAs; uv sync --locked replaces --frozen. scripts/ and docs/scripts/ join the ruff and pyright gates - the bare 'scripts' exclude had silently covered both, leaving the CI-run decision validator unlinted; the widened rule set (SIM, PTH, PIE, RET, PERF, FURB, N; ASYNC dropped, no async code exists) surfaced nine findings, fixed in place, and pyright now reports ignore comments that suppress nothing. Dependabot watches the three pinned surfaces weekly: the uv lockfile, the actions, the Docker digests. | [`DESIGN_NOTES.md section 12`](DESIGN_NOTES.md) | - |
-**Counts:** 25 total decisions - 23 ๐ข decided, 0 ๐ก pending, 0 โธ deferred, 2 ๐ต superseded.
+**Counts:** 26 total decisions - 23 ๐ข decided, 0 ๐ก pending, 0 โธ deferred, 2 ๐ต superseded.
### Index by area
@@ -54,7 +55,8 @@ This document is the authoritative register of all architectural decisions for t
| observability | 1 | D-005 |
| container | 1 | D-025 |
| project | 1 | D-001 |
-| **Total** | **25** | |
+| infra | 1 | D-026 |
+| **Total** | **26** | |
### Open decisions (๐ก Pending + โธ Deferred)
@@ -361,6 +363,17 @@ Per-decision details: status, decided date, rationale, related decisions, and th
- **Reversibility:** cheap
- **Where:** [`DESIGN_NOTES.md section 11`](DESIGN_NOTES.md)
+
+### D-026
+- **Title:** One uv-locked toolchain with SHA-pinned, Dependabot-watched CI
+- **Status:** ๐ข Decided
+- **Area:** infra
+- **Decided on:** 2026-09-04
+- **Summary:** pre-commit runs ruff and pyright as local hooks through uv run --locked, so hooks, CI and a developer's shell all resolve the single version pinned in uv.lock (the remote-hook revs had drifted behind the lock). CI runs with a read-only token, per-ref concurrency, job timeouts and actions pinned to full commit SHAs; uv sync --locked replaces --frozen. scripts/ and docs/scripts/ join the ruff and pyright gates - the bare 'scripts' exclude had silently covered both, leaving the CI-run decision validator unlinted; the widened rule set (SIM, PTH, PIE, RET, PERF, FURB, N; ASYNC dropped, no async code exists) surfaced nine findings, fixed in place, and pyright now reports ignore comments that suppress nothing. Dependabot watches the three pinned surfaces weekly: the uv lockfile, the actions, the Docker digests.
+- **Risk:** low
+- **Reversibility:** cheap
+- **Where:** [`DESIGN_NOTES.md section 12`](DESIGN_NOTES.md)
+
---
## Architectural Decision Records (Full Analysis)
diff --git a/docs/DESIGN_NOTES.md b/docs/DESIGN_NOTES.md
index 7073d6f..5e2b32a 100644
--- a/docs/DESIGN_NOTES.md
+++ b/docs/DESIGN_NOTES.md
@@ -427,3 +427,35 @@ Considered and not taken: distroless/static bases (the pinned slim base with
pip removed reaches most of the value while keeping a debuggable Python
layout); image signing/SBOM (registry- and org-specific, noted as release
engineering rather than image structure).
+
+## 12. Toolchain and CI pinning (per [D-026](DECISIONS.md#D-026))
+
+The repo had three places that could each pick their own tool versions: the
+pre-commit hook pins, uv.lock, and whatever a developer's shell resolved.
+They had already drifted - the hooks pinned older ruff and pyright releases
+than the lockfile. The fix is structural rather than a version bump: the
+Python tools run as local pre-commit hooks through `uv run --locked`, so
+hooks, CI and a developer's shell all execute the single version pinned in
+uv.lock. Only the generic hygiene hooks (whitespace, YAML/TOML syntax,
+large files) still come from a remote hook repo.
+
+Two related gaps closed in the same pass:
+
+- **The gates now cover every script.** The bare `scripts` entry in the
+ ruff exclude list silently matched both `scripts/` and `docs/scripts/`,
+ leaving the decision-register validator - which CI executes on every
+ push - unlinted and untyped. Both directories joined the ruff and pyright
+ gates; the widened rule set (SIM, PTH, PIE, RET, PERF, FURB, N added;
+ ASYNC dropped because no async code exists) surfaced nine findings, all
+ fixed with behavior-preserving edits. pyright's
+ `reportUnnecessaryTypeIgnoreComment` keeps ignore comments honest; four
+ that suppressed nothing were removed.
+- **CI itself is pinned and least-privilege.** Actions are pinned to full
+ commit SHAs (a tag can be moved; a SHA cannot), the workflow token is
+ read-only, runs on the same ref supersede each other, and jobs carry
+ timeouts. `uv sync --locked` verifies the lockfile matches pyproject
+ instead of silently trusting it.
+
+Pinning everything creates a staleness problem, so Dependabot watches the
+three pinned surfaces weekly: the uv lockfile (dev tooling grouped into one
+PR), the action SHAs, and the Docker base-image digests.
diff --git a/docs/scripts/_standard_parser.py b/docs/scripts/_standard_parser.py
index 2ff7f9e..a1147bc 100755
--- a/docs/scripts/_standard_parser.py
+++ b/docs/scripts/_standard_parser.py
@@ -58,6 +58,7 @@
# Vocabulary parsing
# -----------------------------------------------------------------------------
+
# Standard path resolution (v0.11.27): prefer --standard-path CLI arg if
# provided, fall back to relative-to-script default. The default resolves
# correctly when this script lives at docs/scripts/ in a scaffolded project
@@ -68,12 +69,13 @@
# legacy projects whose docs/scripts/ doesn't have a copy of this script.
def _resolve_standard_path() -> Path:
import sys
+
if "--standard-path" in sys.argv:
idx = sys.argv.index("--standard-path")
if idx + 1 < len(sys.argv):
path = Path(sys.argv[idx + 1]).resolve()
# Pop the flag + value so consuming scripts' argparse doesn't reject them.
- del sys.argv[idx:idx + 2]
+ del sys.argv[idx : idx + 2]
return path
return Path(__file__).resolve().parent.parent / "DECISION_TRACKING_STANDARD.md"
@@ -134,8 +136,8 @@ def _abort(msg: str) -> None:
def _load() -> dict[str, frozenset[str]]:
if not STANDARD_PATH.exists():
_abort(
- f"DECISION_TRACKING_STANDARD.md not found - required for vocabulary parsing. "
- f"Run /init-decisions to scaffold it."
+ "DECISION_TRACKING_STANDARD.md not found - required for vocabulary parsing. "
+ "Run /init-decisions to scaffold it."
)
text = STANDARD_PATH.read_text(encoding="utf-8")
sections = _parse_vocab_sections(text)
diff --git a/docs/scripts/validate_decisions.py b/docs/scripts/validate_decisions.py
index d4169e0..879e551 100755
--- a/docs/scripts/validate_decisions.py
+++ b/docs/scripts/validate_decisions.py
@@ -36,6 +36,7 @@
import sys
from collections import Counter, defaultdict
from dataclasses import dataclass, field
+from itertools import pairwise
from pathlib import Path
from _standard_parser import (
@@ -67,9 +68,7 @@
)
GREEN_STATUS_ICON = "๐ข"
-SUMMARY_PLACEHOLDERS: frozenset[str] = frozenset(
- {"tbd", "todo", "tba", "-", " - ", "n/a"}
-)
+SUMMARY_PLACEHOLDERS: frozenset[str] = frozenset({"tbd", "todo", "tba", "-", " - ", "n/a"})
@dataclass
@@ -126,11 +125,7 @@ def parse_master_register(
if stripped == "## Master Register":
in_master = True
continue
- if (
- in_master
- and (stripped.startswith("## ") or stripped.startswith("### "))
- and stripped != "## Master Register"
- ):
+ if in_master and stripped.startswith(("## ", "### ")) and stripped != "## Master Register":
in_master = False
if not in_master:
continue
@@ -165,9 +160,7 @@ def parse_master_register(
return decisions, total_claimed
-def parse_anchor_pages(
- lines: list[str], decisions: dict[str, Decision], report: Report
-) -> None:
+def parse_anchor_pages(lines: list[str], decisions: dict[str, Decision], report: Report) -> None:
"""Populate anchor_fields on each Decision by reading its anchor page."""
i = 0
n = len(lines)
@@ -203,11 +196,7 @@ def parse_anchor_pages(
j = i + 1
while j < n:
sub = lines[j].rstrip("\n")
- if (
- ANCHOR_HEADING_RE.match(sub)
- or sub.startswith("## ")
- or sub.startswith("---")
- ):
+ if ANCHOR_HEADING_RE.match(sub) or sub.startswith(("## ", "---")):
break
fm = ANCHOR_FIELD_RE.match(sub)
if fm:
@@ -233,11 +222,11 @@ def check_sequence(decisions: dict[str, Decision], report: Report) -> None:
return
if nums[0] != 1:
report.err(f"ID sequence starts at D-{nums[0]:03d}, expected D-001")
- for prev, curr in zip(nums, nums[1:]):
+ for prev, curr in pairwise(nums):
if curr != prev + 1:
report.err(
f"ID sequence gap: D-{prev:03d} -> D-{curr:03d} "
- f"(missing D-{prev+1:03d}..D-{curr-1:03d})"
+ f"(missing D-{prev + 1:03d}..D-{curr - 1:03d})"
)
@@ -258,14 +247,11 @@ def check_areas(decisions: dict[str, Decision], report: Report) -> None:
for did, d in sorted(decisions.items()):
if d.area not in AREAS:
report.err(
- f"{did}: area '{d.area}' not in standard's taxonomy "
- f"({', '.join(sorted(AREAS))})"
+ f"{did}: area '{d.area}' not in standard's taxonomy ({', '.join(sorted(AREAS))})"
)
anchor_area = d.anchor_fields.get("Area", "").strip()
if anchor_area and anchor_area != d.area:
- report.err(
- f"{did}: area mismatch - master='{d.area}' anchor='{anchor_area}'"
- )
+ report.err(f"{did}: area mismatch - master='{d.area}' anchor='{anchor_area}'")
def check_vocabularies(decisions: dict[str, Decision], report: Report) -> None:
@@ -311,9 +297,7 @@ def check_vocabularies(decisions: dict[str, Decision], report: Report) -> None:
)
-def check_index_counts(
- lines: list[str], decisions: dict[str, Decision], report: Report
-) -> None:
+def check_index_counts(lines: list[str], decisions: dict[str, Decision], report: Report) -> None:
"""Verify the 'Index by area' table agrees with the master register."""
actual_counts: Counter[str] = Counter(d.area for d in decisions.values())
actual_ids: dict[str, list[str]] = defaultdict(list)
@@ -353,20 +337,18 @@ def check_index_counts(
if count != len(decisions):
report.err(
f"Index by area: Total count {count} != actual {len(decisions)} "
- f"(line {i+1})"
+ f"(line {i + 1})"
)
else:
claimed_areas.add(area)
if area not in AREAS:
- report.err(
- f"Index by area: unknown area '{area}' at line {i+1}"
- )
+ report.err(f"Index by area: unknown area '{area}' at line {i + 1}")
i += 1
continue
if count != actual_counts[area]:
report.err(
f"Index by area: area '{area}' count {count} != actual "
- f"{actual_counts[area]} at line {i+1}"
+ f"{actual_counts[area]} at line {i + 1}"
)
listed = sorted(set(ID_RE.findall(ids_cell)))
actual_set = sorted({i.split("-")[1] for i in actual_ids[area]})
@@ -375,11 +357,11 @@ def check_index_counts(
extra = sorted(set(listed) - set(actual_set))
parts = []
if missing:
- parts.append(f"missing {', '.join('D-'+m for m in missing)}")
+ parts.append(f"missing {', '.join('D-' + m for m in missing)}")
if extra:
- parts.append(f"extra {', '.join('D-'+e for e in extra)}")
+ parts.append(f"extra {', '.join('D-' + e for e in extra)}")
report.err(
- f"Index by area: area '{area}' ID list drift at line {i+1} - "
+ f"Index by area: area '{area}' ID list drift at line {i + 1} - "
+ "; ".join(parts)
)
i += 1
@@ -390,8 +372,7 @@ def check_index_counts(
for area in AREAS:
if actual_counts[area] and area not in claimed_areas:
report.err(
- f"Index by area: area '{area}' has {actual_counts[area]} "
- f"decisions but no row"
+ f"Index by area: area '{area}' has {actual_counts[area]} decisions but no row"
)
@@ -410,9 +391,7 @@ def check_related_ids(decisions: dict[str, Decision], report: Report) -> None:
)
-def check_doc_links(
- decisions: dict[str, Decision], docs_root: Path, report: Report
-) -> None:
+def check_doc_links(decisions: dict[str, Decision], docs_root: Path, report: Report) -> None:
"""Every `discussion` / `Where` link target (relative path) must exist in docs/."""
for did, d in sorted(decisions.items()):
candidates: list[tuple[str, str]] = []
@@ -431,10 +410,7 @@ def check_doc_links(
continue
resolved = (docs_root / path_part).resolve()
if not resolved.exists():
- report.err(
- f"{did}: {label} link '{path_part}' -> {resolved} "
- f"does not exist"
- )
+ report.err(f"{did}: {label} link '{path_part}' -> {resolved} does not exist")
def check_green_summaries(decisions: dict[str, Decision], report: Report) -> None:
@@ -457,9 +433,7 @@ def check_green_summaries(decisions: dict[str, Decision], report: Report) -> Non
)
-def check_adr_references(
- lines: list[str], decisions: dict[str, Decision], report: Report
-) -> None:
+def check_adr_references(lines: list[str], decisions: dict[str, Decision], report: Report) -> None:
"""Every ``ADR-###`` link in the master register's "ADR" column must
resolve to an ``## ADR-###:`` heading in the same document."""
existing_adrs: set[str] = set()
@@ -497,7 +471,8 @@ def check_decide_under_assumption_cross_links(
DECISION_TRACKING_STANDARD.md for the rationale.
"""
assumed = [
- d for d in decisions.values()
+ d
+ for d in decisions.values()
if "assumed" in d.anchor_fields.get("Status", d.status).lower()
]
if not assumed:
@@ -557,13 +532,9 @@ def main(argv: list[str]) -> int:
check_decide_under_assumption_cross_links(decisions, docs_root, report)
if total_claimed is not None and total_claimed != len(decisions):
- report.err(
- f"Counts line claims {total_claimed} total decisions, actual = {len(decisions)}"
- )
+ report.err(f"Counts line claims {total_claimed} total decisions, actual = {len(decisions)}")
- rel_path = (
- path.relative_to(Path.cwd()) if path.is_relative_to(Path.cwd()) else path
- )
+ rel_path = path.relative_to(Path.cwd()) if path.is_relative_to(Path.cwd()) else path
print(f"Parsed {len(decisions)} decisions from {rel_path}")
if report.warnings:
print(f"\n{len(report.warnings)} warning(s):")
diff --git a/pyproject.toml b/pyproject.toml
index 86f397f..36d653c 100644
--- a/pyproject.toml
+++ b/pyproject.toml
@@ -19,10 +19,13 @@ pdf-ops = "pdf_ops.__main__:main"
[tool.ruff]
line-length = 100
target-version = "py314"
-exclude = [".venv", "__pycache__", "build", "dist", "scripts"]
+exclude = [".venv", "__pycache__", "build", "dist"]
[tool.ruff.lint]
-select = ["E", "W", "F", "I", "UP", "B", "C4", "ARG", "ASYNC", "RUF"]
+select = [
+ "E", "W", "F", "I", "UP", "B", "C4", "ARG", "RUF",
+ "SIM", "PTH", "PIE", "RET", "PERF", "FURB", "N",
+]
ignore = ["E501"]
[tool.ruff.lint.per-file-ignores]
@@ -39,7 +42,7 @@ known-first-party = ["pdf_ops"]
pythonVersion = "3.14"
venvPath = "."
venv = ".venv"
-include = ["src", "tests"]
+include = ["src", "tests", "scripts", "docs/scripts"]
exclude = [".venv", "__pycache__", "build", "dist"]
typeCheckingMode = "strict"
reportMissingTypeStubs = false
@@ -49,6 +52,8 @@ reportUnknownVariableType = false
reportUnknownParameterType = false
reportUnusedImport = false
reportPrivateUsage = false
+# An ignore comment that suppresses nothing is drift - keep them honest.
+reportUnnecessaryTypeIgnoreComment = true
[tool.pytest.ini_options]
testpaths = ["tests"]
diff --git a/src/pdf_ops/engine_pikepdf.py b/src/pdf_ops/engine_pikepdf.py
index ecadeb1..4ace556 100644
--- a/src/pdf_ops/engine_pikepdf.py
+++ b/src/pdf_ops/engine_pikepdf.py
@@ -245,8 +245,7 @@ def _walk_name_tree(node: Any, out: list[tuple[Any, Any]], seen: set[tuple[int,
_walk_name_tree(kid, out, seen)
names: Any = node.get("/Names")
if isinstance(names, pikepdf.Array):
- for index in range(0, len(names) - 1, 2):
- out.append((names[index], names[index + 1]))
+ out.extend((names[index], names[index + 1]) for index in range(0, len(names) - 1, 2))
def _describe_encryption(pdf: pikepdf.Pdf) -> str:
diff --git a/src/pdf_ops/logging_setup.py b/src/pdf_ops/logging_setup.py
index 32536be..3314d19 100644
--- a/src/pdf_ops/logging_setup.py
+++ b/src/pdf_ops/logging_setup.py
@@ -70,9 +70,9 @@ def _scrub(value: Any) -> Any:
value = value.replace(secret, "***")
return value
if isinstance(value, dict):
- return {key: _scrub(item) for key, item in value.items()} # pyright: ignore[reportUnknownVariableType]
+ return {key: _scrub(item) for key, item in value.items()}
if isinstance(value, (list, tuple)):
- return [_scrub(item) for item in value] # pyright: ignore[reportUnknownVariableType]
+ return [_scrub(item) for item in value]
return value
diff --git a/src/pdf_ops/output.py b/src/pdf_ops/output.py
index 195b902..33c7a41 100644
--- a/src/pdf_ops/output.py
+++ b/src/pdf_ops/output.py
@@ -13,7 +13,7 @@
import os
import tempfile
from collections.abc import Generator
-from contextlib import contextmanager
+from contextlib import contextmanager, suppress
from pathlib import Path
from typing import NoReturn
@@ -128,7 +128,7 @@ def atomic_output(path: Path) -> Generator[Path]:
yield tmp_path
with tmp_path.open("rb") as handle:
os.fsync(handle.fileno())
- os.replace(tmp_path, path)
+ tmp_path.replace(path)
_fsync_dir(path.parent)
except OSError as err:
_cleanup(tmp_path)
@@ -167,10 +167,8 @@ def _translate_os_error(err: OSError, path: Path) -> Exception:
def _cleanup(tmp_path: Path) -> None:
- try:
+ with suppress(OSError): # best effort - never mask the original failure
tmp_path.unlink(missing_ok=True)
- except OSError: # best effort - never mask the original failure
- pass
def _fsync_dir(directory: Path) -> None:
diff --git a/tests/unit/test_encryption_label.py b/tests/unit/test_encryption_label.py
index 8bf591b..4930881 100644
--- a/tests/unit/test_encryption_label.py
+++ b/tests/unit/test_encryption_label.py
@@ -9,7 +9,7 @@
import pytest
from pdf_ops.engine_pikepdf import (
- _describe_encryption_raw, # pyright: ignore[reportPrivateUsage]
+ _describe_encryption_raw,
)
pytestmark = pytest.mark.unit
diff --git a/tests/unit/test_logging.py b/tests/unit/test_logging.py
index 1aa2291..b0d11d8 100644
--- a/tests/unit/test_logging.py
+++ b/tests/unit/test_logging.py
@@ -9,7 +9,7 @@
from pdf_ops.logging_setup import (
JsonFormatter,
- _ThirdPartyEventFilter, # pyright: ignore[reportPrivateUsage]
+ _ThirdPartyEventFilter,
emit_terminal,
setup_logging,
)
From fcf71ad89deba8587e21861c00501b2ad674f11c Mon Sep 17 00:00:00 2001
From: Radoslav Dimitrov <29573973+Radko-D@users.noreply.github.com>
Date: Fri, 4 Sep 2026 08:15:36 +0300
Subject: [PATCH 09/15] refactor: move input validation into its own module
extract.py imported validate_inputs from merge.py - the one import edge
between two modules the design doc presents as parallel peers. The
validation, the magic-bytes probe and the problem classification set
move byte-for-byte into inputs.py; both operations now depend on the
shared module instead of one depending on the other. Error codes, the
collect-all contract and the context.problems log shape are unchanged.
Recorded as D-027.
---
docs/DECISIONS.md | 18 ++++++++++---
docs/DESIGN.md | 1 +
docs/DESIGN_NOTES.md | 9 ++++++-
src/pdf_ops/extract.py | 2 +-
src/pdf_ops/inputs.py | 61 ++++++++++++++++++++++++++++++++++++++++++
src/pdf_ops/merge.py | 54 +++----------------------------------
6 files changed, 89 insertions(+), 56 deletions(-)
create mode 100644 src/pdf_ops/inputs.py
diff --git a/docs/DECISIONS.md b/docs/DECISIONS.md
index e93a713..8329041 100644
--- a/docs/DECISIONS.md
+++ b/docs/DECISIONS.md
@@ -38,8 +38,9 @@ This document is the authoritative register of all architectural decisions for t
| [`D-024`](#D-024) | ๐ข | security | Secrets stay stdlib, consolidated into one module | 2026-09-03 | All secret handling (Secret wrapper, EnvSecret/FileSecret source refs with resolve()/describe(), Secrets bundle, scrub registration) consolidated into secrets.py; config.py only parses which source is configured. Shelf options evaluated and rejected: pydantic SecretStr (compiled dependency for one masked-repr class), pydantic-settings (config-layer rewrite; secrets_dir expects field-named files in a fixed directory - a different contract from PDFOPS_PASSWORD_FILE=; ValidationError would need retranslation into the error_code taxonomy), scanner-style log redactors (pattern heuristics, weaker than the exact-value field-restricted scrub that avoids the password-oracle problem). | [`DESIGN_NOTES.md section 8`](DESIGN_NOTES.md) | - |
| [`D-025`](#D-025) | ๐ข | container | Hardened runtime image: digest-pinned multi-stage build, read-only rootfs | 2026-09-03 | The image is a two-stage build: a digest-pinned uv stage resolves the lockfile into a self-contained virtualenv with compiled bytecode; the runtime stage (digest-pinned python:3.14-slim) carries only that venv, uninstalls the base image's pip, removes stdlib ensurepip (its bundled wheel would restore pip in one command), and runs as fixed non-root UID 10001. Read-only root filesystem is proven by a container test running the golden merge under --read-only --cap-drop ALL --security-opt no-new-privileges (all writes land in the output mount by design). deploy/argo-example.yaml ships the full posture incl. fsGroup, secret-mounted password, a retry expression covering exit 1 plus pod-level Error nodes (which carry no exit code), and memory sized by the measured input+128MB rule. Distroless bases considered and not taken: pinned slim minus pip reaches most of the value while staying debuggable. | [`DESIGN_NOTES.md section 11`](DESIGN_NOTES.md) | - |
| [`D-026`](#D-026) | ๐ข | infra | One uv-locked toolchain with SHA-pinned, Dependabot-watched CI | 2026-09-04 | pre-commit runs ruff and pyright as local hooks through uv run --locked, so hooks, CI and a developer's shell all resolve the single version pinned in uv.lock (the remote-hook revs had drifted behind the lock). CI runs with a read-only token, per-ref concurrency, job timeouts and actions pinned to full commit SHAs; uv sync --locked replaces --frozen. scripts/ and docs/scripts/ join the ruff and pyright gates - the bare 'scripts' exclude had silently covered both, leaving the CI-run decision validator unlinted; the widened rule set (SIM, PTH, PIE, RET, PERF, FURB, N; ASYNC dropped, no async code exists) surfaced nine findings, fixed in place, and pyright now reports ignore comments that suppress nothing. Dependabot watches the three pinned surfaces weekly: the uv lockfile, the actions, the Docker digests. | [`DESIGN_NOTES.md section 12`](DESIGN_NOTES.md) | - |
+| [`D-027`](#D-027) | ๐ข | error-handling | Input validation extracted into its own module | 2026-09-04 | validate_inputs, the magic-bytes probe and the input-problem classification set moved byte-for-byte from merge.py into inputs.py; extract.py imports from inputs instead of reaching into merge. This removes the only import edge between the two operation modules, which the design doc presents as parallel peers, and gives the input-problem vocabulary a single home. Pure code motion: error codes, the one-failure-reports-all contract (D-012) and the context.problems log shape are unchanged. | [`DESIGN_NOTES.md section 6`](DESIGN_NOTES.md) | - |
-**Counts:** 26 total decisions - 23 ๐ข decided, 0 ๐ก pending, 0 โธ deferred, 2 ๐ต superseded.
+**Counts:** 27 total decisions - 23 ๐ข decided, 0 ๐ก pending, 0 โธ deferred, 2 ๐ต superseded.
### Index by area
@@ -47,7 +48,7 @@ This document is the authoritative register of all architectural decisions for t
| Area | Count | IDs |
|---|---|---|
-| error-handling | 5 | D-003, D-006, D-012, D-015, D-020 |
+| error-handling | 6 | D-003, D-006, D-012, D-015, D-020, D-027 |
| config | 4 | D-004, D-007, D-008, D-009 |
| pdf-engine | 5 | D-002, D-011, D-016, D-018, D-023 |
| security | 5 | D-013, D-014, D-017, D-019, D-024 |
@@ -56,7 +57,7 @@ This document is the authoritative register of all architectural decisions for t
| container | 1 | D-025 |
| project | 1 | D-001 |
| infra | 1 | D-026 |
-| **Total** | **26** | |
+| **Total** | **27** | |
### Open decisions (๐ก Pending + โธ Deferred)
@@ -374,6 +375,17 @@ Per-decision details: status, decided date, rationale, related decisions, and th
- **Reversibility:** cheap
- **Where:** [`DESIGN_NOTES.md section 12`](DESIGN_NOTES.md)
+
+### D-027
+- **Title:** Input validation extracted into its own module
+- **Status:** ๐ข Decided
+- **Area:** error-handling
+- **Decided on:** 2026-09-04
+- **Summary:** validate_inputs, the magic-bytes probe and the input-problem classification set moved byte-for-byte from merge.py into inputs.py; extract.py imports from inputs instead of reaching into merge. This removes the only import edge between the two operation modules, which the design doc presents as parallel peers, and gives the input-problem vocabulary a single home. Pure code motion: error codes, the one-failure-reports-all contract (D-012) and the context.problems log shape are unchanged.
+- **Risk:** low
+- **Reversibility:** cheap
+- **Where:** [`DESIGN_NOTES.md section 6`](DESIGN_NOTES.md)
+
---
## Architectural Decision Records (Full Analysis)
diff --git a/docs/DESIGN.md b/docs/DESIGN.md
index 6d36c26..744d9d9 100644
--- a/docs/DESIGN.md
+++ b/docs/DESIGN.md
@@ -34,6 +34,7 @@ Small modules with one-way dependencies:
| `main.py` | `run(env) -> int` - the single error boundary; emits the one terminal event |
| `engine.py` | the `PdfEngine` Protocol - the library swap seam |
| `engine_pikepdf.py` | the **only** module importing pikepdf; translates qpdf's failure modes into the taxonomy |
+| `inputs.py` | up-front input validation shared by both operations; every bad input reported in one failure |
| `merge.py` / `extract.py` | orchestration: validate everything, then write |
| `output.py` | atomic writes, existing-output policy, stale-temp cleanup |
| `secrets.py` | the whole secret lifecycle: `Secret` wrapper, source refs, resolution, scrub registration |
diff --git a/docs/DESIGN_NOTES.md b/docs/DESIGN_NOTES.md
index 5e2b32a..bd248c3 100644
--- a/docs/DESIGN_NOTES.md
+++ b/docs/DESIGN_NOTES.md
@@ -172,7 +172,7 @@ copies `/Names/EmbeddedFiles` on a page-level merge, so attachments in merge inp
silently dropped - options (detect-and-warn, fail-loud flag, qpdf's
`--copy-attachments-from`) are evaluated when attachment handling is built out.
-## 6. Input validation and atomic output (per [D-010](DECISIONS.md#D-010), [D-012](DECISIONS.md#D-012))
+## 6. Input validation and atomic output (per [D-010](DECISIONS.md#D-010), [D-012](DECISIONS.md#D-012), [D-027](DECISIONS.md#D-027))
**Collect-all validation ([D-012](DECISIONS.md#D-012)):** every input is checked up front
(exists, is a file, readable, starts with `%PDF-`) and *all* problems are reported in one
@@ -180,6 +180,13 @@ failure event - an operator fixing a broken workflow learns about every bad inpu
single run, not one per retry. The exit class follows the first problem in input order
(deterministic); the full list travels in `context.problems`.
+**One home for the check ([D-027](DECISIONS.md#D-027)):** the validation originally
+lived in `merge.py` with `extract.py` importing it from there - the only import edge
+between two modules the design doc presents as parallel peers. It moved byte-for-byte
+into `inputs.py`, so both operations depend on the shared module instead of one
+depending on the other. Pure code motion: error codes, the collect-all contract and
+the `context.problems` shape are unchanged.
+
**Atomic output ([D-010](DECISIONS.md#D-010)):** all output is written to a temp file
created *in the destination directory* - same filesystem, because `os.replace` is only
atomic within one - then fsynced and renamed over the final path in a single step (plus a
diff --git a/src/pdf_ops/extract.py b/src/pdf_ops/extract.py
index 4c2b27a..57045ae 100644
--- a/src/pdf_ops/extract.py
+++ b/src/pdf_ops/extract.py
@@ -15,7 +15,7 @@
from pdf_ops.config import ExtractConfig, OnExists
from pdf_ops.engine import Attachment, get_engine
from pdf_ops.errors import InputError, OutputError
-from pdf_ops.merge import validate_inputs
+from pdf_ops.inputs import validate_inputs
from pdf_ops.output import atomic_output, check_output_dir, clean_stale_temps
from pdf_ops.secrets import Secrets
diff --git a/src/pdf_ops/inputs.py b/src/pdf_ops/inputs.py
new file mode 100644
index 0000000..0f7fcfa
--- /dev/null
+++ b/src/pdf_ops/inputs.py
@@ -0,0 +1,61 @@
+"""Up-front input validation shared by every operation.
+
+Both operations check their inputs before anything is written; the probe
+and the problem classification live here so merge and extract cannot
+drift apart.
+"""
+
+from __future__ import annotations
+
+from collections.abc import Sequence
+from pathlib import Path
+
+from pdf_ops.errors import InputError, InvalidPdfError, PdfOpsError
+
+PDF_MAGIC = b"%PDF-"
+
+# Problem kinds found during input validation, in exit-code class order.
+_INPUT_PROBLEMS = frozenset({"INPUT_MISSING", "INPUT_IS_DIRECTORY", "INPUT_UNREADABLE"})
+
+
+def validate_inputs(inputs: Sequence[Path]) -> None:
+ """Check every input up front and report all problems in one failure.
+
+ An operator fixing a broken workflow should learn about every bad input
+ from a single run, not one per retry. The raised error's class (and thus
+ the exit code) follows the first problem in input order; the full list
+ travels in ``context``.
+ """
+ problems: list[dict[str, str]] = []
+ for path in inputs:
+ code = _check_one(path)
+ if code is not None:
+ problems.append({"input": str(path), "error_code": code})
+ if not problems:
+ return
+
+ first = problems[0]
+ error_class: type[PdfOpsError] = (
+ InputError if first["error_code"] in _INPUT_PROBLEMS else InvalidPdfError
+ )
+ raise error_class(
+ f"{len(problems)} of {len(inputs)} input(s) unusable; "
+ f"first: {first['input']} ({first['error_code']})",
+ error_code=first["error_code"],
+ context={"problems": problems},
+ )
+
+
+def _check_one(path: Path) -> str | None:
+ if path.is_dir():
+ return "INPUT_IS_DIRECTORY"
+ if not path.is_file():
+ return "INPUT_MISSING"
+ try:
+ with path.open("rb") as handle:
+ head = handle.read(len(PDF_MAGIC))
+ except OSError:
+ return "INPUT_UNREADABLE"
+ if not head.startswith(PDF_MAGIC):
+ return "NOT_A_PDF"
+ return None
diff --git a/src/pdf_ops/merge.py b/src/pdf_ops/merge.py
index c1a170c..ed572d3 100644
--- a/src/pdf_ops/merge.py
+++ b/src/pdf_ops/merge.py
@@ -3,21 +3,16 @@
from __future__ import annotations
import logging
-from collections.abc import Callable, Sequence
-from pathlib import Path
+from collections.abc import Callable
from typing import Any
from pdf_ops.config import MergeConfig, OutputEncryption
from pdf_ops.engine import OpenedInput, get_engine
-from pdf_ops.errors import ConfigError, InputError, InvalidPdfError, PdfOpsError
+from pdf_ops.errors import ConfigError
+from pdf_ops.inputs import validate_inputs
from pdf_ops.output import atomic_output, check_output_path, clean_stale_temps
from pdf_ops.secrets import Secret, Secrets
-PDF_MAGIC = b"%PDF-"
-
-# Problem kinds found during input validation, in exit-code class order.
-_INPUT_PROBLEMS = frozenset({"INPUT_MISSING", "INPUT_IS_DIRECTORY", "INPUT_UNREADABLE"})
-
def run_merge(
config: MergeConfig, get_secrets: Callable[[], Secrets], logger: logging.Logger
@@ -135,46 +130,3 @@ def _choose_output_password(
error_code="MISSING_OUTPUT_PASSWORD",
context={"output_encryption": mode.value},
)
-
-
-def validate_inputs(inputs: Sequence[Path]) -> None:
- """Check every input up front and report all problems in one failure.
-
- An operator fixing a broken workflow should learn about every bad input
- from a single run, not one per retry. The raised error's class (and thus
- the exit code) follows the first problem in input order; the full list
- travels in ``context``.
- """
- problems: list[dict[str, str]] = []
- for path in inputs:
- code = _check_one(path)
- if code is not None:
- problems.append({"input": str(path), "error_code": code})
- if not problems:
- return
-
- first = problems[0]
- error_class: type[PdfOpsError] = (
- InputError if first["error_code"] in _INPUT_PROBLEMS else InvalidPdfError
- )
- raise error_class(
- f"{len(problems)} of {len(inputs)} input(s) unusable; "
- f"first: {first['input']} ({first['error_code']})",
- error_code=first["error_code"],
- context={"problems": problems},
- )
-
-
-def _check_one(path: Path) -> str | None:
- if path.is_dir():
- return "INPUT_IS_DIRECTORY"
- if not path.is_file():
- return "INPUT_MISSING"
- try:
- with path.open("rb") as handle:
- head = handle.read(len(PDF_MAGIC))
- except OSError:
- return "INPUT_UNREADABLE"
- if not head.startswith(PDF_MAGIC):
- return "NOT_A_PDF"
- return None
From 895bff0326e5d88628e10aec4ae5ce35a5671234 Mon Sep 17 00:00:00 2001
From: Radoslav Dimitrov <29573973+Radko-D@users.noreply.github.com>
Date: Fri, 4 Sep 2026 08:16:30 +0300
Subject: [PATCH 10/15] refactor: type the error-code vocabulary
The fine-grained error_code tokens were string literals at each raise
site - 32 of them, readable nowhere as a whole, and a typo would have
shipped a new code silently. They become one ErrorCode StrEnum, and
PdfOpsError now takes error_code: ErrorCode, so pyright catches a bad
code at the raise site. StrEnum serializes exactly like the raw string,
so the JSON log contract is byte-identical; the log-parsing tests that
compare raw strings pin that independently.
docs/OPERATIONS.md gains the complete table, grouped by the exit code
each code travels with, and a unit test fails when the enum and the
table drift in either direction. The remaining small vocabularies get
pyright-checked Literal aliases: password_type (user/owner/empty), the
output-password source, and the existing-output action.
Recorded as D-028.
---
README.md | 3 ++
docs/DECISIONS.md | 18 +++++++++--
docs/DESIGN_NOTES.md | 10 +++++-
docs/OPERATIONS.md | 41 +++++++++++++++++++++++
src/pdf_ops/config.py | 34 +++++++++----------
src/pdf_ops/engine.py | 8 +++--
src/pdf_ops/engine_pikepdf.py | 18 +++++------
src/pdf_ops/errors.py | 54 +++++++++++++++++++++++++++++--
src/pdf_ops/extract.py | 8 ++---
src/pdf_ops/inputs.py | 33 +++++++++++--------
src/pdf_ops/main.py | 4 +--
src/pdf_ops/merge.py | 11 ++++---
src/pdf_ops/output.py | 21 ++++++------
src/pdf_ops/secrets.py | 10 +++---
tests/integration/test_merge.py | 4 +--
tests/integration/test_retries.py | 6 ++--
tests/unit/test_errors.py | 20 ++++++++++--
17 files changed, 223 insertions(+), 80 deletions(-)
diff --git a/README.md b/README.md
index b56cf84..2d4ea74 100644
--- a/README.md
+++ b/README.md
@@ -119,6 +119,9 @@ mounted volumes with absolute in-container paths, the container runs as non-root
| 5 | password required/wrong/unsupported |
| 6 | output conflict or output location unusable |
+The finer-grained `error_code` carried by every `operation_failed` event is listed
+per exit code in [`docs/OPERATIONS.md`](docs/OPERATIONS.md#error-codes).
+
## Logging
Output is JSON lines on stdout - one event per line, stderr stays empty. Lifecycle
diff --git a/docs/DECISIONS.md b/docs/DECISIONS.md
index 8329041..4675fab 100644
--- a/docs/DECISIONS.md
+++ b/docs/DECISIONS.md
@@ -39,8 +39,9 @@ This document is the authoritative register of all architectural decisions for t
| [`D-025`](#D-025) | ๐ข | container | Hardened runtime image: digest-pinned multi-stage build, read-only rootfs | 2026-09-03 | The image is a two-stage build: a digest-pinned uv stage resolves the lockfile into a self-contained virtualenv with compiled bytecode; the runtime stage (digest-pinned python:3.14-slim) carries only that venv, uninstalls the base image's pip, removes stdlib ensurepip (its bundled wheel would restore pip in one command), and runs as fixed non-root UID 10001. Read-only root filesystem is proven by a container test running the golden merge under --read-only --cap-drop ALL --security-opt no-new-privileges (all writes land in the output mount by design). deploy/argo-example.yaml ships the full posture incl. fsGroup, secret-mounted password, a retry expression covering exit 1 plus pod-level Error nodes (which carry no exit code), and memory sized by the measured input+128MB rule. Distroless bases considered and not taken: pinned slim minus pip reaches most of the value while staying debuggable. | [`DESIGN_NOTES.md section 11`](DESIGN_NOTES.md) | - |
| [`D-026`](#D-026) | ๐ข | infra | One uv-locked toolchain with SHA-pinned, Dependabot-watched CI | 2026-09-04 | pre-commit runs ruff and pyright as local hooks through uv run --locked, so hooks, CI and a developer's shell all resolve the single version pinned in uv.lock (the remote-hook revs had drifted behind the lock). CI runs with a read-only token, per-ref concurrency, job timeouts and actions pinned to full commit SHAs; uv sync --locked replaces --frozen. scripts/ and docs/scripts/ join the ruff and pyright gates - the bare 'scripts' exclude had silently covered both, leaving the CI-run decision validator unlinted; the widened rule set (SIM, PTH, PIE, RET, PERF, FURB, N; ASYNC dropped, no async code exists) surfaced nine findings, fixed in place, and pyright now reports ignore comments that suppress nothing. Dependabot watches the three pinned surfaces weekly: the uv lockfile, the actions, the Docker digests. | [`DESIGN_NOTES.md section 12`](DESIGN_NOTES.md) | - |
| [`D-027`](#D-027) | ๐ข | error-handling | Input validation extracted into its own module | 2026-09-04 | validate_inputs, the magic-bytes probe and the input-problem classification set moved byte-for-byte from merge.py into inputs.py; extract.py imports from inputs instead of reaching into merge. This removes the only import edge between the two operation modules, which the design doc presents as parallel peers, and gives the input-problem vocabulary a single home. Pure code motion: error codes, the one-failure-reports-all contract (D-012) and the context.problems log shape are unchanged. | [`DESIGN_NOTES.md section 6`](DESIGN_NOTES.md) | - |
+| [`D-028`](#D-028) | ๐ข | error-handling | Error codes typed as a StrEnum with a drift-tested documentation table | 2026-09-04 | The 32 error_code string literals scattered across src/ become a single ErrorCode StrEnum in errors.py, and PdfOpsError takes error_code: ErrorCode - a typo in a code is now a pyright error instead of a silent new vocabulary entry. StrEnum serializes identically to the raw strings, so the JSON log contract is byte-identical; the untouched test assertions that parse log output and compare raw strings pin that independently. docs/OPERATIONS.md gains the complete code table grouped by exit class, and a unit test fails when the enum and the table drift in either direction, so a new code cannot ship undocumented. The remaining small string vocabularies (password_type, password source, output action) get pyright-checked Literal aliases. | [`DESIGN_NOTES.md section 2`](DESIGN_NOTES.md) | - |
-**Counts:** 27 total decisions - 23 ๐ข decided, 0 ๐ก pending, 0 โธ deferred, 2 ๐ต superseded.
+**Counts:** 28 total decisions - 23 ๐ข decided, 0 ๐ก pending, 0 โธ deferred, 2 ๐ต superseded.
### Index by area
@@ -48,7 +49,7 @@ This document is the authoritative register of all architectural decisions for t
| Area | Count | IDs |
|---|---|---|
-| error-handling | 6 | D-003, D-006, D-012, D-015, D-020, D-027 |
+| error-handling | 7 | D-003, D-006, D-012, D-015, D-020, D-027, D-028 |
| config | 4 | D-004, D-007, D-008, D-009 |
| pdf-engine | 5 | D-002, D-011, D-016, D-018, D-023 |
| security | 5 | D-013, D-014, D-017, D-019, D-024 |
@@ -57,7 +58,7 @@ This document is the authoritative register of all architectural decisions for t
| container | 1 | D-025 |
| project | 1 | D-001 |
| infra | 1 | D-026 |
-| **Total** | **27** | |
+| **Total** | **28** | |
### Open decisions (๐ก Pending + โธ Deferred)
@@ -386,6 +387,17 @@ Per-decision details: status, decided date, rationale, related decisions, and th
- **Reversibility:** cheap
- **Where:** [`DESIGN_NOTES.md section 6`](DESIGN_NOTES.md)
+
+### D-028
+- **Title:** Error codes typed as a StrEnum with a drift-tested documentation table
+- **Status:** ๐ข Decided
+- **Area:** error-handling
+- **Decided on:** 2026-09-04
+- **Summary:** The 32 error_code string literals scattered across src/ become a single ErrorCode StrEnum in errors.py, and PdfOpsError takes error_code: ErrorCode - a typo in a code is now a pyright error instead of a silent new vocabulary entry. StrEnum serializes identically to the raw strings, so the JSON log contract is byte-identical; the untouched test assertions that parse log output and compare raw strings pin that independently. docs/OPERATIONS.md gains the complete code table grouped by exit class, and a unit test fails when the enum and the table drift in either direction, so a new code cannot ship undocumented. The remaining small string vocabularies (password_type, password source, output action) get pyright-checked Literal aliases.
+- **Risk:** low
+- **Reversibility:** cheap
+- **Where:** [`DESIGN_NOTES.md section 2`](DESIGN_NOTES.md)
+
---
## Architectural Decision Records (Full Analysis)
diff --git a/docs/DESIGN_NOTES.md b/docs/DESIGN_NOTES.md
index bd248c3..31d44ee 100644
--- a/docs/DESIGN_NOTES.md
+++ b/docs/DESIGN_NOTES.md
@@ -59,7 +59,7 @@ plaintext `/Encrypt` dictionary; and duplicate-name fidelity required walking
`/Names/EmbeddedFiles` directly (cycle-guarded) because `Pdf.attachments` is a
Mapping, exactly as predicted in section 1.
-## 2. Exit-code taxonomy (per [D-003](DECISIONS.md#D-003))
+## 2. Exit-code taxonomy (per [D-003](DECISIONS.md#D-003), [D-028](DECISIONS.md#D-028))
The process exit code is the application's external API toward the workflow engine. Classes,
not fine-grained codes - workflow engines branch on codes, and codes are a scarce, stable
@@ -85,6 +85,14 @@ dedicated `10+` transient band was resolved with the retry-semantics work
deterministic, and retryability is better expressed as documentation the operator
composes (README's per-code table + retryStrategy expression) than as more codes.
+**Typed vocabulary ([D-028](DECISIONS.md#D-028)):** the fine-grained `error_code`
+tokens started as string literals at each raise site. They are now a single
+`ErrorCode` StrEnum, so the complete vocabulary is readable in one place and a
+typo is a type error rather than a silent new code. StrEnum members serialize
+exactly like the raw strings, so nothing changes on the wire; the log-parsing
+tests that compare raw strings pin that independently. The full table, grouped
+by exit class, lives in OPERATIONS.md with a drift test against the enum.
+
## 3. Environment-variable contract (per [D-004](DECISIONS.md#D-004))
Conventions:
diff --git a/docs/OPERATIONS.md b/docs/OPERATIONS.md
index a588a48..3ecade5 100644
--- a/docs/OPERATIONS.md
+++ b/docs/OPERATIONS.md
@@ -130,3 +130,44 @@ event is never suppressed by `PDFOPS_LOG_LEVEL`:
- `operation_failed` - `error_code` (machine-readable, finer-grained than the exit
code), `error_message`, `exit_code`, `context` (e.g. the failing input), and a
`traceback` for unexpected errors.
+
+### Error codes
+
+`error_code` values, grouped by the exit code they travel with. This is the
+complete vocabulary: a test checks the table against the `ErrorCode` enum in
+`errors.py`, so a new code cannot ship undocumented.
+
+| Code | Exit | Meaning |
+|---|---|---|
+| `UNEXPECTED_ERROR` | 1 | an internal error; the event carries `exc_type` and `traceback` |
+| `UNKNOWN_VAR` | 2 | a `PDFOPS_*` variable the application does not understand (probable typo) |
+| `INAPPLICABLE_VAR` | 2 | a variable that belongs to the other operation |
+| `MISSING_VAR` | 2 | a required variable is unset or empty |
+| `INVALID_OPERATION` | 2 | `PDFOPS_OPERATION` is not `merge` or `extract` |
+| `INVALID_LOG_LEVEL` | 2 | `PDFOPS_LOG_LEVEL` is not one of the accepted levels |
+| `INVALID_INPUTS` | 2 | `PDFOPS_INPUTS` has an empty path component |
+| `DUPLICATE_INPUTS` | 2 | `PDFOPS_INPUTS` lists the same path more than once |
+| `INVALID_FLAG` | 2 | a boolean variable is not `true` or `false` |
+| `INVALID_ON_EXISTS` | 2 | `PDFOPS_ON_EXISTS` is not `fail`, `overwrite` or `skip` |
+| `INVALID_OUTPUT_ENCRYPTION` | 2 | `PDFOPS_OUTPUT_ENCRYPTION` is not `never`, `inherit` or `always` |
+| `CONFLICTING_PASSWORD_SOURCES` | 2 | both the value and the file channel of one password are set |
+| `OUTPUT_PASSWORD_WITHOUT_ENCRYPTION` | 2 | an output password is supplied while output encryption is `never` |
+| `MISSING_OUTPUT_PASSWORD` | 2 | output encryption is required but no explicit password is available |
+| `PASSWORD_FILE_UNREADABLE` | 2 | the password file cannot be read or is not UTF-8 text |
+| `EMPTY_PASSWORD` | 2 | the password file is empty |
+| `PASSWORD_UNSUPPORTED_CHARACTERS` | 2 | the password contains control characters |
+| `INPUT_MISSING` | 3 | an input path does not exist or is not a regular file |
+| `INPUT_IS_DIRECTORY` | 3 | an input path is a directory |
+| `INPUT_UNREADABLE` | 3 | an input file cannot be opened for reading |
+| `NO_ATTACHMENTS` | 3 | the PDF has no attachments and `PDFOPS_FAIL_ON_NO_ATTACHMENTS=true` |
+| `NOT_A_PDF` | 4 | an input does not start with the `%PDF-` header |
+| `CORRUPT_PDF` | 4 | the PDF engine cannot parse or fully read the file |
+| `UNSUPPORTED_PDF_FEATURE` | 4 | the file uses a stream filter this build cannot decode |
+| `PASSWORD_REQUIRED` | 5 | the input is encrypted and no password was supplied |
+| `WRONG_PASSWORD` | 5 | the supplied password does not open the input |
+| `UNSUPPORTED_ENCRYPTION` | 5 | the input uses an encryption scheme this build cannot process |
+| `OUTPUT_DIR_MISSING` | 6 | the output directory does not exist |
+| `OUTPUT_IS_DIRECTORY` | 6 | an output path is a directory |
+| `OUTPUT_EXISTS` | 6 | an output exists and `PDFOPS_ON_EXISTS` is `fail` |
+| `OUTPUT_NOT_WRITABLE` | 6 | the output location is not writable |
+| `DISK_FULL` | 6 | no space left on device while writing |
diff --git a/src/pdf_ops/config.py b/src/pdf_ops/config.py
index 58bc2f0..40034ea 100644
--- a/src/pdf_ops/config.py
+++ b/src/pdf_ops/config.py
@@ -17,7 +17,7 @@
from pathlib import Path
from typing import ClassVar, Literal
-from pdf_ops.errors import ConfigError
+from pdf_ops.errors import ConfigError, ErrorCode
from pdf_ops.secrets import EnvSecret, FileSecret, Secret, SecretRef
ENV_PREFIX = "PDFOPS_"
@@ -178,7 +178,7 @@ def _reject_unknown_vars(env: Mapping[str, str]) -> None:
raise ConfigError(
f"unknown environment variable(s): {', '.join(unknown)}; "
f"accepted: {', '.join(sorted(KNOWN_VARS))}",
- error_code="UNKNOWN_VAR",
+ error_code=ErrorCode.UNKNOWN_VAR,
context={"unknown_vars": unknown},
)
@@ -195,7 +195,7 @@ def _reject_inapplicable_vars(
if present:
raise ConfigError(
f"variable(s) not applicable to operation '{operation.value}': {', '.join(present)}",
- error_code="INAPPLICABLE_VAR",
+ error_code=ErrorCode.INAPPLICABLE_VAR,
context={"operation": operation.value, "inapplicable_vars": present},
)
@@ -205,7 +205,7 @@ def _parse_operation(env: Mapping[str, str]) -> Operation:
if not raw:
raise ConfigError(
f"{VAR_OPERATION} is required (accepted values: merge, extract)",
- error_code="MISSING_VAR",
+ error_code=ErrorCode.MISSING_VAR,
context={"var": VAR_OPERATION},
)
try:
@@ -213,7 +213,7 @@ def _parse_operation(env: Mapping[str, str]) -> Operation:
except ValueError:
raise ConfigError(
f"{VAR_OPERATION} has invalid value {raw!r} (accepted values: merge, extract)",
- error_code="INVALID_OPERATION",
+ error_code=ErrorCode.INVALID_OPERATION,
context={"var": VAR_OPERATION, "value": raw},
) from None
@@ -227,7 +227,7 @@ def _parse_log_level(env: Mapping[str, str]) -> int:
raise ConfigError(
f"{VAR_LOG_LEVEL} has invalid value {raw!r} "
f"(accepted values: {', '.join(_LOG_LEVELS).lower()}, case-insensitive)",
- error_code="INVALID_LOG_LEVEL",
+ error_code=ErrorCode.INVALID_LOG_LEVEL,
context={"var": VAR_LOG_LEVEL, "value": raw},
)
return level
@@ -239,7 +239,7 @@ def _parse_inputs(env: Mapping[str, str]) -> tuple[Path, ...]:
raise ConfigError(
f"{VAR_INPUTS} is required for merge "
f"(ordered file paths separated by {INPUTS_SEPARATOR!r})",
- error_code="MISSING_VAR",
+ error_code=ErrorCode.MISSING_VAR,
context={"var": VAR_INPUTS},
)
parts = [part.strip() for part in raw.split(INPUTS_SEPARATOR)]
@@ -247,7 +247,7 @@ def _parse_inputs(env: Mapping[str, str]) -> tuple[Path, ...]:
raise ConfigError(
f"{VAR_INPUTS} contains an empty path component "
f"(check for stray {INPUTS_SEPARATOR!r} separators)",
- error_code="INVALID_INPUTS",
+ error_code=ErrorCode.INVALID_INPUTS,
context={"var": VAR_INPUTS, "value": raw},
)
paths = [Path(part) for part in parts]
@@ -262,7 +262,7 @@ def _parse_inputs(env: Mapping[str, str]) -> tuple[Path, ...]:
duplicates = sorted({part for part in parts if Path(part) in duplicated_paths})
raise ConfigError(
f"{VAR_INPUTS} lists the same path more than once: {', '.join(duplicates)}",
- error_code="DUPLICATE_INPUTS",
+ error_code=ErrorCode.DUPLICATE_INPUTS,
context={"var": VAR_INPUTS, "duplicates": duplicates},
)
return tuple(paths)
@@ -273,7 +273,7 @@ def _parse_output(env: Mapping[str, str]) -> Path:
if not raw:
raise ConfigError(
f"{VAR_OUTPUT} is required for merge (path of the output PDF)",
- error_code="MISSING_VAR",
+ error_code=ErrorCode.MISSING_VAR,
context={"var": VAR_OUTPUT},
)
return Path(raw)
@@ -284,7 +284,7 @@ def _parse_single_path(env: Mapping[str, str], var: str, purpose: str) -> Path:
if not raw:
raise ConfigError(
f"{var} is required for extract ({purpose})",
- error_code="MISSING_VAR",
+ error_code=ErrorCode.MISSING_VAR,
context={"var": var},
)
return Path(raw)
@@ -299,7 +299,7 @@ def _parse_flag(env: Mapping[str, str], var: str, *, default: bool = False) -> b
return normalized == "true"
raise ConfigError(
f"{var} has invalid value {raw!r} (accepted values: true, false, case-insensitive)",
- error_code="INVALID_FLAG",
+ error_code=ErrorCode.INVALID_FLAG,
context={"var": var, "value": raw},
)
@@ -316,7 +316,7 @@ def _parse_secret_pair(env: Mapping[str, str], value_var: str, file_var: str) ->
if raw_value and raw_file:
raise ConfigError(
f"{value_var} and {file_var} are mutually exclusive - supply one",
- error_code="CONFLICTING_PASSWORD_SOURCES",
+ error_code=ErrorCode.CONFLICTING_PASSWORD_SOURCES,
context={"vars": [value_var, file_var]},
)
if raw_value:
@@ -336,7 +336,7 @@ def _parse_on_exists(env: Mapping[str, str]) -> OnExists:
raise ConfigError(
f"{VAR_ON_EXISTS} has invalid value {raw!r} "
"(accepted values: fail, overwrite, skip, case-insensitive)",
- error_code="INVALID_ON_EXISTS",
+ error_code=ErrorCode.INVALID_ON_EXISTS,
context={"var": VAR_ON_EXISTS, "value": raw},
) from None
@@ -351,7 +351,7 @@ def _parse_output_encryption(env: Mapping[str, str]) -> OutputEncryption:
raise ConfigError(
f"{VAR_OUTPUT_ENCRYPTION} has invalid value {raw!r} "
"(accepted values: never, inherit, always, case-insensitive)",
- error_code="INVALID_OUTPUT_ENCRYPTION",
+ error_code=ErrorCode.INVALID_OUTPUT_ENCRYPTION,
context={"var": VAR_OUTPUT_ENCRYPTION, "value": raw},
) from None
@@ -365,14 +365,14 @@ def _parse_output_password(env: Mapping[str, str], password: SecretRef | None) -
raise ConfigError(
f"an output password is supplied but {VAR_OUTPUT_ENCRYPTION} is 'never' "
"(set it to 'inherit' or 'always', or remove the output password)",
- error_code="OUTPUT_PASSWORD_WITHOUT_ENCRYPTION",
+ error_code=ErrorCode.OUTPUT_PASSWORD_WITHOUT_ENCRYPTION,
context={"var": VAR_OUTPUT_ENCRYPTION},
)
if mode is OutputEncryption.ALWAYS and output_password is None and password is None:
raise ConfigError(
f"{VAR_OUTPUT_ENCRYPTION}=always requires an output password "
f"({VAR_OUTPUT_PASSWORD_FILE}/{VAR_OUTPUT_PASSWORD}) or an input password to fall back to",
- error_code="MISSING_OUTPUT_PASSWORD",
+ error_code=ErrorCode.MISSING_OUTPUT_PASSWORD,
context={"var": VAR_OUTPUT_ENCRYPTION},
)
return output_password
diff --git a/src/pdf_ops/engine.py b/src/pdf_ops/engine.py
index 23db30b..997c97b 100644
--- a/src/pdf_ops/engine.py
+++ b/src/pdf_ops/engine.py
@@ -15,10 +15,14 @@
from collections.abc import Sequence
from dataclasses import dataclass
from pathlib import Path
-from typing import Protocol
+from typing import Literal, Protocol
from pdf_ops.secrets import Secret
+# How an encrypted input opened: with the user or owner password supplied,
+# or through the spec-standard empty-password try.
+type PasswordKind = Literal["user", "owner", "empty"]
+
@dataclass(frozen=True, slots=True)
class OpenedInput:
@@ -37,7 +41,7 @@ class OpenedInput:
pages: int
encrypted: bool
algorithm: str | None
- password_type: str | None
+ password_type: PasswordKind | None
# Recoverable-damage messages the library reported while parsing
# ("repairing", xref rebuilt, ...). The operation layer surfaces them as
# events; anything unrecoverable raises instead.
diff --git a/src/pdf_ops/engine_pikepdf.py b/src/pdf_ops/engine_pikepdf.py
index 4ace556..bd15cb7 100644
--- a/src/pdf_ops/engine_pikepdf.py
+++ b/src/pdf_ops/engine_pikepdf.py
@@ -15,8 +15,8 @@
import pikepdf
-from pdf_ops.engine import Attachment, OpenedInput
-from pdf_ops.errors import InvalidPdfError, PasswordError
+from pdf_ops.engine import Attachment, OpenedInput, PasswordKind
+from pdf_ops.errors import ErrorCode, InvalidPdfError, PasswordError
from pdf_ops.secrets import Secret
# pikepdf converts PDF integers/reals/booleans/null to native Python values,
@@ -49,7 +49,7 @@ def open_input(self, path: Path, password: Secret | None) -> OpenedInput:
if password is None:
raise PasswordError(
f"{path} is encrypted ({algorithm}) and requires a password",
- error_code="PASSWORD_REQUIRED",
+ error_code=ErrorCode.PASSWORD_REQUIRED,
context={"input": str(path), "algorithm": algorithm},
) from None
# The supplied password failed - but this input may not need it
@@ -62,7 +62,7 @@ def open_input(self, path: Path, password: Secret | None) -> OpenedInput:
except pikepdf.PasswordError:
raise PasswordError(
f"the supplied password does not open {path} ({algorithm})",
- error_code="WRONG_PASSWORD",
+ error_code=ErrorCode.WRONG_PASSWORD,
context={"input": str(path), "algorithm": algorithm},
) from None
except pikepdf.PdfError as err:
@@ -74,14 +74,14 @@ def open_input(self, path: Path, password: Secret | None) -> OpenedInput:
# remedy lives in the password class.
raise PasswordError(
f"{path} uses an encryption scheme this build cannot process",
- error_code="UNSUPPORTED_ENCRYPTION",
+ error_code=ErrorCode.UNSUPPORTED_ENCRYPTION,
context={"input": str(path)},
) from err
raise _corrupt(path, err) from err
encrypted = bool(pdf.is_encrypted)
algorithm = _describe_encryption(pdf) if encrypted else None
- password_type: str | None = None
+ password_type: PasswordKind | None = None
if encrypted:
if not used:
password_type = "empty"
@@ -148,7 +148,7 @@ def merge_to(
# turns out unreadable surfaces here without attribution.
raise InvalidPdfError(
f"a merge input could not be fully read while writing: {err}",
- error_code="CORRUPT_PDF",
+ error_code=ErrorCode.CORRUPT_PDF,
context={"inputs": [str(one.path) for one in inputs]},
) from err
# Repairs discovered during the lazy copy accumulate on the
@@ -342,7 +342,7 @@ def _translated_data_error(path: Path, err: Exception) -> InvalidPdfError:
# condition distinct from structural corruption.
return InvalidPdfError(
f"{path} uses a PDF feature this build cannot process: {err}",
- error_code="UNSUPPORTED_PDF_FEATURE",
+ error_code=ErrorCode.UNSUPPORTED_PDF_FEATURE,
context={"input": str(path)},
)
return _corrupt(path, err)
@@ -351,6 +351,6 @@ def _translated_data_error(path: Path, err: Exception) -> InvalidPdfError:
def _corrupt(path: Path, err: Exception) -> InvalidPdfError:
return InvalidPdfError(
f"cannot parse {path} as a PDF: {err}",
- error_code="CORRUPT_PDF",
+ error_code=ErrorCode.CORRUPT_PDF,
context={"input": str(path)},
)
diff --git a/src/pdf_ops/errors.py b/src/pdf_ops/errors.py
index 4990744..06861df 100644
--- a/src/pdf_ops/errors.py
+++ b/src/pdf_ops/errors.py
@@ -9,7 +9,7 @@
from __future__ import annotations
-from enum import IntEnum
+from enum import IntEnum, StrEnum
from typing import Any
@@ -25,10 +25,58 @@ class ExitCode(IntEnum):
OUTPUT = 6
+class ErrorCode(StrEnum):
+ """The machine-readable vocabulary carried by every ``operation_failed`` event.
+
+ Grouped by the exit class each code travels with. The complete table with
+ meanings lives in docs/OPERATIONS.md; a test keeps the two in sync, so a
+ new code cannot ship undocumented.
+ """
+
+ # exit 1 - unexpected
+ UNEXPECTED_ERROR = "UNEXPECTED_ERROR"
+ # exit 2 - configuration
+ UNKNOWN_VAR = "UNKNOWN_VAR"
+ INAPPLICABLE_VAR = "INAPPLICABLE_VAR"
+ MISSING_VAR = "MISSING_VAR"
+ INVALID_OPERATION = "INVALID_OPERATION"
+ INVALID_LOG_LEVEL = "INVALID_LOG_LEVEL"
+ INVALID_INPUTS = "INVALID_INPUTS"
+ DUPLICATE_INPUTS = "DUPLICATE_INPUTS"
+ INVALID_FLAG = "INVALID_FLAG"
+ INVALID_ON_EXISTS = "INVALID_ON_EXISTS"
+ INVALID_OUTPUT_ENCRYPTION = "INVALID_OUTPUT_ENCRYPTION"
+ CONFLICTING_PASSWORD_SOURCES = "CONFLICTING_PASSWORD_SOURCES"
+ OUTPUT_PASSWORD_WITHOUT_ENCRYPTION = "OUTPUT_PASSWORD_WITHOUT_ENCRYPTION"
+ MISSING_OUTPUT_PASSWORD = "MISSING_OUTPUT_PASSWORD"
+ PASSWORD_FILE_UNREADABLE = "PASSWORD_FILE_UNREADABLE"
+ EMPTY_PASSWORD = "EMPTY_PASSWORD"
+ PASSWORD_UNSUPPORTED_CHARACTERS = "PASSWORD_UNSUPPORTED_CHARACTERS"
+ # exit 3 - input
+ INPUT_MISSING = "INPUT_MISSING"
+ INPUT_IS_DIRECTORY = "INPUT_IS_DIRECTORY"
+ INPUT_UNREADABLE = "INPUT_UNREADABLE"
+ NO_ATTACHMENTS = "NO_ATTACHMENTS"
+ # exit 4 - invalid PDF
+ NOT_A_PDF = "NOT_A_PDF"
+ CORRUPT_PDF = "CORRUPT_PDF"
+ UNSUPPORTED_PDF_FEATURE = "UNSUPPORTED_PDF_FEATURE"
+ # exit 5 - password
+ PASSWORD_REQUIRED = "PASSWORD_REQUIRED"
+ WRONG_PASSWORD = "WRONG_PASSWORD"
+ UNSUPPORTED_ENCRYPTION = "UNSUPPORTED_ENCRYPTION"
+ # exit 6 - output
+ OUTPUT_DIR_MISSING = "OUTPUT_DIR_MISSING"
+ OUTPUT_IS_DIRECTORY = "OUTPUT_IS_DIRECTORY"
+ OUTPUT_EXISTS = "OUTPUT_EXISTS"
+ OUTPUT_NOT_WRITABLE = "OUTPUT_NOT_WRITABLE"
+ DISK_FULL = "DISK_FULL"
+
+
class PdfOpsError(Exception):
"""Base class for every predictable failure.
- ``error_code`` is a stable machine-readable token (e.g. ``MISSING_VAR``);
+ ``error_code`` is a stable machine-readable token from ``ErrorCode``;
``context`` holds structured detail for the failure log event. Neither may
ever contain secret material - messages carry paths and names, not values.
"""
@@ -39,7 +87,7 @@ def __init__(
self,
message: str,
*,
- error_code: str,
+ error_code: ErrorCode,
context: dict[str, Any] | None = None,
) -> None:
super().__init__(message)
diff --git a/src/pdf_ops/extract.py b/src/pdf_ops/extract.py
index 57045ae..c360697 100644
--- a/src/pdf_ops/extract.py
+++ b/src/pdf_ops/extract.py
@@ -14,7 +14,7 @@
from pdf_ops.config import ExtractConfig, OnExists
from pdf_ops.engine import Attachment, get_engine
-from pdf_ops.errors import InputError, OutputError
+from pdf_ops.errors import ErrorCode, InputError, OutputError
from pdf_ops.inputs import validate_inputs
from pdf_ops.output import atomic_output, check_output_dir, clean_stale_temps
from pdf_ops.secrets import Secrets
@@ -59,7 +59,7 @@ def run_extract(config: ExtractConfig, secrets: Secrets, logger: logging.Logger)
raise InputError(
f"{config.input} contains no embedded attachments "
"(failing because PDFOPS_FAIL_ON_NO_ATTACHMENTS=true)",
- error_code="NO_ATTACHMENTS",
+ error_code=ErrorCode.NO_ATTACHMENTS,
context={"input": str(config.input)},
)
return {"attachments_extracted": 0, "bytes_written": 0}
@@ -77,7 +77,7 @@ def run_extract(config: ExtractConfig, secrets: Secrets, logger: logging.Logger)
if directories:
raise OutputError(
f"target name(s) are directories in {config.output_dir}: {', '.join(directories)}",
- error_code="OUTPUT_IS_DIRECTORY",
+ error_code=ErrorCode.OUTPUT_IS_DIRECTORY,
context={"output_dir": str(config.output_dir), "directories": directories},
)
@@ -99,7 +99,7 @@ def run_extract(config: ExtractConfig, secrets: Secrets, logger: logging.Logger)
f"{len(conflicts)} file(s) already exist in {config.output_dir}: "
f"{', '.join(conflicts)} (refusing to overwrite; "
"set PDFOPS_ON_EXISTS to overwrite or skip for retry semantics)",
- error_code="OUTPUT_EXISTS",
+ error_code=ErrorCode.OUTPUT_EXISTS,
context={"output_dir": str(config.output_dir), "conflicts": conflicts},
)
case OnExists.SKIP:
diff --git a/src/pdf_ops/inputs.py b/src/pdf_ops/inputs.py
index 0f7fcfa..c3e4531 100644
--- a/src/pdf_ops/inputs.py
+++ b/src/pdf_ops/inputs.py
@@ -10,12 +10,14 @@
from collections.abc import Sequence
from pathlib import Path
-from pdf_ops.errors import InputError, InvalidPdfError, PdfOpsError
+from pdf_ops.errors import ErrorCode, InputError, InvalidPdfError, PdfOpsError
PDF_MAGIC = b"%PDF-"
# Problem kinds found during input validation, in exit-code class order.
-_INPUT_PROBLEMS = frozenset({"INPUT_MISSING", "INPUT_IS_DIRECTORY", "INPUT_UNREADABLE"})
+_INPUT_PROBLEMS = frozenset(
+ {ErrorCode.INPUT_MISSING, ErrorCode.INPUT_IS_DIRECTORY, ErrorCode.INPUT_UNREADABLE}
+)
def validate_inputs(inputs: Sequence[Path]) -> None:
@@ -27,35 +29,38 @@ def validate_inputs(inputs: Sequence[Path]) -> None:
travels in ``context``.
"""
problems: list[dict[str, str]] = []
+ first_code: ErrorCode | None = None
for path in inputs:
code = _check_one(path)
- if code is not None:
- problems.append({"input": str(path), "error_code": code})
- if not problems:
+ if code is None:
+ continue
+ if first_code is None:
+ first_code = code
+ problems.append({"input": str(path), "error_code": code})
+ if first_code is None:
return
- first = problems[0]
error_class: type[PdfOpsError] = (
- InputError if first["error_code"] in _INPUT_PROBLEMS else InvalidPdfError
+ InputError if first_code in _INPUT_PROBLEMS else InvalidPdfError
)
raise error_class(
f"{len(problems)} of {len(inputs)} input(s) unusable; "
- f"first: {first['input']} ({first['error_code']})",
- error_code=first["error_code"],
+ f"first: {problems[0]['input']} ({first_code})",
+ error_code=first_code,
context={"problems": problems},
)
-def _check_one(path: Path) -> str | None:
+def _check_one(path: Path) -> ErrorCode | None:
if path.is_dir():
- return "INPUT_IS_DIRECTORY"
+ return ErrorCode.INPUT_IS_DIRECTORY
if not path.is_file():
- return "INPUT_MISSING"
+ return ErrorCode.INPUT_MISSING
try:
with path.open("rb") as handle:
head = handle.read(len(PDF_MAGIC))
except OSError:
- return "INPUT_UNREADABLE"
+ return ErrorCode.INPUT_UNREADABLE
if not head.startswith(PDF_MAGIC):
- return "NOT_A_PDF"
+ return ErrorCode.NOT_A_PDF
return None
diff --git a/src/pdf_ops/main.py b/src/pdf_ops/main.py
index fbc106b..236dcea 100644
--- a/src/pdf_ops/main.py
+++ b/src/pdf_ops/main.py
@@ -8,7 +8,7 @@
from typing import Any
from pdf_ops.config import Config, ExtractConfig, MergeConfig, parse_config
-from pdf_ops.errors import ExitCode, PdfOpsError
+from pdf_ops.errors import ErrorCode, ExitCode, PdfOpsError
from pdf_ops.extract import run_extract
from pdf_ops.logging_setup import emit_terminal, setup_logging
from pdf_ops.merge import run_merge
@@ -71,7 +71,7 @@ def get_secrets() -> Secrets:
logging.ERROR,
"operation_failed",
{
- "error_code": "UNEXPECTED_ERROR",
+ "error_code": ErrorCode.UNEXPECTED_ERROR,
"exit_code": int(ExitCode.UNEXPECTED),
"duration_s": round(time.monotonic() - started, 3),
},
diff --git a/src/pdf_ops/merge.py b/src/pdf_ops/merge.py
index ed572d3..a29a1b4 100644
--- a/src/pdf_ops/merge.py
+++ b/src/pdf_ops/merge.py
@@ -4,15 +4,18 @@
import logging
from collections.abc import Callable
-from typing import Any
+from typing import Any, Literal
from pdf_ops.config import MergeConfig, OutputEncryption
from pdf_ops.engine import OpenedInput, get_engine
-from pdf_ops.errors import ConfigError
+from pdf_ops.errors import ConfigError, ErrorCode
from pdf_ops.inputs import validate_inputs
from pdf_ops.output import atomic_output, check_output_path, clean_stale_temps
from pdf_ops.secrets import Secret, Secrets
+# Where the output password came from, for the output_encrypted event.
+type PasswordSource = Literal["output", "input-fallback"]
+
def run_merge(
config: MergeConfig, get_secrets: Callable[[], Secrets], logger: logging.Logger
@@ -106,7 +109,7 @@ def run_merge(
def _choose_output_password(
config: MergeConfig, secrets: Secrets, encrypted_count: int
-) -> tuple[Secret | None, str | None]:
+) -> tuple[Secret | None, PasswordSource | None]:
"""Apply the output-encryption policy; returns (password, source-label).
The fallback to the input password uses only an *explicitly supplied*
@@ -127,6 +130,6 @@ def _choose_output_password(
f"output encryption is required (PDFOPS_OUTPUT_ENCRYPTION={mode.value}) but no "
"explicit password is available - the encrypted input(s) opened with the empty "
"password; supply PDFOPS_OUTPUT_PASSWORD_FILE or PDFOPS_OUTPUT_PASSWORD",
- error_code="MISSING_OUTPUT_PASSWORD",
+ error_code=ErrorCode.MISSING_OUTPUT_PASSWORD,
context={"output_encryption": mode.value},
)
diff --git a/src/pdf_ops/output.py b/src/pdf_ops/output.py
index 33c7a41..dce6dde 100644
--- a/src/pdf_ops/output.py
+++ b/src/pdf_ops/output.py
@@ -15,10 +15,13 @@
from collections.abc import Generator
from contextlib import contextmanager, suppress
from pathlib import Path
-from typing import NoReturn
+from typing import Literal, NoReturn
from pdf_ops.config import OnExists
-from pdf_ops.errors import OutputError
+from pdf_ops.errors import ErrorCode, OutputError
+
+# What check_output_path decided for an existing-output policy.
+type OutputAction = Literal["proceed", "skip", "overwrite"]
def check_output_dir(directory: Path) -> None:
@@ -27,12 +30,12 @@ def check_output_dir(directory: Path) -> None:
raise OutputError(
f"output directory {directory} does not exist "
"(output locations are mounted; a missing directory is a workflow bug)",
- error_code="OUTPUT_DIR_MISSING",
+ error_code=ErrorCode.OUTPUT_DIR_MISSING,
context={"output_dir": str(directory)},
)
-def check_output_path(path: Path, on_exists: OnExists) -> str:
+def check_output_path(path: Path, on_exists: OnExists) -> OutputAction:
"""Fail fast on unusable output locations, before any work is done.
Returns the resolved action: ``"proceed"`` (no conflict), ``"skip"``
@@ -44,7 +47,7 @@ def check_output_path(path: Path, on_exists: OnExists) -> str:
raise OutputError(
f"output directory {parent} does not exist "
"(output locations are mounted; a missing directory is a workflow bug)",
- error_code="OUTPUT_DIR_MISSING",
+ error_code=ErrorCode.OUTPUT_DIR_MISSING,
context={"output": str(path)},
)
if path.is_dir() and not path.is_symlink():
@@ -52,7 +55,7 @@ def check_output_path(path: Path, on_exists: OnExists) -> str:
# work and os.replace cannot atomically replace a directory.
raise OutputError(
f"output path {path} is a directory",
- error_code="OUTPUT_IS_DIRECTORY",
+ error_code=ErrorCode.OUTPUT_IS_DIRECTORY,
context={"output": str(path)},
)
if not (path.is_symlink() or path.exists()):
@@ -68,7 +71,7 @@ def check_output_path(path: Path, on_exists: OnExists) -> str:
raise OutputError(
f"output {path} already exists (refusing to overwrite; "
"set PDFOPS_ON_EXISTS to overwrite or skip for retry semantics)",
- error_code="OUTPUT_EXISTS",
+ error_code=ErrorCode.OUTPUT_EXISTS,
context={"output": str(path)},
)
@@ -154,13 +157,13 @@ def _translate_os_error(err: OSError, path: Path) -> Exception:
if err.errno == errno.ENOSPC:
return OutputError(
f"no space left on device while writing {path}",
- error_code="DISK_FULL",
+ error_code=ErrorCode.DISK_FULL,
context={"output": str(path)},
)
if err.errno in (errno.EACCES, errno.EPERM, errno.EROFS):
return OutputError(
f"output location {path} is not writable",
- error_code="OUTPUT_NOT_WRITABLE",
+ error_code=ErrorCode.OUTPUT_NOT_WRITABLE,
context={"output": str(path)},
)
return err
diff --git a/src/pdf_ops/secrets.py b/src/pdf_ops/secrets.py
index f3df42b..ed7f5c0 100644
--- a/src/pdf_ops/secrets.py
+++ b/src/pdf_ops/secrets.py
@@ -24,7 +24,7 @@
from dataclasses import dataclass
from pathlib import Path
-from pdf_ops.errors import ConfigError
+from pdf_ops.errors import ConfigError, ErrorCode
from pdf_ops.logging_setup import register_secret_value
@@ -84,7 +84,7 @@ def resolve(self) -> Secret:
except OSError as err:
raise ConfigError(
f"cannot read password file {self.path}: {err.strerror or err}",
- error_code="PASSWORD_FILE_UNREADABLE",
+ error_code=ErrorCode.PASSWORD_FILE_UNREADABLE,
context={"path": str(self.path)},
) from err
except UnicodeDecodeError as err:
@@ -92,14 +92,14 @@ def resolve(self) -> Secret:
# secret and its position.
raise ConfigError(
f"password file {self.path} is not valid UTF-8 text",
- error_code="PASSWORD_FILE_UNREADABLE",
+ error_code=ErrorCode.PASSWORD_FILE_UNREADABLE,
context={"path": str(self.path)},
) from err
raw = raw.removesuffix("\n").removesuffix("\r")
if not raw:
raise ConfigError(
f"password file {self.path} is empty",
- error_code="EMPTY_PASSWORD",
+ error_code=ErrorCode.EMPTY_PASSWORD,
context={"path": str(self.path)},
)
_reject_control_characters(raw, str(self.path))
@@ -162,6 +162,6 @@ def _reject_control_characters(value: str, origin: str) -> None:
raise ConfigError(
f"the password from {origin} contains control characters "
"(check for encoding or copy-paste issues)",
- error_code="PASSWORD_UNSUPPORTED_CHARACTERS",
+ error_code=ErrorCode.PASSWORD_UNSUPPORTED_CHARACTERS,
context={"source": origin},
)
diff --git a/tests/integration/test_merge.py b/tests/integration/test_merge.py
index 6b83ddc..30cda71 100644
--- a/tests/integration/test_merge.py
+++ b/tests/integration/test_merge.py
@@ -12,7 +12,7 @@
import pdf_ops.merge
from pdf_ops.engine import OpenedInput
-from pdf_ops.errors import InvalidPdfError
+from pdf_ops.errors import ErrorCode, InvalidPdfError
from tests.conftest import RunApp, _build_raw_pdf
pytestmark = pytest.mark.integration
@@ -351,7 +351,7 @@ def test_app_error_after_partial_write_cleans_temp(
out_dir: Path,
run_app: RunApp,
) -> None:
- fake_engine(InvalidPdfError("boom mid-write", error_code="CORRUPT_PDF", context={}))
+ fake_engine(InvalidPdfError("boom mid-write", error_code=ErrorCode.CORRUPT_PDF, context={}))
source = make_pdf()
code, _ = run_app(merge_env([source], out_dir / "m.pdf"))
assert code == 4
diff --git a/tests/integration/test_retries.py b/tests/integration/test_retries.py
index a2cf584..a55cfdf 100644
--- a/tests/integration/test_retries.py
+++ b/tests/integration/test_retries.py
@@ -14,7 +14,7 @@
from pypdf import PdfReader
import pdf_ops.merge
-from pdf_ops.errors import InvalidPdfError
+from pdf_ops.errors import ErrorCode, InvalidPdfError
from tests.conftest import RunApp
from tests.integration.test_extract import extract_env
from tests.integration.test_merge import FakeEngine, merge_env
@@ -304,7 +304,9 @@ def test_failed_overwrite_preserves_the_original(
assert run_app(merge_env([source], output))[0] == 0
original = output.read_bytes()
- fake_engine(InvalidPdfError("boom mid-rewrite", error_code="CORRUPT_PDF", context={}))
+ fake_engine(
+ InvalidPdfError("boom mid-rewrite", error_code=ErrorCode.CORRUPT_PDF, context={})
+ )
code, _ = run_app(merge_env([source], output, PDFOPS_ON_EXISTS="overwrite"))
assert code == 4
diff --git a/tests/unit/test_errors.py b/tests/unit/test_errors.py
index 0d94dbd..b62e367 100644
--- a/tests/unit/test_errors.py
+++ b/tests/unit/test_errors.py
@@ -2,10 +2,14 @@
from __future__ import annotations
+import re
+from pathlib import Path
+
import pytest
from pdf_ops.errors import (
ConfigError,
+ ErrorCode,
ExitCode,
InputError,
InvalidPdfError,
@@ -28,7 +32,7 @@
],
)
def test_error_class_maps_to_exit_code(error_class: type[PdfOpsError], expected_code: int) -> None:
- err = error_class("boom", error_code="SOME_CODE")
+ err = error_class("boom", error_code=ErrorCode.CORRUPT_PDF)
assert int(err.exit_code) == expected_code
@@ -37,7 +41,7 @@ def test_exit_code_values_are_stable() -> None:
def test_error_carries_code_message_and_context() -> None:
- err = ConfigError("bad value", error_code="INVALID_OPERATION", context={"var": "X"})
+ err = ConfigError("bad value", error_code=ErrorCode.INVALID_OPERATION, context={"var": "X"})
assert err.message == "bad value"
assert err.error_code == "INVALID_OPERATION"
assert err.context == {"var": "X"}
@@ -45,5 +49,15 @@ def test_error_carries_code_message_and_context() -> None:
def test_context_defaults_to_empty_dict() -> None:
- err = InputError("missing", error_code="INPUT_MISSING")
+ err = InputError("missing", error_code=ErrorCode.INPUT_MISSING)
assert err.context == {}
+
+
+def test_every_error_code_is_documented() -> None:
+ # The error-code table in docs/OPERATIONS.md is the operator-facing
+ # vocabulary; the enum is the code-facing one. Neither may drift.
+ guide = Path(__file__).resolve().parents[2] / "docs" / "OPERATIONS.md"
+ documented = set(
+ re.findall(r"^\| `([A-Z_]+)` \| [1-6] \|", guide.read_text(), flags=re.MULTILINE)
+ )
+ assert documented == {code.value for code in ErrorCode}
From 166495f369ef09b52a934e503e17731a5eef9b91 Mon Sep 17 00:00:00 2001
From: Radoslav Dimitrov <29573973+Radko-D@users.noreply.github.com>
Date: Fri, 4 Sep 2026 08:17:56 +0300
Subject: [PATCH 11/15] refactor: say each repeated thing once
Four byte-identical except-pairs around the engine's structure walks
collapse into one _translating context manager (the open and save paths
keep their finer handling), and the previously untestable duplicated
branches get a direct pin: every _STRUCTURE_FAILURES member classifies
as CORRUPT_PDF, exit 4, never as an internal error. The hand-built
input_opened payload in merge.py and extract.py moves into
OpenedInput.event_fields() so the two operations cannot drift apart.
On the test side, tests/helpers.py (a plain module) takes over the
RunApp alias, the raw-PDF builder and a new shared make_record helper,
so no test imports from conftest.py - a pytest plugin, not an import
target. Two identical per-class autouse cleanup fixtures in
test_secrets.py become one module-level fixture.
---
src/pdf_ops/engine.py | 10 ++++
src/pdf_ops/engine_pikepdf.py | 43 +++++++-------
src/pdf_ops/extract.py | 11 +---
src/pdf_ops/merge.py | 11 +---
tests/conftest.py | 30 ++--------
tests/helpers.py | 51 +++++++++++++++++
tests/integration/test_extract.py | 14 ++---
tests/integration/test_merge.py | 4 +-
tests/integration/test_passwords.py | 4 +-
tests/integration/test_retries.py | 2 +-
tests/integration/test_run.py | 2 +-
tests/unit/test_engine_translation.py | 26 +++++++++
tests/unit/test_logging.py | 36 ++----------
tests/unit/test_secrets.py | 82 ++++++++-------------------
14 files changed, 157 insertions(+), 169 deletions(-)
create mode 100644 tests/helpers.py
create mode 100644 tests/unit/test_engine_translation.py
diff --git a/src/pdf_ops/engine.py b/src/pdf_ops/engine.py
index 997c97b..1f5c860 100644
--- a/src/pdf_ops/engine.py
+++ b/src/pdf_ops/engine.py
@@ -47,6 +47,16 @@ class OpenedInput:
# events; anything unrecoverable raises instead.
warnings: tuple[str, ...] = ()
+ def event_fields(self) -> dict[str, str | int | bool | None]:
+ """The ``input_opened`` payload - one schema for every operation."""
+ return {
+ "input": str(self.path),
+ "pages": self.pages,
+ "encrypted": self.encrypted,
+ "algorithm": self.algorithm,
+ "password_type": self.password_type,
+ }
+
@dataclass(frozen=True, slots=True)
class Attachment:
diff --git a/src/pdf_ops/engine_pikepdf.py b/src/pdf_ops/engine_pikepdf.py
index bd15cb7..9276bc5 100644
--- a/src/pdf_ops/engine_pikepdf.py
+++ b/src/pdf_ops/engine_pikepdf.py
@@ -9,7 +9,8 @@
import re
import warnings
-from collections.abc import Sequence
+from collections.abc import Generator, Sequence
+from contextlib import contextmanager
from pathlib import Path
from typing import Any, cast
@@ -90,12 +91,8 @@ def open_input(self, path: Path, password: Secret | None) -> OpenedInput:
# supplied string matched.
password_type = "owner" if pdf.owner_password_matched else "user"
- try:
+ with _translating(path):
pages = len(pdf.pages)
- except pikepdf.PdfError as err:
- raise _translated_data_error(path, err) from err
- except _STRUCTURE_FAILURES as err:
- raise _corrupt(path, err) from err
return OpenedInput(
path=path,
@@ -119,12 +116,8 @@ def merge_to(
with pikepdf.Pdf.new() as merged:
for opened in inputs:
source = cast(pikepdf.Pdf, opened.handle)
- try:
+ with _translating(opened.path):
merged.pages.extend(source.pages)
- except pikepdf.PdfError as err:
- raise _translated_data_error(opened.path, err) from err
- except _STRUCTURE_FAILURES as err:
- raise _corrupt(opened.path, err) from err
encryption = None
if output_password is not None:
@@ -160,16 +153,12 @@ def merge_to(
def list_attachments(self, opened: OpenedInput) -> list[Attachment]:
pdf = cast(pikepdf.Pdf, opened.handle)
- try:
+ with _translating(opened.path):
entries = _embedded_file_entries(pdf)
- except pikepdf.PdfError as err:
- raise _translated_data_error(opened.path, err) from err
- except _STRUCTURE_FAILURES as err:
- raise _corrupt(opened.path, err) from err
attachments: list[Attachment] = []
for raw_name, spec in entries:
- try:
+ with _translating(opened.path):
embedded: Any = spec.get("/EF") if isinstance(spec, pikepdf.Dictionary) else None
if embedded is None or not isinstance(embedded, pikepdf.Dictionary):
# A bare file reference (no embedded stream) or a
@@ -181,10 +170,6 @@ def list_attachments(self, opened: OpenedInput) -> list[Attachment]:
if stream is None:
continue
data = bytes(stream.read_bytes())
- except pikepdf.PdfError as err:
- raise _translated_data_error(opened.path, err) from err
- except _STRUCTURE_FAILURES as err:
- raise _corrupt(opened.path, err) from err
# A name-tree key must be a PDF string; qpdf hands anything else
# over as a native Python value (an integer key would make a
# bytes()/str() conversion attacker-sized). A non-string key gets
@@ -336,6 +321,22 @@ def _mentions_encryption(path: Path) -> bool:
return False
+@contextmanager
+def _translating(path: Path) -> Generator[None]:
+ """Translate pikepdf's failure modes while walking ``path``'s structure.
+
+ Wraps ONLY walks over document structure (see ``_STRUCTURE_FAILURES``):
+ a builtin exception inside means a file shape the engine cannot process,
+ never a bug in our code. The open and save paths keep their finer handling.
+ """
+ try:
+ yield
+ except pikepdf.PdfError as err:
+ raise _translated_data_error(path, err) from err
+ except _STRUCTURE_FAILURES as err:
+ raise _corrupt(path, err) from err
+
+
def _translated_data_error(path: Path, err: Exception) -> InvalidPdfError:
if "unfilterable" in str(err):
# A stream filter qpdf cannot decode: a permanent, data-dependent
diff --git a/src/pdf_ops/extract.py b/src/pdf_ops/extract.py
index c360697..d9b0945 100644
--- a/src/pdf_ops/extract.py
+++ b/src/pdf_ops/extract.py
@@ -32,16 +32,7 @@ def run_extract(config: ExtractConfig, secrets: Secrets, logger: logging.Logger)
engine = get_engine()
opened = engine.open_input(config.input, secrets.password)
- logger.info(
- "input_opened",
- extra={
- "input": str(config.input),
- "pages": opened.pages,
- "encrypted": opened.encrypted,
- "algorithm": opened.algorithm,
- "password_type": opened.password_type,
- },
- )
+ logger.info("input_opened", extra=opened.event_fields())
for message in opened.warnings:
logger.warning("pdf_library_message", extra={"detail": message, "source": "qpdf"})
if secrets.password is not None and not opened.encrypted:
diff --git a/src/pdf_ops/merge.py b/src/pdf_ops/merge.py
index a29a1b4..27b3a21 100644
--- a/src/pdf_ops/merge.py
+++ b/src/pdf_ops/merge.py
@@ -39,16 +39,7 @@ def run_merge(
for path in config.inputs:
one = engine.open_input(path, secrets.password)
opened.append(one)
- logger.info(
- "input_opened",
- extra={
- "input": str(path),
- "pages": one.pages,
- "encrypted": one.encrypted,
- "algorithm": one.algorithm,
- "password_type": one.password_type,
- },
- )
+ logger.info("input_opened", extra=one.event_fields())
for message in one.warnings:
logger.warning("pdf_library_message", extra={"detail": message, "source": "qpdf"})
diff --git a/tests/conftest.py b/tests/conftest.py
index fc8ee8e..fc66028 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -16,11 +16,10 @@
from pypdf import PdfWriter
from pdf_ops.main import run
+from tests.helpers import RunApp, build_raw_pdf
TERMINAL_EVENTS = {"operation_complete", "operation_failed"}
-RunApp = Callable[[dict[str, str]], tuple[int, list[dict[str, Any]]]]
-
@pytest.fixture
def run_app(capsys: pytest.CaptureFixture[str]) -> RunApp:
@@ -115,27 +114,6 @@ def _make(name: str = "damaged.pdf") -> Path:
return _make
-def _build_raw_pdf(objects: list[str | bytes]) -> bytes:
- """Minimal hand-assembled PDF with a correct xref - for structural cases
- the writer API refuses to produce (dangling references, missing /Pages,
- raw name-tree bytes)."""
- out = bytearray(b"%PDF-1.4\n")
- offsets: list[int] = []
- for number, body in enumerate(objects, start=1):
- offsets.append(len(out))
- body_bytes = body if isinstance(body, bytes) else body.encode()
- out += f"{number} 0 obj\n".encode() + body_bytes + b"\nendobj\n"
- xref_pos = len(out)
- out += f"xref\n0 {len(objects) + 1}\n".encode()
- out += b"0000000000 65535 f \n"
- for offset in offsets:
- out += f"{offset:010d} 00000 n \n".encode()
- out += (
- f"trailer\n<< /Size {len(objects) + 1} /Root 1 0 R >>\nstartxref\n{xref_pos}\n%%EOF\n"
- ).encode()
- return bytes(out)
-
-
@pytest.fixture
def make_encrypted_pdf(tmp_path: Path) -> Callable[..., Path]:
"""An encrypted one-page PDF.
@@ -175,7 +153,7 @@ def make_dangling_ref_pdf(tmp_path: Path) -> Callable[..., Path]:
def _make(name: str = "dangling.pdf") -> Path:
path = tmp_path / name
path.write_bytes(
- _build_raw_pdf(
+ build_raw_pdf(
[
"<< /Type /Catalog /Pages 2 0 R >>",
"<< /Type /Pages /Kids [3 0 R] /Count 1 >>",
@@ -195,7 +173,7 @@ def make_pathological_pdf(tmp_path: Path) -> Callable[..., Path]:
def _make(name: str = "pathological.pdf") -> Path:
path = tmp_path / name
- path.write_bytes(_build_raw_pdf(["<< /Type /Catalog >>"]))
+ path.write_bytes(build_raw_pdf(["<< /Type /Catalog >>"]))
return path
return _make
@@ -240,7 +218,7 @@ def _make(
filter_part = (b" /Filter " + filter_entry) if filter_entry else b""
path = tmp_path / name
path.write_bytes(
- _build_raw_pdf(
+ build_raw_pdf(
[
b"<< /Type /Catalog /Pages 2 0 R /Names << /EmbeddedFiles "
b"<< /Names [ " + name_literal + b" 4 0 R ] >> >> >>",
diff --git a/tests/helpers.py b/tests/helpers.py
new file mode 100644
index 0000000..a928ae8
--- /dev/null
+++ b/tests/helpers.py
@@ -0,0 +1,51 @@
+"""Shared test helpers.
+
+A plain module rather than conftest.py: conftest is a pytest plugin, and
+importing names from it couples tests to how pytest loaded it.
+"""
+
+from __future__ import annotations
+
+import logging
+from collections.abc import Callable
+from typing import Any
+
+RunApp = Callable[[dict[str, str]], tuple[int, list[dict[str, Any]]]]
+
+
+def make_record(
+ msg: str = "some_event",
+ *,
+ name: str = "pdf_ops",
+ level: int = logging.INFO,
+ exc_info: Any = None,
+ **extra: Any,
+) -> logging.LogRecord:
+ """A LogRecord as the formatter sees it, with ``extra`` fields attached."""
+ record = logging.LogRecord(
+ name=name, level=level, pathname=__file__, lineno=1, msg=msg, args=None, exc_info=exc_info
+ )
+ for key, value in extra.items():
+ setattr(record, key, value)
+ return record
+
+
+def build_raw_pdf(objects: list[str | bytes]) -> bytes:
+ """Minimal hand-assembled PDF with a correct xref - for structural cases
+ the writer API refuses to produce (dangling references, missing /Pages,
+ raw name-tree bytes)."""
+ out = bytearray(b"%PDF-1.4\n")
+ offsets: list[int] = []
+ for number, body in enumerate(objects, start=1):
+ offsets.append(len(out))
+ body_bytes = body if isinstance(body, bytes) else body.encode()
+ out += f"{number} 0 obj\n".encode() + body_bytes + b"\nendobj\n"
+ xref_pos = len(out)
+ out += f"xref\n0 {len(objects) + 1}\n".encode()
+ out += b"0000000000 65535 f \n"
+ for offset in offsets:
+ out += f"{offset:010d} 00000 n \n".encode()
+ out += (
+ f"trailer\n<< /Size {len(objects) + 1} /Root 1 0 R >>\nstartxref\n{xref_pos}\n%%EOF\n"
+ ).encode()
+ return bytes(out)
diff --git a/tests/integration/test_extract.py b/tests/integration/test_extract.py
index 8c970ea..6351250 100644
--- a/tests/integration/test_extract.py
+++ b/tests/integration/test_extract.py
@@ -13,7 +13,7 @@
import pytest
import pdf_ops.extract
-from tests.conftest import RunApp, _build_raw_pdf
+from tests.helpers import RunApp, build_raw_pdf
pytestmark = pytest.mark.integration
@@ -344,7 +344,7 @@ def test_bare_file_reference_without_embedded_stream_is_skipped(
# entries that do carry data.
carrier = tmp_path / "bare-ref.pdf"
carrier.write_bytes(
- _build_raw_pdf(
+ build_raw_pdf(
[
b"<< /Type /Catalog /Pages 2 0 R /Names << /EmbeddedFiles "
b"<< /Names [ (external.txt) 4 0 R (real.txt) 5 0 R ] >> >> >>",
@@ -367,7 +367,7 @@ def test_uf_only_filespec_extracts(
# /EF may carry the stream under /UF (unicode name) with no /F.
carrier = tmp_path / "uf-only.pdf"
carrier.write_bytes(
- _build_raw_pdf(
+ build_raw_pdf(
[
b"<< /Type /Catalog /Pages 2 0 R /Names << /EmbeddedFiles "
b"<< /Names [ (u.txt) 4 0 R ] >> >> >>",
@@ -407,7 +407,7 @@ def test_malformed_tree_shapes_degrade_never_crash(
# never escape as exit 1 - the only class a workflow engine retries.
carrier = tmp_path / "malformed.pdf"
carrier.write_bytes(
- _build_raw_pdf(
+ build_raw_pdf(
[
b"<< /Type /Catalog /Pages 2 0 R /Names << /EmbeddedFiles 4 0 R >> >>",
"<< /Type /Pages /Kids [3 0 R] /Count 1 >>",
@@ -425,7 +425,7 @@ def test_malformed_filespec_ef_is_skipped_sibling_survives(
) -> None:
carrier = tmp_path / "bad-ef.pdf"
carrier.write_bytes(
- _build_raw_pdf(
+ build_raw_pdf(
[
b"<< /Type /Catalog /Pages 2 0 R /Names << /EmbeddedFiles "
b"<< /Names [ (bad.txt) 4 0 R (good.txt) 5 0 R ] >> >> >>",
@@ -450,7 +450,7 @@ def test_integer_name_key_gets_fallback_name_not_a_giant_allocation(
# key gets the deterministic fallback name; the payload survives.
carrier = tmp_path / "int-key.pdf"
carrier.write_bytes(
- _build_raw_pdf(
+ build_raw_pdf(
[
b"<< /Type /Catalog /Pages 2 0 R /Names << /EmbeddedFiles "
b"<< /Names [ 999999999 4 0 R ] >> >> >>",
@@ -473,7 +473,7 @@ def test_cyclic_name_tree_terminates(
# must terminate instead of hanging the container.
carrier = tmp_path / "cyclic.pdf"
carrier.write_bytes(
- _build_raw_pdf(
+ build_raw_pdf(
[
b"<< /Type /Catalog /Pages 2 0 R /Names << /EmbeddedFiles 4 0 R >> >>",
"<< /Type /Pages /Kids [3 0 R] /Count 1 >>",
diff --git a/tests/integration/test_merge.py b/tests/integration/test_merge.py
index 30cda71..076b301 100644
--- a/tests/integration/test_merge.py
+++ b/tests/integration/test_merge.py
@@ -13,7 +13,7 @@
import pdf_ops.merge
from pdf_ops.engine import OpenedInput
from pdf_ops.errors import ErrorCode, InvalidPdfError
-from tests.conftest import RunApp, _build_raw_pdf
+from tests.helpers import RunApp, build_raw_pdf
pytestmark = pytest.mark.integration
@@ -271,7 +271,7 @@ def test_repair_during_lazy_write_still_surfaces_a_warning(
# must still surface as an event, not be silently absorbed.
source = tmp_path / "late.pdf"
source.write_bytes(
- _build_raw_pdf(
+ build_raw_pdf(
[
"<< /Type /Catalog /Pages 2 0 R >>",
"<< /Type /Pages /Kids [3 0 R] /Count 1 >>",
diff --git a/tests/integration/test_passwords.py b/tests/integration/test_passwords.py
index a2842e5..10aecb6 100644
--- a/tests/integration/test_passwords.py
+++ b/tests/integration/test_passwords.py
@@ -18,7 +18,7 @@
from pdf_ops.engine import OpenedInput
from pdf_ops.main import run
from pdf_ops.secrets import Secret
-from tests.conftest import RunApp, _build_raw_pdf
+from tests.helpers import RunApp, build_raw_pdf
from tests.integration.test_extract import extract_env
from tests.integration.test_merge import merge_env
@@ -124,7 +124,7 @@ def test_certificate_encryption_reports_unsupported(
# classification must stay in the password class - never corrupt, and
# never a retryable internal error.
locked = tmp_path / "cert.pdf"
- raw = _build_raw_pdf(
+ raw = build_raw_pdf(
[
"<< /Type /Catalog /Pages 2 0 R >>",
"<< /Type /Pages /Kids [3 0 R] /Count 1 >>",
diff --git a/tests/integration/test_retries.py b/tests/integration/test_retries.py
index a55cfdf..e1b4ab9 100644
--- a/tests/integration/test_retries.py
+++ b/tests/integration/test_retries.py
@@ -15,7 +15,7 @@
import pdf_ops.merge
from pdf_ops.errors import ErrorCode, InvalidPdfError
-from tests.conftest import RunApp
+from tests.helpers import RunApp
from tests.integration.test_extract import extract_env
from tests.integration.test_merge import FakeEngine, merge_env
diff --git a/tests/integration/test_run.py b/tests/integration/test_run.py
index 4e9df41..7a10d44 100644
--- a/tests/integration/test_run.py
+++ b/tests/integration/test_run.py
@@ -14,7 +14,7 @@
import pytest
import pdf_ops.main
-from tests.conftest import RunApp
+from tests.helpers import RunApp
pytestmark = pytest.mark.integration
diff --git a/tests/unit/test_engine_translation.py b/tests/unit/test_engine_translation.py
new file mode 100644
index 0000000..c898270
--- /dev/null
+++ b/tests/unit/test_engine_translation.py
@@ -0,0 +1,26 @@
+"""The structure-walk translation net: a builtin exception raised while
+walking a document's structure is a data problem, never an internal error."""
+
+from __future__ import annotations
+
+from pathlib import Path
+
+import pytest
+
+from pdf_ops.engine_pikepdf import (
+ _STRUCTURE_FAILURES,
+ _translating,
+)
+from pdf_ops.errors import ErrorCode, ExitCode, InvalidPdfError
+
+pytestmark = pytest.mark.unit
+
+
+@pytest.mark.parametrize("exc_type", _STRUCTURE_FAILURES)
+def test_builtin_failure_inside_a_walk_classifies_as_corrupt(
+ exc_type: type[Exception],
+) -> None:
+ with pytest.raises(InvalidPdfError) as caught, _translating(Path("hostile.pdf")):
+ raise exc_type("hostile shape")
+ assert caught.value.error_code == ErrorCode.CORRUPT_PDF
+ assert caught.value.exit_code == ExitCode.INVALID_PDF
diff --git a/tests/unit/test_logging.py b/tests/unit/test_logging.py
index b0d11d8..ef09c48 100644
--- a/tests/unit/test_logging.py
+++ b/tests/unit/test_logging.py
@@ -4,6 +4,7 @@
import json
import logging
+import sys
import pytest
@@ -13,27 +14,11 @@
emit_terminal,
setup_logging,
)
+from tests.helpers import make_record
pytestmark = pytest.mark.unit
-def make_record(
- msg: str = "some_event", extra: dict[str, object] | None = None
-) -> logging.LogRecord:
- record = logging.LogRecord(
- name="pdf_ops",
- level=logging.INFO,
- pathname=__file__,
- lineno=1,
- msg=msg,
- args=None,
- exc_info=None,
- )
- for key, value in (extra or {}).items():
- setattr(record, key, value)
- return record
-
-
class TestJsonFormatter:
def test_emits_valid_json_with_required_fields(self) -> None:
payload = json.loads(JsonFormatter().format(make_record()))
@@ -43,7 +28,7 @@ def test_emits_valid_json_with_required_fields(self) -> None:
assert payload["ts"].endswith("+00:00")
def test_extra_fields_are_merged_into_payload(self) -> None:
- record = make_record(extra={"operation": "merge", "exit_code": 2, "context": {"a": 1}})
+ record = make_record(operation="merge", exit_code=2, context={"a": 1})
payload = json.loads(JsonFormatter().format(record))
assert payload["operation"] == "merge"
assert payload["exit_code"] == 2
@@ -53,14 +38,13 @@ def test_exception_info_is_included(self) -> None:
try:
raise ValueError("boom")
except ValueError:
- record = make_record()
- record.exc_info = __import__("sys").exc_info()
+ record = make_record(exc_info=sys.exc_info())
payload = json.loads(JsonFormatter().format(record))
assert payload["exc_type"] == "ValueError"
assert "boom" in payload["traceback"]
def test_unserializable_values_fall_back_to_str(self) -> None:
- record = make_record(extra={"path": object()})
+ record = make_record(path=object())
payload = json.loads(JsonFormatter().format(record))
assert isinstance(payload["path"], str)
@@ -126,15 +110,7 @@ def test_includes_exception_info_when_requested(
class TestThirdPartyEventFilter:
def pypdf_record(self, message: str) -> logging.LogRecord:
- return logging.LogRecord(
- name="pypdf",
- level=logging.WARNING,
- pathname=__file__,
- lineno=1,
- msg=message,
- args=None,
- exc_info=None,
- )
+ return make_record(message, name="pypdf", level=logging.WARNING)
def test_saslprep_codepoints_are_masked(self) -> None:
# pypdf's SASLprep warning names the exact codepoint of a password
diff --git a/tests/unit/test_secrets.py b/tests/unit/test_secrets.py
index f158e47..61d950b 100644
--- a/tests/unit/test_secrets.py
+++ b/tests/unit/test_secrets.py
@@ -4,7 +4,9 @@
import json
import logging
+import sys
from collections.abc import Generator
+from typing import Any
import pytest
@@ -14,10 +16,23 @@
register_secret_value,
)
from pdf_ops.secrets import Secret
+from tests.helpers import make_record
pytestmark = pytest.mark.unit
+@pytest.fixture(autouse=True)
+def clean_registry() -> Generator[None]:
+ # The redaction registry is process-global; every test starts and ends empty.
+ clear_registered_secrets()
+ yield
+ clear_registered_secrets()
+
+
+def format_record(msg: str, **extra: Any) -> dict[str, object]:
+ return json.loads(JsonFormatter().format(make_record(msg, level=logging.ERROR, **extra)))
+
+
class TestSecret:
def test_never_leaks_through_repr_str_or_format(self) -> None:
secret = Secret("hunter2")
@@ -32,35 +47,15 @@ def test_bool_reflects_emptiness(self) -> None:
class TestRedactionFilter:
- @pytest.fixture(autouse=True)
- def _clean_registry(self) -> Generator[None]:
- clear_registered_secrets()
- yield
- clear_registered_secrets()
-
- def format_record(self, **extra: object) -> dict[str, object]:
- record = logging.LogRecord(
- name="pdf_ops",
- level=logging.ERROR,
- pathname=__file__,
- lineno=1,
- msg="an_event",
- args=None,
- exc_info=None,
- )
- for key, value in extra.items():
- setattr(record, key, value)
- return json.loads(JsonFormatter().format(record))
-
def test_registered_value_scrubbed_from_string_fields(self) -> None:
register_secret_value("hunter2")
- payload = self.format_record(detail="failed with password hunter2 somewhere")
+ payload = format_record("an_event", detail="failed with password hunter2 somewhere")
assert "hunter2" not in json.dumps(payload)
assert payload["detail"] == "failed with password *** somewhere"
def test_scrub_recurses_into_context_dicts_and_lists(self) -> None:
register_secret_value("hunter2")
- payload = self.format_record(context={"inputs": ["a hunter2 b"], "note": "hunter2"})
+ payload = format_record("an_event", context={"inputs": ["a hunter2 b"], "note": "hunter2"})
serialized = json.dumps(payload)
assert "hunter2" not in serialized
assert "***" in serialized
@@ -70,72 +65,41 @@ def test_traceback_payloads_are_scrubbed(self) -> None:
try:
raise RuntimeError("boom hunter2")
except RuntimeError:
- import sys
-
- record = logging.LogRecord(
- name="pdf_ops",
- level=logging.ERROR,
- pathname=__file__,
- lineno=1,
- msg="operation_failed",
- args=None,
- exc_info=sys.exc_info(),
- )
- payload = json.loads(JsonFormatter().format(record))
+ payload = format_record("operation_failed", exc_info=sys.exc_info())
assert "hunter2" not in json.dumps(payload)
assert "boom ***" in str(payload["traceback"])
class TestScrubIntegrity:
- @pytest.fixture(autouse=True)
- def _clean(self) -> Generator[None]:
- clear_registered_secrets()
- yield
- clear_registered_secrets()
-
- def make_payload(self, **extra: object) -> dict[str, object]:
- record = logging.LogRecord(
- name="pdf_ops",
- level=logging.INFO,
- pathname=__file__,
- lineno=1,
- msg="merge_written",
- args=None,
- exc_info=None,
- )
- for key, value in extra.items():
- setattr(record, key, value)
- return json.loads(JsonFormatter().format(record))
-
def test_token_fields_never_scrubbed(self) -> None:
# A password equal to a known token ("merge") must not rewrite
# code-controlled fields: doing so both breaks workflow-engine
# matching and acts as a password oracle.
register_secret_value("merge")
- payload = self.make_payload(operation="merge", error_code="MERGE_FAILED")
+ payload = format_record("merge_written", operation="merge", error_code="MERGE_FAILED")
assert payload["event"] == "merge_written"
assert payload["operation"] == "merge"
assert payload["error_code"] == "MERGE_FAILED"
def test_free_text_fields_are_scrubbed(self) -> None:
register_secret_value("merge")
- payload = self.make_payload(detail="library said merge is wrong")
+ payload = format_record("merge_written", detail="library said merge is wrong")
assert payload["detail"] == "library said *** is wrong"
def test_overlapping_secrets_scrub_longest_first(self) -> None:
register_secret_value("Spring2026")
register_secret_value("Spring2026!x9")
- payload = self.make_payload(detail="bad key 'Spring2026!x9' rejected")
+ payload = format_record("merge_written", detail="bad key 'Spring2026!x9' rejected")
assert payload["detail"] == "bad key '***' rejected"
assert "!x9" not in str(payload["detail"])
def test_repr_escaped_variant_also_scrubbed(self) -> None:
register_secret_value("back\\slash-pw")
# a library embedding the value via %r doubles the backslash
- payload = self.make_payload(detail="rejected 'back\\\\slash-pw' here")
+ payload = format_record("merge_written", detail="rejected 'back\\\\slash-pw' here")
assert "slash-pw" not in str(payload["detail"])
def test_too_short_secrets_are_not_registered(self) -> None:
assert register_secret_value("abc") is False
- payload = self.make_payload(detail="abc appears here")
+ payload = format_record("merge_written", detail="abc appears here")
assert payload["detail"] == "abc appears here"
From 1f3110262d91eaaa87ad3d3334cba0d054aa5f45 Mon Sep 17 00:00:00 2001
From: Radoslav Dimitrov <29573973+Radko-D@users.noreply.github.com>
Date: Fri, 4 Sep 2026 08:18:50 +0300
Subject: [PATCH 12/15] chore: security policy, package metadata, no future
imports
SECURITY.md documents private vulnerability reporting; its in-scope
list is a one-to-one summary of the guarantees the test suite pins.
pyproject gains project.urls and classifiers, led by
Private :: Do Not Upload - the package is a container payload, not a
library, and the index refuses that classifier if a publish ever runs
by mistake.
from __future__ import annotations is dropped across the tree: the
project pins Python 3.14, where deferred annotation evaluation is the
default, so the import was noise carried from habit. README's dev
commands add the format check CI enforces and the one-time
pre-commit install.
Recorded as D-029.
---
README.md | 2 ++
SECURITY.md | 23 +++++++++++++++++++++++
docs/DECISIONS.md | 18 +++++++++++++++---
docs/DESIGN_NOTES.md | 20 ++++++++++++++++++++
docs/scripts/_standard_parser.py | 2 --
docs/scripts/validate_decisions.py | 2 --
pyproject.toml | 15 +++++++++++++++
scripts/benchmark.py | 2 --
src/pdf_ops/__main__.py | 2 --
src/pdf_ops/config.py | 2 --
src/pdf_ops/engine.py | 2 --
src/pdf_ops/engine_pikepdf.py | 2 --
src/pdf_ops/errors.py | 2 --
src/pdf_ops/extract.py | 2 --
src/pdf_ops/inputs.py | 2 --
src/pdf_ops/logging_setup.py | 2 --
src/pdf_ops/main.py | 2 --
src/pdf_ops/merge.py | 2 --
src/pdf_ops/output.py | 2 --
src/pdf_ops/secrets.py | 2 --
tests/conftest.py | 2 --
tests/container/test_image.py | 2 --
tests/helpers.py | 2 --
tests/integration/test_entrypoint.py | 2 --
tests/integration/test_extract.py | 2 --
tests/integration/test_merge.py | 2 --
tests/integration/test_passwords.py | 2 --
tests/integration/test_retries.py | 2 --
tests/integration/test_run.py | 2 --
tests/unit/test_config.py | 2 --
tests/unit/test_encryption_label.py | 2 --
tests/unit/test_engine_translation.py | 2 --
tests/unit/test_errors.py | 2 --
tests/unit/test_logging.py | 2 --
tests/unit/test_sanitize.py | 2 --
tests/unit/test_secrets.py | 2 --
36 files changed, 75 insertions(+), 65 deletions(-)
create mode 100644 SECURITY.md
diff --git a/README.md b/README.md
index 2d4ea74..d00a0d5 100644
--- a/README.md
+++ b/README.md
@@ -174,9 +174,11 @@ retry strategy, measured resource sizing - lives in
```sh
uv sync # deps + venv
+uv run pre-commit install # once: the hooks run the same tools through uv
uv run pytest # unit + integration tests
uv run pytest -m container # container-contract tests (needs Docker)
uv run ruff check . # lint
+uv run ruff format --check . # formatting (CI enforces it)
uv run pyright # strict type check
uv run pre-commit run -a # full hook chain
```
diff --git a/SECURITY.md b/SECURITY.md
new file mode 100644
index 0000000..270e0fb
--- /dev/null
+++ b/SECURITY.md
@@ -0,0 +1,23 @@
+# Security policy
+
+pdf-ops parses untrusted PDFs and writes their attachment names to a mounted
+filesystem, so input handling is security-relevant by design. The posture -
+attachment-name sanitization, the layered no-leak guarantee for passwords,
+atomic outputs, a hardened read-only container - is described in
+`docs/DESIGN.md`.
+
+## Reporting a vulnerability
+
+Please do not open a public issue. Use GitHub's private vulnerability reporting
+on this repository (Security tab, "Report a vulnerability"); if that is not
+enabled, contact the maintainer directly. Include a way to reproduce: the PDF
+or how to build one, the `PDFOPS_*` configuration, and the log lines.
+
+## In scope
+
+- An attachment name that writes outside `PDFOPS_OUTPUT_DIR`.
+- Password material appearing in any output: stdout, stderr, files, tracebacks.
+- A partial or mixed output surviving a failed or killed run.
+- A deterministic bad input classified as retryable (exit 1).
+- A hostile PDF structure that escapes the error taxonomy (exit 1) instead of
+ classifying as a data problem.
diff --git a/docs/DECISIONS.md b/docs/DECISIONS.md
index 4675fab..1697914 100644
--- a/docs/DECISIONS.md
+++ b/docs/DECISIONS.md
@@ -40,8 +40,9 @@ This document is the authoritative register of all architectural decisions for t
| [`D-026`](#D-026) | ๐ข | infra | One uv-locked toolchain with SHA-pinned, Dependabot-watched CI | 2026-09-04 | pre-commit runs ruff and pyright as local hooks through uv run --locked, so hooks, CI and a developer's shell all resolve the single version pinned in uv.lock (the remote-hook revs had drifted behind the lock). CI runs with a read-only token, per-ref concurrency, job timeouts and actions pinned to full commit SHAs; uv sync --locked replaces --frozen. scripts/ and docs/scripts/ join the ruff and pyright gates - the bare 'scripts' exclude had silently covered both, leaving the CI-run decision validator unlinted; the widened rule set (SIM, PTH, PIE, RET, PERF, FURB, N; ASYNC dropped, no async code exists) surfaced nine findings, fixed in place, and pyright now reports ignore comments that suppress nothing. Dependabot watches the three pinned surfaces weekly: the uv lockfile, the actions, the Docker digests. | [`DESIGN_NOTES.md section 12`](DESIGN_NOTES.md) | - |
| [`D-027`](#D-027) | ๐ข | error-handling | Input validation extracted into its own module | 2026-09-04 | validate_inputs, the magic-bytes probe and the input-problem classification set moved byte-for-byte from merge.py into inputs.py; extract.py imports from inputs instead of reaching into merge. This removes the only import edge between the two operation modules, which the design doc presents as parallel peers, and gives the input-problem vocabulary a single home. Pure code motion: error codes, the one-failure-reports-all contract (D-012) and the context.problems log shape are unchanged. | [`DESIGN_NOTES.md section 6`](DESIGN_NOTES.md) | - |
| [`D-028`](#D-028) | ๐ข | error-handling | Error codes typed as a StrEnum with a drift-tested documentation table | 2026-09-04 | The 32 error_code string literals scattered across src/ become a single ErrorCode StrEnum in errors.py, and PdfOpsError takes error_code: ErrorCode - a typo in a code is now a pyright error instead of a silent new vocabulary entry. StrEnum serializes identically to the raw strings, so the JSON log contract is byte-identical; the untouched test assertions that parse log output and compare raw strings pin that independently. docs/OPERATIONS.md gains the complete code table grouped by exit class, and a unit test fails when the enum and the table drift in either direction, so a new code cannot ship undocumented. The remaining small string vocabularies (password_type, password source, output action) get pyright-checked Literal aliases. | [`DESIGN_NOTES.md section 2`](DESIGN_NOTES.md) | - |
+| [`D-029`](#D-029) | ๐ข | project | Security policy and package metadata; future-import dropped on 3.14 | 2026-09-04 | SECURITY.md documents private vulnerability reporting with an in-scope list that maps one-to-one onto the test-pinned guarantees (attachment-name containment, the password no-leak layers, atomic outputs, no taxonomy escapes). pyproject gains project.urls and trove classifiers, including Private :: Do Not Upload so an accidental publish is refused by the index. from __future__ import annotations dropped across the tree: the project pins Python 3.14, where deferred annotation evaluation is the default, so the import was pure noise; there are no TYPE_CHECKING guards anywhere that depended on it. README's dev commands now include the format check CI enforces and the one-time pre-commit install. | [`DESIGN_NOTES.md section 13`](DESIGN_NOTES.md) | - |
-**Counts:** 28 total decisions - 23 ๐ข decided, 0 ๐ก pending, 0 โธ deferred, 2 ๐ต superseded.
+**Counts:** 29 total decisions - 23 ๐ข decided, 0 ๐ก pending, 0 โธ deferred, 2 ๐ต superseded.
### Index by area
@@ -56,9 +57,9 @@ This document is the authoritative register of all architectural decisions for t
| reliability | 3 | D-010, D-021, D-022 |
| observability | 1 | D-005 |
| container | 1 | D-025 |
-| project | 1 | D-001 |
+| project | 2 | D-001, D-029 |
| infra | 1 | D-026 |
-| **Total** | **28** | |
+| **Total** | **29** | |
### Open decisions (๐ก Pending + โธ Deferred)
@@ -398,6 +399,17 @@ Per-decision details: status, decided date, rationale, related decisions, and th
- **Reversibility:** cheap
- **Where:** [`DESIGN_NOTES.md section 2`](DESIGN_NOTES.md)
+
+### D-029
+- **Title:** Security policy and package metadata; future-import dropped on 3.14
+- **Status:** ๐ข Decided
+- **Area:** project
+- **Decided on:** 2026-09-04
+- **Summary:** SECURITY.md documents private vulnerability reporting with an in-scope list that maps one-to-one onto the test-pinned guarantees (attachment-name containment, the password no-leak layers, atomic outputs, no taxonomy escapes). pyproject gains project.urls and trove classifiers, including Private :: Do Not Upload so an accidental publish is refused by the index. from __future__ import annotations dropped across the tree: the project pins Python 3.14, where deferred annotation evaluation is the default, so the import was pure noise; there are no TYPE_CHECKING guards anywhere that depended on it. README's dev commands now include the format check CI enforces and the one-time pre-commit install.
+- **Risk:** low
+- **Reversibility:** cheap
+- **Where:** [`DESIGN_NOTES.md section 13`](DESIGN_NOTES.md)
+
---
## Architectural Decision Records (Full Analysis)
diff --git a/docs/DESIGN_NOTES.md b/docs/DESIGN_NOTES.md
index 31d44ee..49f5870 100644
--- a/docs/DESIGN_NOTES.md
+++ b/docs/DESIGN_NOTES.md
@@ -474,3 +474,23 @@ Two related gaps closed in the same pass:
Pinning everything creates a staleness problem, so Dependabot watches the
three pinned surfaces weekly: the uv lockfile (dev tooling grouped into one
PR), the action SHAs, and the Docker base-image digests.
+
+## 13. Repository housekeeping (per [D-029](DECISIONS.md#D-029))
+
+Three small decisions that make the repository read correctly from the
+outside:
+
+- **SECURITY.md**: private vulnerability reporting, with an in-scope list
+ that is a one-to-one summary of the guarantees the test suite pins -
+ attachment-name containment, the password no-leak layers, whole-or-absent
+ outputs, and the rule that a hostile input must classify as a data
+ problem, never as a retryable internal error.
+- **Package metadata**: `project.urls` and trove classifiers in pyproject,
+ led by `Private :: Do Not Upload` - the package is a container payload,
+ not a library, and the index refuses that classifier if anyone ever runs
+ a publish by mistake.
+- **No `from __future__ import annotations`**: the project pins Python
+ 3.14, where deferred annotation evaluation (PEP 649) is the default. The
+ import survived from habit in every module; dropping it removes a
+ visible signal of code written for older interpreters. No TYPE_CHECKING
+ guard anywhere depended on it.
diff --git a/docs/scripts/_standard_parser.py b/docs/scripts/_standard_parser.py
index a1147bc..c189839 100755
--- a/docs/scripts/_standard_parser.py
+++ b/docs/scripts/_standard_parser.py
@@ -12,8 +12,6 @@
controlled-vocabularies section to change vocabularies - every script picks up the change.
"""
-from __future__ import annotations
-
import re
import sys
from pathlib import Path
diff --git a/docs/scripts/validate_decisions.py b/docs/scripts/validate_decisions.py
index 879e551..1553937 100755
--- a/docs/scripts/validate_decisions.py
+++ b/docs/scripts/validate_decisions.py
@@ -30,8 +30,6 @@
Exit code 0 on clean run, 1 on any violation. Prints one line per issue.
"""
-from __future__ import annotations
-
import re
import sys
from collections import Counter, defaultdict
diff --git a/pyproject.toml b/pyproject.toml
index 36d653c..a86bde0 100644
--- a/pyproject.toml
+++ b/pyproject.toml
@@ -9,10 +9,25 @@ authors = [
{ email = "29573973+Radko-D@users.noreply.github.com" }
]
requires-python = ">=3.14"
+classifiers = [
+ # Guard against an accidental `uv publish`: PyPI refuses this classifier.
+ # Remove it when the package is meant to be published.
+ "Private :: Do Not Upload",
+ "Environment :: No Input/Output (Daemon)",
+ "Intended Audience :: System Administrators",
+ "Operating System :: POSIX :: Linux",
+ "Programming Language :: Python :: 3.14",
+ "Topic :: Office/Business",
+ "Typing :: Typed",
+]
dependencies = [
"pikepdf>=10.12.0",
]
+[project.urls]
+Repository = "https://github.com/Radko-D/python-pdfops"
+Design = "https://github.com/Radko-D/python-pdfops/blob/main/docs/DESIGN.md"
+
[project.scripts]
pdf-ops = "pdf_ops.__main__:main"
diff --git a/scripts/benchmark.py b/scripts/benchmark.py
index ac6e114..c9680b2 100644
--- a/scripts/benchmark.py
+++ b/scripts/benchmark.py
@@ -12,8 +12,6 @@
The workdir defaults to /tmp/pdfops-bench (a path Docker Desktop shares).
"""
-from __future__ import annotations
-
import argparse
import json
import os
diff --git a/src/pdf_ops/__main__.py b/src/pdf_ops/__main__.py
index 63a373a..ae08eb3 100644
--- a/src/pdf_ops/__main__.py
+++ b/src/pdf_ops/__main__.py
@@ -4,8 +4,6 @@
everything else operates on plain mappings and return values.
"""
-from __future__ import annotations
-
import os
import sys
diff --git a/src/pdf_ops/config.py b/src/pdf_ops/config.py
index 40034ea..bea88dc 100644
--- a/src/pdf_ops/config.py
+++ b/src/pdf_ops/config.py
@@ -7,8 +7,6 @@
readability of paths are operation-stage concerns, not configuration ones.
"""
-from __future__ import annotations
-
import logging
import os
from collections.abc import Mapping
diff --git a/src/pdf_ops/engine.py b/src/pdf_ops/engine.py
index 1f5c860..9bea013 100644
--- a/src/pdf_ops/engine.py
+++ b/src/pdf_ops/engine.py
@@ -10,8 +10,6 @@
before any output work starts.
"""
-from __future__ import annotations
-
from collections.abc import Sequence
from dataclasses import dataclass
from pathlib import Path
diff --git a/src/pdf_ops/engine_pikepdf.py b/src/pdf_ops/engine_pikepdf.py
index 9276bc5..c844888 100644
--- a/src/pdf_ops/engine_pikepdf.py
+++ b/src/pdf_ops/engine_pikepdf.py
@@ -5,8 +5,6 @@
the only code that calls ``Secret.reveal()``.
"""
-from __future__ import annotations
-
import re
import warnings
from collections.abc import Generator, Sequence
diff --git a/src/pdf_ops/errors.py b/src/pdf_ops/errors.py
index 06861df..6b96a4e 100644
--- a/src/pdf_ops/errors.py
+++ b/src/pdf_ops/errors.py
@@ -7,8 +7,6 @@
carried by every raised error and emitted in the terminal log event.
"""
-from __future__ import annotations
-
from enum import IntEnum, StrEnum
from typing import Any
diff --git a/src/pdf_ops/extract.py b/src/pdf_ops/extract.py
index d9b0945..4f0f169 100644
--- a/src/pdf_ops/extract.py
+++ b/src/pdf_ops/extract.py
@@ -6,8 +6,6 @@
write is verified to stay inside the output directory.
"""
-from __future__ import annotations
-
import logging
from dataclasses import dataclass
from typing import Any
diff --git a/src/pdf_ops/inputs.py b/src/pdf_ops/inputs.py
index c3e4531..6845241 100644
--- a/src/pdf_ops/inputs.py
+++ b/src/pdf_ops/inputs.py
@@ -5,8 +5,6 @@
drift apart.
"""
-from __future__ import annotations
-
from collections.abc import Sequence
from pathlib import Path
diff --git a/src/pdf_ops/logging_setup.py b/src/pdf_ops/logging_setup.py
index 3314d19..9840062 100644
--- a/src/pdf_ops/logging_setup.py
+++ b/src/pdf_ops/logging_setup.py
@@ -5,8 +5,6 @@
detail is passed via ``extra`` and merged into the payload.
"""
-from __future__ import annotations
-
import json
import logging
import re
diff --git a/src/pdf_ops/main.py b/src/pdf_ops/main.py
index 236dcea..e5bdce2 100644
--- a/src/pdf_ops/main.py
+++ b/src/pdf_ops/main.py
@@ -1,7 +1,5 @@
"""Top-level orchestration: the single error boundary and operation dispatch."""
-from __future__ import annotations
-
import logging
import time
from collections.abc import Callable, Mapping
diff --git a/src/pdf_ops/merge.py b/src/pdf_ops/merge.py
index 27b3a21..b244d8a 100644
--- a/src/pdf_ops/merge.py
+++ b/src/pdf_ops/merge.py
@@ -1,7 +1,5 @@
"""The merge operation: validate everything, then write once, atomically."""
-from __future__ import annotations
-
import logging
from collections.abc import Callable
from typing import Any, Literal
diff --git a/src/pdf_ops/output.py b/src/pdf_ops/output.py
index dce6dde..b15d3ac 100644
--- a/src/pdf_ops/output.py
+++ b/src/pdf_ops/output.py
@@ -7,8 +7,6 @@
PDF where a downstream workflow step could read it.
"""
-from __future__ import annotations
-
import errno
import os
import tempfile
diff --git a/src/pdf_ops/secrets.py b/src/pdf_ops/secrets.py
index ed7f5c0..011495d 100644
--- a/src/pdf_ops/secrets.py
+++ b/src/pdf_ops/secrets.py
@@ -18,8 +18,6 @@
guarantee; scrubbing catches library residue such as exception messages).
"""
-from __future__ import annotations
-
import logging
from dataclasses import dataclass
from pathlib import Path
diff --git a/tests/conftest.py b/tests/conftest.py
index fc66028..caba690 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -5,8 +5,6 @@
from the library version.
"""
-from __future__ import annotations
-
import json
from collections.abc import Callable
from pathlib import Path
diff --git a/tests/container/test_image.py b/tests/container/test_image.py
index efeba61..2ce6398 100644
--- a/tests/container/test_image.py
+++ b/tests/container/test_image.py
@@ -8,8 +8,6 @@
host paths, and pytest's default tmp dir is not among them.
"""
-from __future__ import annotations
-
import json
import os
import shutil
diff --git a/tests/helpers.py b/tests/helpers.py
index a928ae8..b5eaa77 100644
--- a/tests/helpers.py
+++ b/tests/helpers.py
@@ -4,8 +4,6 @@
importing names from it couples tests to how pytest loaded it.
"""
-from __future__ import annotations
-
import logging
from collections.abc import Callable
from typing import Any
diff --git a/tests/integration/test_entrypoint.py b/tests/integration/test_entrypoint.py
index 067c984..8489d9e 100644
--- a/tests/integration/test_entrypoint.py
+++ b/tests/integration/test_entrypoint.py
@@ -5,8 +5,6 @@
and stdout/stderr split match the documented contract.
"""
-from __future__ import annotations
-
import json
import subprocess
import sys
diff --git a/tests/integration/test_extract.py b/tests/integration/test_extract.py
index 6351250..dace7be 100644
--- a/tests/integration/test_extract.py
+++ b/tests/integration/test_extract.py
@@ -5,8 +5,6 @@
PDFOPS_OUTPUT_DIR.
"""
-from __future__ import annotations
-
from collections.abc import Callable
from pathlib import Path
diff --git a/tests/integration/test_merge.py b/tests/integration/test_merge.py
index 076b301..040f2cf 100644
--- a/tests/integration/test_merge.py
+++ b/tests/integration/test_merge.py
@@ -1,7 +1,5 @@
"""End-to-end merge runs through run(env): the merge operator contract."""
-from __future__ import annotations
-
import errno
import os
from collections.abc import Callable
diff --git a/tests/integration/test_passwords.py b/tests/integration/test_passwords.py
index 10aecb6..ecf1819 100644
--- a/tests/integration/test_passwords.py
+++ b/tests/integration/test_passwords.py
@@ -6,8 +6,6 @@
or crash.
"""
-from __future__ import annotations
-
from collections.abc import Callable
from pathlib import Path
diff --git a/tests/integration/test_retries.py b/tests/integration/test_retries.py
index e1b4ab9..528e67b 100644
--- a/tests/integration/test_retries.py
+++ b/tests/integration/test_retries.py
@@ -5,8 +5,6 @@
"the step ran before - what does running it again do?"
"""
-from __future__ import annotations
-
from collections.abc import Callable
from pathlib import Path
diff --git a/tests/integration/test_run.py b/tests/integration/test_run.py
index 7a10d44..9e44383 100644
--- a/tests/integration/test_run.py
+++ b/tests/integration/test_run.py
@@ -6,8 +6,6 @@
one terminal event, emitted last.
"""
-from __future__ import annotations
-
import logging
from typing import Any
diff --git a/tests/unit/test_config.py b/tests/unit/test_config.py
index b8673fc..ac878ce 100644
--- a/tests/unit/test_config.py
+++ b/tests/unit/test_config.py
@@ -1,7 +1,5 @@
"""Table-driven tests for the env-var configuration contract."""
-from __future__ import annotations
-
import logging
import os
from pathlib import Path
diff --git a/tests/unit/test_encryption_label.py b/tests/unit/test_encryption_label.py
index 4930881..af15bae 100644
--- a/tests/unit/test_encryption_label.py
+++ b/tests/unit/test_encryption_label.py
@@ -1,8 +1,6 @@
"""The failed-open encryption label: a raw scan of an attacker-controlled
file that must stay correct on real layouts and harmless on hostile ones."""
-from __future__ import annotations
-
from collections.abc import Callable
from pathlib import Path
diff --git a/tests/unit/test_engine_translation.py b/tests/unit/test_engine_translation.py
index c898270..bbc7090 100644
--- a/tests/unit/test_engine_translation.py
+++ b/tests/unit/test_engine_translation.py
@@ -1,8 +1,6 @@
"""The structure-walk translation net: a builtin exception raised while
walking a document's structure is a data problem, never an internal error."""
-from __future__ import annotations
-
from pathlib import Path
import pytest
diff --git a/tests/unit/test_errors.py b/tests/unit/test_errors.py
index b62e367..2a436b5 100644
--- a/tests/unit/test_errors.py
+++ b/tests/unit/test_errors.py
@@ -1,7 +1,5 @@
"""The exception-to-exit-code mapping is the external API - pin it."""
-from __future__ import annotations
-
import re
from pathlib import Path
diff --git a/tests/unit/test_logging.py b/tests/unit/test_logging.py
index ef09c48..ea123ff 100644
--- a/tests/unit/test_logging.py
+++ b/tests/unit/test_logging.py
@@ -1,7 +1,5 @@
"""The JSON log line shape is an operator interface - pin it."""
-from __future__ import annotations
-
import json
import logging
import sys
diff --git a/tests/unit/test_sanitize.py b/tests/unit/test_sanitize.py
index 6e05ef9..9bed841 100644
--- a/tests/unit/test_sanitize.py
+++ b/tests/unit/test_sanitize.py
@@ -4,8 +4,6 @@
filesystem - this table is the security contract for that boundary.
"""
-from __future__ import annotations
-
import pytest
from pdf_ops.engine import Attachment
diff --git a/tests/unit/test_secrets.py b/tests/unit/test_secrets.py
index 61d950b..76e42c5 100644
--- a/tests/unit/test_secrets.py
+++ b/tests/unit/test_secrets.py
@@ -1,7 +1,5 @@
"""The Secret wrapper and the log-redaction layer - the no-leak machinery."""
-from __future__ import annotations
-
import json
import logging
import sys
From 7327d5388dcfbb1e2e5b7cf7b1cf38f68d5b7782 Mon Sep 17 00:00:00 2001
From: Radoslav Dimitrov <29573973+Radko-D@users.noreply.github.com>
Date: Fri, 4 Sep 2026 08:21:33 +0300
Subject: [PATCH 13/15] docs: draw the architecture in rendered views; make the
operator guide exact
docs/ARCHITECTURE.md shows the system in eight views that render
directly on GitHub - deployment context, the module import graph, one
run end to end, the failure taxonomy, the extract trust boundary, the
retry state machine, the test-oracle map and the commit-to-image path -
each linking into the interactive diagrams for the deep-dive. A unit
test keeps every module named on the page.
docs/OPERATIONS.md's logging section becomes a complete event table -
all 16 events with level and fields - pinned by a test that harvests
every event name the code emits. A line-by-line pass against the code
fixes the inaccuracies the old prose carried: stale-temp cleanup runs
before the first write, not at startup; both password variables are
scrubbed from the environment; the scrub has a four-character floor;
duplicate attachment names dedupe casefolded with a 200-byte cap;
original_name is null when nothing was renamed; a directory at any
output path is refused under every policy; the no-attachments flag
requires the literal true. Merge resolves secrets only after the skip
decision - extract up front - and the docs now say exactly that.
README points the env-var table at the operator guide anchors; the
design doc links the rendered views.
---
README.md | 18 +--
docs/ARCHITECTURE.md | 262 ++++++++++++++++++++++++++++++++++++++++
docs/DESIGN.md | 5 +-
docs/OPERATIONS.md | 66 ++++++----
tests/unit/test_docs.py | 37 ++++++
5 files changed, 357 insertions(+), 31 deletions(-)
create mode 100644 docs/ARCHITECTURE.md
create mode 100644 tests/unit/test_docs.py
diff --git a/README.md b/README.md
index d00a0d5..ba10704 100644
--- a/README.md
+++ b/README.md
@@ -8,10 +8,12 @@ operation per container run - **merge** multiple PDFs into one, or **extract** t
attachments embedded in a PDF - configured entirely through environment variables.
The design - architecture, library tradeoffs, security posture, limitations - is
-summarized in [`docs/DESIGN.md`](docs/DESIGN.md); the working notes behind it are
-[`docs/DESIGN_NOTES.md`](docs/DESIGN_NOTES.md), and individual choices, with their
-alternatives and status, live in the decision register at
-[`docs/DECISIONS.md`](docs/DECISIONS.md).
+summarized in [`docs/DESIGN.md`](docs/DESIGN.md) and drawn, view by view, in
+[`docs/ARCHITECTURE.md`](docs/ARCHITECTURE.md); the runtime behavior contract - mounts,
+passwords, output policy, log events, error codes - is [`docs/OPERATIONS.md`](docs/OPERATIONS.md);
+the working notes behind the design are [`docs/DESIGN_NOTES.md`](docs/DESIGN_NOTES.md),
+and individual choices, with their alternatives and status, live in the decision
+register at [`docs/DECISIONS.md`](docs/DECISIONS.md).
## Quick start
@@ -86,10 +88,10 @@ from `PDFOPS_*` variables, and the mounted volumes provide inputs and receive ou
| `PDFOPS_FAIL_ON_NO_ATTACHMENTS` | extract | no | `true`, `false` (case-insensitive) - fail (exit 3) when the PDF has no attachments | `false` |
| `PDFOPS_PASSWORD_FILE` | both | no | path to a mounted secret file holding the password (preferred channel; one trailing newline stripped) | - |
| `PDFOPS_PASSWORD` | both | no | the password itself - discouraged: env values leak via `kubectl describe`, `/proc//environ`, crash tooling | - |
-| `PDFOPS_OUTPUT_ENCRYPTION` | merge | no | `never`, `inherit`, `always` (case-insensitive) - see below | `never` |
+| `PDFOPS_OUTPUT_ENCRYPTION` | merge | no | `never`, `inherit`, `always` (case-insensitive) - see [Output encryption](docs/OPERATIONS.md#output-encryption) | `never` |
| `PDFOPS_OUTPUT_PASSWORD_FILE` | merge | no | secret file holding the password for the merged output | - |
| `PDFOPS_OUTPUT_PASSWORD` | merge | no | output password as a direct value (same caveats as `PDFOPS_PASSWORD`) | - |
-| `PDFOPS_ON_EXISTS` | both | no | `fail`, `overwrite`, `skip` (case-insensitive) - see Retries | `fail` |
+| `PDFOPS_ON_EXISTS` | both | no | `fail`, `overwrite`, `skip` (case-insensitive) - see [Existing outputs](docs/OPERATIONS.md#atomic-writes-and-existing-outputs) | `fail` |
| `PDFOPS_LOG_LEVEL` | - | no | `debug`, `info`, `warning`, `error` (case-insensitive) | `info` |
Strictness rules, all exit 2: any other `PDFOPS_*` variable is rejected as a probable
@@ -125,8 +127,8 @@ per exit code in [`docs/OPERATIONS.md`](docs/OPERATIONS.md#error-codes).
## Logging
Output is JSON lines on stdout - one event per line, stderr stays empty. Lifecycle
-events narrate progress and respect `PDFOPS_LOG_LEVEL`; passwords are echoed as
-presence only (`unset` / `set(env)` / `set(file)`), never as values. Every run ends
+events narrate progress and respect `PDFOPS_LOG_LEVEL`; the `config_loaded` event
+echoes passwords as presence only (`unset` / `set(env)` / `set(file)`), never as values. Every run ends
with exactly one terminal event - `operation_complete` or `operation_failed` (with a
machine-readable `error_code`) - which no log level suppresses, so a workflow engine
can always branch on the last line:
diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md
new file mode 100644
index 0000000..39ab709
--- /dev/null
+++ b/docs/ARCHITECTURE.md
@@ -0,0 +1,262 @@
+# Architecture in diagrams
+
+Eight views of the same system, drawn so they render on GitHub, in pull requests
+and in most editors. Each says why the shape is what it is and links into the
+interactive deep-dive at [`diagrams/index.html`](diagrams/index.html), where nodes
+link to the source lines they describe. Prose lives in [`DESIGN.md`](DESIGN.md), the
+operator contract in [`OPERATIONS.md`](OPERATIONS.md), and every choice in the
+decision register [`DECISIONS.md`](DECISIONS.md).
+
+A test (`tests/unit/test_docs.py`) checks that every module under `src/pdf_ops` is
+named on this page, so no module can go missing from this view.
+
+## 1. Where it runs
+
+One container run is one workflow step. Everything the process needs arrives as
+environment variables and mounted volumes; everything it reports leaves as JSON
+lines on stdout and an exit code. For merge, the password file is read only after
+the existing-output check says work remains, which is why a merge retry after a
+lost-but-successful pod succeeds even when the secret mount is already gone;
+extract resolves it up front, before touching the carrier.
+
+```mermaid
+flowchart LR
+ engine["Workflow engine Argo step with retryStrategy"]
+ subgraph pod["Pod: one container run"]
+ env["PDFOPS_* environment"]
+ proc["python -m pdf_ops UID 10001, read-only rootfs"]
+ data[("/data volume inputs read, outputs written atomically")]
+ secret[("/secrets volume password file, read-only")]
+ end
+ engine -- "env, volumes, memory limit" --> env
+ env --> proc
+ data <--> proc
+ secret -. "merge reads it late; extract up front" .-> proc
+ proc -- "JSON lines on stdout" --> engine
+ proc -- "exit code 0-6" --> engine
+```
+
+Deep-dive: the deployment posture this is tested under is
+[`../deploy/argo-example.yaml`](../deploy/argo-example.yaml).
+
+## 2. Modules and the direction of dependency
+
+Every arrow is a real `import`; nothing points upward. `errors.py` and
+`logging_setup.py` import nothing from the package, so anything may use them.
+`engine_pikepdf.py` is the only module that imports pikepdf, reached through
+`engine.py`'s `get_engine()` with a lazy import, so the seam knows its
+implementation while the rest of the package never does.
+
+```mermaid
+flowchart TB
+ subgraph entry["Entry and orchestration"]
+ n_entry["__main__.py process boundary: os.environ in, exit code out"]
+ n_main["main.py run(env): the one error boundary"]
+ end
+ subgraph ops["Operations"]
+ n_merge["merge.py validate everything, then write once"]
+ n_extract["extract.py untrusted names in, atomic files out"]
+ end
+ subgraph seam["Engine seam"]
+ n_engine["engine.py PdfEngine protocol, OpenedInput"]
+ n_pike["engine_pikepdf.py the only module importing pikepdf"]
+ end
+ subgraph found["Foundations"]
+ n_config["config.py pure parse of the env contract"]
+ n_inputs["inputs.py up-front input validation"]
+ n_output["output.py atomic writes, existing-output policy"]
+ n_secrets["secrets.py Secret wrapper, refs, late resolution"]
+ n_logging["logging_setup.py JSON lines, scrubbing, terminal events"]
+ n_errors["errors.py ExitCode, ErrorCode, the error classes"]
+ end
+ n_entry --> n_main
+ n_entry --> n_config
+ n_main --> n_config
+ n_main --> n_merge
+ n_main --> n_extract
+ n_main --> n_secrets
+ n_main --> n_logging
+ n_main --> n_errors
+ n_merge --> n_config
+ n_merge --> n_engine
+ n_merge --> n_inputs
+ n_merge --> n_output
+ n_merge --> n_secrets
+ n_merge --> n_errors
+ n_extract --> n_config
+ n_extract --> n_engine
+ n_extract --> n_inputs
+ n_extract --> n_output
+ n_extract --> n_secrets
+ n_extract --> n_errors
+ n_engine -. "lazy, inside get_engine()" .-> n_pike
+ n_engine --> n_secrets
+ n_pike --> n_engine
+ n_pike --> n_secrets
+ n_pike --> n_errors
+ n_config --> n_secrets
+ n_config --> n_errors
+ n_output --> n_config
+ n_output --> n_errors
+ n_inputs --> n_errors
+ n_secrets --> n_logging
+ n_secrets --> n_errors
+```
+
+Deep-dive: [`diagrams/index.html#architecture`](diagrams/index.html#architecture).
+
+## 3. One run, end to end
+
+`run(env)` is the only place that catches exceptions and the only place that emits a
+terminal event. Configuration is parsed completely before any file is touched;
+merge resolves secrets only after the existing-output policy has decided the run
+proceeds, extract at dispatch; output goes to a temp file in the destination
+directory and is renamed in one step.
+
+```mermaid
+sequenceDiagram
+ autonumber
+ participant E as __main__
+ participant R as main.run
+ participant C as config
+ participant O as merge / extract
+ participant F as inputs / output
+ participant S as secrets
+ participant P as engine_pikepdf
+ participant L as logging_setup
+ E->>R: run(env)
+ R->>L: setup_logging()
+ R->>C: parse_config(env)
+ C-->>R: MergeConfig or ExtractConfig, else ConfigError
+ R->>L: config_loaded (passwords as presence only)
+ R->>O: dispatch by config type
+ O->>F: check_output_path / validate_inputs
+ O->>S: resolve_and_register (merge: after the skip decision)
+ O->>P: open_input(path, password)
+ P-->>O: OpenedInput: pages, encryption facts, repair warnings
+ O->>F: atomic_output(path): temp file beside the target
+ O->>P: merge_to / list_attachments
+ F-->>O: fsync and rename, or cleanup on failure
+ O-->>R: result fields
+ R->>L: emit_terminal(operation_complete or operation_failed)
+ R-->>E: exit code
+```
+
+## 4. Failures: classes, codes, exit codes
+
+The exit code is the external API and stays small. Each error class maps to exactly
+one code; the finer `error_code` travels in the terminal event. Anything that is not
+a `PdfOpsError` is a bug and exits 1, the only class an engine should retry. The
+complete code table is in [`OPERATIONS.md#error-codes`](OPERATIONS.md#error-codes).
+
+```mermaid
+flowchart LR
+ any["A failure inside run(env)"] --> pred{"PdfOpsError?"}
+ pred -- "no" --> c1["exit 1 UNEXPECTED UNEXPECTED_ERROR, exc_type, traceback"]
+ pred -- "yes" --> cls{"which class?"}
+ cls --> c2["ConfigError, exit 2 UNKNOWN_VAR, MISSING_VAR, INVALID_*, DUPLICATE_INPUTS, CONFLICTING_PASSWORD_SOURCES, MISSING_OUTPUT_PASSWORD, PASSWORD_FILE_UNREADABLE, EMPTY_PASSWORD, ..."]
+ cls --> c3["InputError, exit 3 INPUT_MISSING, INPUT_IS_DIRECTORY, INPUT_UNREADABLE, NO_ATTACHMENTS"]
+ cls --> c4["InvalidPdfError, exit 4 NOT_A_PDF, CORRUPT_PDF, UNSUPPORTED_PDF_FEATURE"]
+ cls --> c5["PasswordError, exit 5 PASSWORD_REQUIRED, WRONG_PASSWORD, UNSUPPORTED_ENCRYPTION"]
+ cls --> c6["OutputError, exit 6 OUTPUT_DIR_MISSING, OUTPUT_IS_DIRECTORY, OUTPUT_EXISTS, OUTPUT_NOT_WRITABLE, DISK_FULL"]
+```
+
+## 5. Extract: the boundary an attachment name crosses
+
+An attachment name is attacker-controlled text that ends up as a filename on a
+mounted volume. Every name passes through a pure, table-tested sanitizer, then a
+casefolded collision check (the volume may be case-insensitive), then the
+existing-output policy, then a containment re-check that backs the sanitizer up,
+and only then an atomic write.
+
+```mermaid
+flowchart LR
+ pdf["Untrusted PDF /Names/EmbeddedFiles name tree"] --> raw["Raw name separators, traversal, control characters, any length, duplicates, or not even a string"]
+ raw --> san["sanitize_attachment_name basename, strip C0/C1, cap at 200 bytes, deterministic attachment_n fallback"]
+ san --> dedupe["_dedupe casefolded collisions get -1, -2 suffixes"]
+ dedupe --> policy{"target exists?"}
+ policy -- "fail (default)" --> refuse["OUTPUT_EXISTS, exit 6 nothing written"]
+ policy -- "skip" --> keep["completed prior work left as is"]
+ policy -- "absent, or overwrite" --> check["containment re-check parent of target is PDFOPS_OUTPUT_DIR"]
+ check --> write["atomic_output temp file in the output dir, then rename"]
+ write --> fs[("PDFOPS_OUTPUT_DIR")]
+```
+
+Deep-dive: [`diagrams/index.html#extract`](diagrams/index.html#extract).
+
+## 6. Retries: the existing-output policy as a state machine
+
+Workflow engines retry at least once, and a pod can vanish after its work
+succeeded. `PDFOPS_ON_EXISTS` decides what the next attempt does with an output
+that already exists; stale temp debris from a killed write is removed before the
+first write of the next attempt.
+
+```mermaid
+stateDiagram-v2
+ state "Config parsed" as Parsed
+ state "Output exists?" as Exists
+ state "Temp write" as Writing
+ state "Refused: OUTPUT_EXISTS, exit 6" as Refused
+ state "Skipped: exit 0, skipped true" as Skipped
+ state "Complete: exit 0" as Complete
+ state "Killed mid-write" as Killed
+ [*] --> Invoked
+ Invoked --> Parsed : parse_config
+ Invoked --> [*] : ConfigError, exit 2
+ Parsed --> Exists : check_output_path
+ Exists --> Writing : absent
+ Exists --> Writing : present and overwrite
+ Exists --> Refused : present and fail (default)
+ Exists --> Skipped : present and skip
+ Writing --> Complete : fsync, rename, terminal event
+ Writing --> Killed : pod lost
+ Killed --> Parsed : retry, stale temp removed before the first write
+ Refused --> [*]
+ Skipped --> [*]
+ Complete --> [*]
+ note right of Skipped
+ merge: whole-run no-op, inputs and password file never read
+ extract: per file, only the missing attachments are written
+ end note
+```
+
+Deep-dive: [`diagrams/index.html#lifecycle`](diagrams/index.html#lifecycle).
+
+## 7. Tests and the cross-library oracle
+
+pypdf is a dev-only dependency with one job: it builds every fixture and re-reads
+every output, so each test is a check between two independent PDF libraries.
+Fixtures are generated, never checked in. Three tiers run at three costs.
+
+```mermaid
+flowchart LR
+ subgraph oracle["pypdf: the test oracle"]
+ build["conftest factories build fixtures plain, encrypted, damaged, dangling refs, hostile name trees, raw attachments"]
+ verify["pypdf re-reads the outputs page counts, encryption, attachment bytes"]
+ end
+ subgraph sut["pdf_ops with the pikepdf engine"]
+ unit["unit: pure functions config, sanitizer, errors, secrets, logging, docs sync"]
+ integ["integration: run(env) in-process invariants on every run: empty stderr, JSON lines only, exactly one terminal event, last"]
+ cont["container: docker build and run golden merge and extract, mounted secret, hardened posture, no package installer"]
+ end
+ build --> integ
+ build --> cont
+ integ --> verify
+ cont --> verify
+```
+
+## 8. From commit to image
+
+One toolchain, one version of each tool, pinned in `uv.lock`: the hooks, CI and a
+developer's shell all run ruff and pyright through uv. The image is built from the
+same lockfile and ships nothing that could change it.
+
+```mermaid
+flowchart LR
+ commit["commit"] --> hooks["pre-commit through uv hygiene, ruff check and format, pyright"]
+ hooks --> push["push, pull request"]
+ push --> quality["CI quality job uv sync --locked, ruff, format, pyright, pytest, decision-register validator"]
+ push --> docker["CI docker job build the image, container contract tests"]
+ docker --> image["image: builder stage uv sync --locked, then runtime digest-pinned base, no pip or ensurepip, UID 10001"]
+ bot["Dependabot uv.lock, actions, base image"] -.-> push
+```
diff --git a/docs/DESIGN.md b/docs/DESIGN.md
index 744d9d9..f10bf85 100644
--- a/docs/DESIGN.md
+++ b/docs/DESIGN.md
@@ -43,7 +43,10 @@ Small modules with one-way dependencies:
An interactive map of these modules - each node linking to the source lines it
describes - lives at [`diagrams/index.html`](diagrams/index.html#architecture),
alongside diagrams of the retry lifecycle, the extract trust boundary, and the
-password flow, all navigable from one page.
+password flow, all navigable from one page. The same structure drawn to render on
+GitHub - deployment context, module graph, run sequence, failure taxonomy, trust
+boundary, retry machine, test oracle, delivery pipeline - is
+[`ARCHITECTURE.md`](ARCHITECTURE.md).
Cross-cutting rules: unknown or operation-inapplicable `PDFOPS_*` variables are hard
errors - a silently ignored misspelling becomes a confusing downstream failure
diff --git a/docs/OPERATIONS.md b/docs/OPERATIONS.md
index 3ecade5..48a0d57 100644
--- a/docs/OPERATIONS.md
+++ b/docs/OPERATIONS.md
@@ -27,16 +27,18 @@ output policies, and what the log stream carries. The short reference tables
the spec-standard empty-password try, exactly like every PDF viewer. Wrong
password -> exit 5 naming the failing input.
- The password itself never appears in any output: the in-process `Secret` wrapper
- renders as `***`, the logging layer scrubs registered secret values from every
- event including tracebacks, and the process scrubs `PDFOPS_PASSWORD` from its own
- environment on startup.
+ renders as `***`, the logging layer scrubs registered secret values (four
+ characters or longer; a shorter one draws a `redaction_degraded` warning) from the
+ free-text fields of every event including tracebacks, and the process scrubs
+ `PDFOPS_PASSWORD` and `PDFOPS_OUTPUT_PASSWORD` from its own environment on startup.
- Each `input_opened` event reports the encryption algorithm (read from the PDF's
plaintext `/Encrypt` dictionary) and how the file opened (`user`/`owner`/`empty`).
- A permissions-locked input among user-locked ones never fails just because a
password was supplied: the empty try still applies per input (the exact call
sequence is drawn in [`diagrams/index.html#passwords`](diagrams/index.html#passwords)).
- Passwords containing control characters are rejected (exit 2) as encoding
- accidents.
+ accidents, at the moment the password is resolved - a merge that short-circuits
+ on `skip` never reads it.
- Note the env channel's inherent limit: the initial environment block stays visible
to `docker inspect` and `/proc//environ` - the file channel is the one that
keeps the value out of the process's environment entirely.
@@ -50,7 +52,11 @@ this step; `always` encrypts unconditionally. The output password comes from
`PDFOPS_OUTPUT_PASSWORD_FILE`/`PDFOPS_OUTPUT_PASSWORD`, falling back to the
*explicitly supplied* input password (never the empty auto-try). Output encryption is
always AES-256, whatever the inputs used. Supplying an output password while the mode
-is `never` is a hard configuration error.
+is `never` is a hard configuration error. With no password available anywhere
+(`always` at config parse, or `inherit` when the only encrypted inputs opened via the
+empty try) the run fails with `MISSING_OUTPUT_PASSWORD`, exit 2, before anything is
+written. `security_downgrade` is a warning-level event, so `PDFOPS_LOG_LEVEL=error`
+hides it; the terminal event's `output_encrypted: false` is the level-proof signal.
## Atomic writes and existing outputs
@@ -67,9 +73,11 @@ is `never` is a hard configuration error.
are written (`attachments_skipped` reports the rest), so a crashed run's partial
set gets finished by the retry - sound because every file this tool writes is
atomic and therefore whole. Both `skip` modes trust that an existing file is a
- completed prior output.
+ completed prior output. A directory at the output path, or at any extraction
+ target name, is refused under every policy (`OUTPUT_IS_DIRECTORY`, exit 6).
- Temp debris from a crashed prior run (`.name.*.tmp` matching this run's own
- targets) is removed at startup with a `stale_temp_removed` event. The whole
+ targets) is removed before the first write, with a `stale_temp_removed` event; a
+ run refused or failed before that point leaves it for the next attempt. The whole
run/retry state machine is drawn in
[`diagrams/index.html#lifecycle`](diagrams/index.html#lifecycle). One writer per
output path at a time is assumed - which a workflow engine guarantees per step.
@@ -81,8 +89,11 @@ is `never` is a hard configuration error.
- **Attachment names are treated as untrusted input**: extraction reduces every name
to a sanitized basename (path separators, traversal segments, and control
characters removed; deterministic `attachment_` fallback), so a hostile PDF can
- never write outside `PDFOPS_OUTPUT_DIR`. Duplicate names get deterministic
- `-1`/`-2` suffixes; the original name is logged whenever sanitization changed it.
+ never write outside `PDFOPS_OUTPUT_DIR`. Names longer than 200 bytes are
+ truncated. Duplicate names get deterministic `-1`/`-2` suffixes, with collisions
+ detected case-insensitively (the volume may be), so `Report.txt` and `report.txt`
+ become `Report.txt` and `report-1.txt` on every filesystem; the original name is
+ logged whenever sanitization changed it.
- The path every untrusted name travels is drawn in
[`diagrams/index.html#extract`](diagrams/index.html#extract).
- Extraction order is the PDF's name-tree order - deterministic across runs. Each
@@ -91,7 +102,7 @@ is `never` is a hard configuration error.
(exit 6) - see `PDFOPS_ON_EXISTS` above for the retry-friendly modes. A directory
at an output path is refused under every policy (`OUTPUT_IS_DIRECTORY`). A PDF
with zero attachments is a success with `attachments_extracted=0` unless
- `PDFOPS_FAIL_ON_NO_ATTACHMENTS` is set.
+ `PDFOPS_FAIL_ON_NO_ATTACHMENTS=true` (exit 3, `NO_ATTACHMENTS`).
## Resource sizing
@@ -118,18 +129,29 @@ the workflow engine reports the kill itself.
## Log events
-One JSON object per line on stdout; stderr stays empty. Lifecycle events narrate
-progress at their log levels (`config_loaded`, `input_opened`, `merge_written`,
-`attachment_extracted`, `stale_temp_removed`, `pdf_library_message` for damage the
-PDF engine repaired, `security_downgrade`, `password_unused`, ...). The terminal
-event is never suppressed by `PDFOPS_LOG_LEVEL`:
-
-- `operation_complete` - merge: `pages`, `bytes_written`, `output_path`,
- `output_encrypted`; extract: `attachments_extracted`, `bytes_written`, plus
- `attachments_skipped` under `skip`. Always: `exit_code`, `duration_s`.
-- `operation_failed` - `error_code` (machine-readable, finer-grained than the exit
- code), `error_message`, `exit_code`, `context` (e.g. the failing input), and a
- `traceback` for unexpected errors.
+One JSON object per line on stdout; stderr stays empty. Every event carries `ts`,
+`level` and `event`; the other fields depend on the event. Lifecycle events respect
+`PDFOPS_LOG_LEVEL`; the two terminal events never do. This is the complete
+vocabulary: a test checks it against every event the code emits.
+
+| Event | Level | When, and what it carries |
+|---|---|---|
+| `config_loaded` | info | the parsed configuration: `operation`, `log_level`, `on_exists`, `output_encryption` (merge), and `password` / `output_password` as presence only (`unset` / `set(env)` / `set(file)`), never values |
+| `operation_started` | info | dispatch into merge or extract |
+| `input_opened` | info | per input: `input`, `pages`, `encrypted`, `algorithm`, `password_type` (`user` / `owner` / `empty`) |
+| `pdf_library_message` | warning, or the library's own higher level | `detail` and `source`: damage the engine repaired (`source: qpdf`), or anything the PDF library or a Python warning routes through logging - those records bypass `PDFOPS_LOG_LEVEL` |
+| `password_unused` | warning | a password was supplied but no input needed it |
+| `security_downgrade` | warning | encrypted inputs merged into a plaintext output under `never`: `encrypted_inputs` |
+| `redaction_degraded` | warning | a supplied secret is shorter than four characters and is not scrubbed from free text |
+| `stale_temp_removed` | warning | a prior run's temp debris for this target was removed: `temp_file` |
+| `output_skipped` | info | merge under `skip`: the existing output is accepted as completed work, `output_path` |
+| `output_overwritten` | info | after the write: `output_path` for merge, `replaced` and `count` for extract |
+| `output_encrypted` | info | the merged output was encrypted: `algorithm`, `password_source` (`output` / `input-fallback`) |
+| `merge_written` | info | `output_path`, `pages_per_input`, `output_encrypted` |
+| `attachments_skipped` | info | extract under `skip`: `skipped` (names), `count` |
+| `attachment_extracted` | info | per file: `attachment`, `bytes`, and `original_name` - the raw document name (capped at 200 characters) when sanitization or dedupe renamed the file, `null` otherwise |
+| `operation_complete` | info | terminal, exit 0. Always `operation`, `exit_code`, `duration_s`. Merge adds `inputs_merged`, `pages`, `bytes_written`, `output_path`, `output_encrypted`, or under `skip` only `skipped: true` and `output_path`. Extract adds `attachments_extracted`, `bytes_written`, and `attachments_skipped` when any file was skipped |
+| `operation_failed` | error | terminal, exit 1-6. Always `error_code`, `exit_code`, `duration_s`. Predictable failures add `error_message` and `context`; an unexpected error (exit 1) adds `exc_type` and `traceback` instead |
### Error codes
diff --git a/tests/unit/test_docs.py b/tests/unit/test_docs.py
new file mode 100644
index 0000000..1a5c2ba
--- /dev/null
+++ b/tests/unit/test_docs.py
@@ -0,0 +1,37 @@
+"""Documentation that doubles as a contract stays in step with the code."""
+
+import re
+from pathlib import Path
+
+import pytest
+
+pytestmark = pytest.mark.unit
+
+ROOT = Path(__file__).resolve().parents[2]
+SRC = ROOT / "src" / "pdf_ops"
+
+# Every way the package names a log event: a logger call, the terminal emitter,
+# and the third-party filter that rewrites foreign records.
+_EVENT_PATTERNS = (
+ re.compile(r'logger\.(?:debug|info|warning|error)\(\s*"([a-z_]+)"'),
+ re.compile(r'emit_terminal\([^)]*?"([a-z_]+)"', re.DOTALL),
+ re.compile(r'record\.msg = "([a-z_]+)"'),
+)
+
+
+def test_every_module_is_in_the_architecture_doc() -> None:
+ doc = (ROOT / "docs" / "ARCHITECTURE.md").read_text()
+ modules = sorted(p.name for p in SRC.glob("*.py") if p.name != "__init__.py")
+ missing = [name for name in modules if name not in doc]
+ assert modules and not missing, missing
+
+
+def test_every_log_event_is_documented() -> None:
+ emitted: set[str] = set()
+ for path in SRC.glob("*.py"):
+ text = path.read_text()
+ for pattern in _EVENT_PATTERNS:
+ emitted.update(pattern.findall(text))
+ guide = (ROOT / "docs" / "OPERATIONS.md").read_text()
+ missing = sorted(name for name in emitted if f"`{name}`" not in guide)
+ assert emitted and not missing, missing
From 1ca9d84c61bd80981888e6d7c614549ec41cec8d Mon Sep 17 00:00:00 2001
From: Radoslav Dimitrov <29573973+Radko-D@users.noreply.github.com>
Date: Fri, 4 Sep 2026 08:24:46 +0300
Subject: [PATCH 14/15] docs: add the input validation module to the
interactive diagram
The module map gains inputs.py between the two operations - both call
the shared collect-all validation before any engine work - and the
evidence pins move to the current revision with every source range
re-checked against the tree.
---
docs/diagrams/pdf-ops-architecture.html | 52 ++++++++++------
docs/diagrams/pdf-ops.architecture.json | 82 +++++++++++++++++--------
2 files changed, 92 insertions(+), 42 deletions(-)
diff --git a/docs/diagrams/pdf-ops-architecture.html b/docs/diagrams/pdf-ops-architecture.html
index d12d46d..d90271b 100644
--- a/docs/diagrams/pdf-ops-architecture.html
+++ b/docs/diagrams/pdf-ops-architecture.html
@@ -4959,8 +4959,8 @@