Repository navigation
Conversation
Execute the planned engine swap (D-023): engine_pikepdf.py replaces engine_pypdf.py behind the unchanged Protocol. qpdf brings the production qualities the seam was built for - it repairs light damage (truncation, mangled xref) the old engine refused, with every repair surfaced as a pdf_library_message event rather than silently absorbed: open-time warnings ride on OpenedInput.warnings, and because qpdf reads stream data lazily, repairs discovered during the write or during attachment reads are drained afterwards via collect_warnings. Password reporting survives intact (qpdf records which document password matched, so input_opened still says user/owner/empty), output encryption pins R=6 (always AES-256), and the merged output is saved through the atomic layer's already-open temp file - handed a path, pikepdf would create a second hidden temp whose post-kill debris no cleanup could ever match. Hostile-input hardening, since qpdf hands malformed structures over as native Python values: the /Names/EmbeddedFiles walk (needed because the attachments mapping collapses duplicate names) guards node types and reference cycles, classifies structural garbage as a data problem instead of a retryable internal error, refuses to turn an integer name-tree key into an attacker-sized bytes() allocation (the payload still extracts under the fallback name), and caps the logged original_name. Failed opens keep their algorithm label via a raw scan scoped to the actual /Encrypt object - bounded token matches, so decoy digit walls neither crash int() nor mislabel. Attachment names now decode per spec (PDFDocEncoding/UTF-16) instead of as UTF-8 mojibake. Corruption fixtures became genuinely unrecoverable (qpdf repairs the old ones - that upgrade is pinned as behavior), and pypdf moves to the dev group where it builds every fixture and independently verifies outputs: each green test is a two-library cross-check. 269 tests + 5 container, 95% line+branch coverage.
The secret machinery had spread across three modules: the wrapper in secret.py, source refs and resolution in config.py, and the register-for-scrubbing dance inline in main.py's closure. It now lives in secrets.py as one readable lifecycle - wrapper, EnvSecret/FileSecret refs with resolve()/describe() methods, the Secrets bundle, and resolve_and_register wiring resolved values into log scrubbing. The config module goes back to doing only what its docstring promises: parsing. No behavior changes except one deliberate ordering choice, now pinned: when both password sources are broken, the failure names the primary (input) password's problem first. Considered and rejected before writing any of this: pydantic's SecretStr (a compiled dependency for one masked-repr class), pydantic-settings (a config-layer rewrite whose secrets_dir convention - field-named files in a fixed directory - is a different contract from PDFOPS_PASSWORD_FILE pointing at any mounted path, plus a ValidationError-to-error_code translation layer roughly the size of the parser it replaces), and scanner-style log redactors (pattern heuristics, strictly weaker than the exact-value field-restricted scrub that exists precisely because naive scrubbing becomes a password oracle). Recorded as D-024.
scripts/benchmark.py generates large fixtures (incompressible random bytes, never committed) and runs each scenario through the real container image, reporting the operation's peak RSS and the cgroup memory peak Kubernetes actually meters. The headline: peak process memory is linear in total input bytes - roughly input size plus ~40 MB of fixed overhead, indifferent to file count (2 x 250 MB and 20 x 25 MB profile identically), because the writer holds copied stream data until save() completes. Crypto costs time (~1 s per 250 MB), not memory. The cgroup peak runs 2-3x the RSS purely from reclaimable page cache, so limits are sized against RSS: total expected input + 128 MB, now documented in OPERATIONS.md with the measured table. The design doc's unmeasured-resources limitation becomes a measured, linear profile.
The image becomes 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 AND removes stdlib ensurepip - its bundled wheel would restore pip in one command - and runs as fixed non-root UID 10001. The posture is proven, not promised: one container test runs the golden merge under --read-only --cap-drop ALL --security-opt no-new-privileges (all writes land in the output mount by design), and another probes BOTH interpreters for pip and ensurepip - the venv python cannot see base site-packages, so a venv-only probe would stay green even with the uninstall reverted. deploy/argo-example.yaml ships the full WorkflowTemplate: the security context, fsGroup for output writability, the password as a mounted secret file, memory sized by the measured input+128MB rule, and a retry expression covering exit 1 plus pod-level errors - an Error-phase node (the lost-pod case) never produces an exit code, Argo substitutes -1, so the common exit-code-only expression silently skips exactly the scenario the ON_EXISTS=skip pairing exists for. The README snippet is fixed the same way. Recorded as D-025.
…pen decisions settled A walk of the failure taxonomy against the test suite found exactly one error code without a pinning test: UNSUPPORTED_ENCRYPTION, the certificate-security-handler path. A raw fixture with an /Adobe.PubSec encryption dictionary now pins it - exit 5, password class, never corrupt and never a retryable internal error. A fresh-clone walkthrough surfaced the one README friction: the quick start assumed the reader had PDFs at hand. It now opens with a snippet that conjures every input the examples use (and the encrypted example references a file the snippet actually creates). The three open decisions in the register settle as-is, each with its revisit trigger evaluated rather than waved off: the colon separator survived the container suite and the deployment example (D-007), operation values come from templates so case tolerance adds surface without value (D-008), and the deployment example injects no foreign PDFOPS_* variables, so hard unknown-var rejection keeps its typo protection (D-009). The register now carries no open decisions.
Four diagrams as self-contained interactive HTML, each authored from a typed JSON source kept beside it and validated before render: - pdf-ops-architecture: the module map, every node linking to the exact source lines it describes, pinned to a public commit - retry-machine: the run lifecycle as a state machine - the skip short-circuit, the fail refusal, and the pod-lost -> cleanup -> retry loop - extract-trust: the path an attacker-controlled attachment name travels (sanitizer, casefolded dedupe, containment recheck) with the payload bytes bypassing path handling entirely - password-flow: the call sequence for lazy secret resolution, the supplied-password try, and the spec-standard empty fallback index.html unites them behind a tab bar (hash-anchored, keys 1-4) using relative frames, so it always shows the latest delivered version of each diagram. The design doc and the operator guide deep-link into the right tab.
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.
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.
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.
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.
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.
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.
…e 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.
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.
The counts line still said 23 decided after four decisions landed - it is 27. The config_loaded row now says plainly that the two output fields appear only on merge runs; an extract event carries neither key. The register scripts' comments described the tooling checkout they were originally copied from; they now describe only the in-repo layout.
Owner
Author
|
Architecture delta, base main vs this branch (from the authored diagram sources): 0 components added or removed, 2 changed - the engine node (pypdf, pure-Python, no repair path -> pikepdf, qpdf-backed taxonomy translation) and the secrets node (wrapper with resolution spread into config.py -> one consolidated module). Connections, boundaries, and the operation topology are unchanged, which is the point of the seam: the whole swap plus the secrets cleanup alters exactly two boxes on the map. The interactive map itself is in docs/diagrams/index.html on this branch. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Executes the engine plan recorded in D-002 (see docs/DECISIONS.md, now D-023): the runtime engine moves from pypdf to pikepdf (qpdf-backed), behind the unchanged PdfEngine Protocol - a single-module replacement, which was the point of the seam. pypdf drops to the dev group, where it still builds every test fixture and independently verifies outputs, so each green test is a two-library cross-check.
What changes for an operator
Hostile-input hardening
qpdf hands malformed PDF structures over as native Python values, so the direct /Names/EmbeddedFiles walk (needed because Pdf.attachments collapses duplicate names) guards node types and reference cycles, classifies structural garbage as a data problem (exit 4) rather than a retryable internal error, refuses to turn an integer name-tree key into an attacker-sized bytes() allocation (the payload still extracts under the deterministic fallback name), caps the logged original_name, and bounds every token match in the failed-open /Encrypt scan so decoy digit walls neither crash int() nor mislabel.
The merged output is saved through the atomic-write layer's already-open temp file rather than by path - handed a path to an existing file, pikepdf routes through its own second hidden temp, whose post-kill debris the stale-temp cleanup could never match.
Second commit: one home for secrets
The secret machinery had spread across three modules (wrapper in secret.py, source refs and resolution in config.py, scrub registration inline in main.py). It now lives in src/pdf_ops/secrets.py as one readable lifecycle: the Secret wrapper, EnvSecret/FileSecret refs with resolve()/describe() methods, and resolve_and_register wiring resolved values into log scrubbing. config.py shrinks ~100 lines back to pure parsing; resolution stays deliberately lazy (merge's skip short-circuit reads nothing, not even the mounted password file).
Why not a library (recorded as D-024): pydantic's SecretStr means a compiled dependency for one masked-repr class; pydantic-settings would rewrite the config layer, and its secrets_dir convention (field-named files in a fixed directory) is a different contract from PDFOPS_PASSWORD_FILE=, plus its ValidationErrors would need retranslating into the error_code taxonomy; scanner-style log redactors are pattern heuristics, strictly weaker than the exact-value field-restricted scrub this codebase uses precisely because naive scrubbing becomes a password oracle. One deliberate, pinned ordering choice: when both password sources are broken, the failure event names the primary (input) password's problem first.
Tests
270 local + 5 container tests, 95% line+branch coverage. Corruption fixtures became genuinely unrecoverable (qpdf repairs the old ones - that upgrade is pinned as its own test), and new pins cover malformed tree shapes, integer name keys, UTF-16/PDFDoc name fidelity, write-time repair warnings, and the failed-open label scan.
Design notes: docs/DESIGN.md (library section), docs/DESIGN_NOTES.md section 1 and 8, docs/DECISIONS.md D-023 and D-024.
Third commit: measured resource behavior
scripts/benchmark.py generates large fixtures (never committed) and measures each scenario in the real container: peak process memory is linear in total input bytes - roughly input size plus ~40 MB fixed overhead, indifferent to file count - crypto costs time (~1 s per 250 MB) rather than memory, and the cgroup peak runs 2-3x the RSS purely from reclaimable page cache. Practical sizing (requests/limits = total expected input + 128 MB) and the measured table are in docs/OPERATIONS.md; the design doc's unmeasured-resources limitation is now a measured, linear profile.
Fourth commit: hardened runtime image and a deployable Argo example
The image becomes a two-stage build - a digest-pinned uv stage resolving the lockfile into a self-contained virtualenv with compiled bytecode, and a digest-pinned python:3.14-slim runtime carrying only that venv, with the base image's pip uninstalled and stdlib ensurepip removed (its bundled wheel would restore pip in one command). The posture is proven by tests, not promised: the golden merge runs under --read-only --cap-drop ALL --security-opt no-new-privileges, and an installer probe checks BOTH interpreters (the venv python cannot see base site-packages, so a venv-only probe would be vacuous). deploy/argo-example.yaml ships the full WorkflowTemplate: security context, fsGroup, secret-mounted password, memory per the measured sizing rule, and a retry expression that also covers pod-level Error nodes - a lost pod never produces an exit code (Argo substitutes -1), so the common exit-code-only expression silently skips exactly the lost-pod scenario the ON_EXISTS=skip pairing exists for; the README snippet is fixed the same way. Recorded as D-025. Container suite is now 7 tests.
Fifth commit: audit closure
A taxonomy-to-test walk found one error code without a pin (UNSUPPORTED_ENCRYPTION, the certificate-security-handler path) - now covered by a raw /Adobe.PubSec fixture asserting exit 5. A fresh-clone walkthrough surfaced the README's one friction point - the quick start now opens with a snippet that creates every input the examples use. And the register's three open decisions settle as-is with their revisit triggers evaluated: the colon separator, strict lowercase operations, and hard unknown-var rejection all stand. No open decisions remain; 271 local + 7 container tests.
Sixth commit: interactive system diagrams
Four self-contained interactive HTML diagrams in docs/diagrams, each compiled from a typed JSON source that lives beside it: the module architecture (every node links to the exact source lines it describes, pinned to a commit), the run/retry state machine, the extract trust boundary, and the password flow. index.html unites them behind a hash-anchored tab bar. The design doc and operator guide deep-link into the right tab.
Follow-up commits: toolchain coherence, typed vocabularies, docs exactness
uv run --locked, so hooks, CI and a developer's shell resolve the single pinned version. CI gets a read-only token, per-ref concurrency, timeouts, SHA-pinned actions anduv sync --locked. The bare "scripts" ruff exclude had silently left docs/scripts - including the CI-run decision validator - unlinted; both script directories join the gates and the nine findings from the widened rule set are fixed. Dependabot watches the three pinned surfaces: lockfile, actions, image digests.Tests: 280 local + 7 container. Register: 29 decisions, none open.