From 967160c705f8a3c985110bddd2586172d1f042f3 Mon Sep 17 00:00:00 2001 From: mberrys Date: Sun, 13 Sep 2026 23:50:35 -0700 Subject: [PATCH 01/26] feat(core): reject the undeclared save-policy fallback as a declaration (#239) --- LoopLibCore/sources/pdfrepairoperation.h | 5 ++-- LoopLibCore/sources/pdfrepairprimitives.cpp | 1 + LoopLibCore/sources/pdfsavepolicy.cpp | 21 +++++++++++++--- LoopLibCore/sources/pdfsavepolicy.h | 17 ++++++++++--- UnitTests/tst_repairoperationtest.cpp | 27 +++++++++++++++++++++ 5 files changed, 62 insertions(+), 9 deletions(-) diff --git a/LoopLibCore/sources/pdfrepairoperation.h b/LoopLibCore/sources/pdfrepairoperation.h index 68d608b76..886fe296a 100644 --- a/LoopLibCore/sources/pdfrepairoperation.h +++ b/LoopLibCore/sources/pdfrepairoperation.h @@ -192,10 +192,11 @@ class LOOPLIBCORESHARED_EXPORT PDFRepairOperation virtual PDFRepairDomains domains() const = 0; /// Declares the serialization and signature consequences of this operation. /// The conservative default prevents an unclassified operation from being - /// appended to a signed or revisioned source. + /// appended to a signed or revisioned source. A registered operation must + /// override this; the registry-wide test rejects the undeclared default. virtual PDFOperationSavePolicy savePolicy() const { - return PDFOperationSavePolicy::saveAsNewArtifact(QStringLiteral("operation did not declare a save policy")); + return PDFOperationSavePolicy::undeclared(); } /// Unknown or incomplete impact forces full revalidation. virtual PDFOperationImpact impact() const diff --git a/LoopLibCore/sources/pdfrepairprimitives.cpp b/LoopLibCore/sources/pdfrepairprimitives.cpp index eaa85781b..a3f8df387 100644 --- a/LoopLibCore/sources/pdfrepairprimitives.cpp +++ b/LoopLibCore/sources/pdfrepairprimitives.cpp @@ -521,6 +521,7 @@ class PDFStandardConversionRepair final : public PDFRepairOperation { return PDFRepairDomain::Color | PDFRepairDomain::Fonts | PDFRepairDomain::Images | PDFRepairDomain::Metadata | PDFRepairDomain::PageGeometry | PDFRepairDomain::Structure; } + PDFOperationSavePolicy savePolicy() const override { return PDFOperationSavePolicy::fullRewrite(QStringLiteral("standard conversion removes prior content")); } PDFOperationImpact impact(const PDFDocument*, const QJsonObject&) const override { PDFOperationImpact declared; diff --git a/LoopLibCore/sources/pdfsavepolicy.cpp b/LoopLibCore/sources/pdfsavepolicy.cpp index 333360479..9488cbb69 100644 --- a/LoopLibCore/sources/pdfsavepolicy.cpp +++ b/LoopLibCore/sources/pdfsavepolicy.cpp @@ -31,9 +31,12 @@ const char* getPDFSaveModeName(PDFSaveMode mode) { switch (mode) { - case PDFSaveMode::IncrementalAppend: return "incremental-append"; - case PDFSaveMode::FullRewrite: return "full-rewrite"; - case PDFSaveMode::SaveAsNewArtifact: return "save-as-new-artifact"; + case PDFSaveMode::IncrementalAppend: + return "incremental-append"; + case PDFSaveMode::FullRewrite: + return "full-rewrite"; + case PDFSaveMode::SaveAsNewArtifact: + return "save-as-new-artifact"; } return "unknown"; } @@ -66,6 +69,16 @@ PDFOperationSavePolicy PDFOperationSavePolicy::saveAsNewArtifact(QString rationa return policy; } +PDFOperationSavePolicy PDFOperationSavePolicy::undeclared() +{ + return PDFOperationSavePolicy::saveAsNewArtifact(QStringLiteral("operation did not declare a save policy")); +} + +bool PDFOperationSavePolicy::isUndeclared() const +{ + return rationale == undeclared().rationale; +} + QJsonObject PDFOperationSavePolicy::toJson() const { return QJsonObject{ @@ -97,4 +110,4 @@ PDFOperationSavePolicy mergePDFSavePolicies(const PDFOperationSavePolicy& first, return result; } -} // namespace pdf +} // namespace pdf diff --git a/LoopLibCore/sources/pdfsavepolicy.h b/LoopLibCore/sources/pdfsavepolicy.h index 00340e860..ee87b3b5b 100644 --- a/LoopLibCore/sources/pdfsavepolicy.h +++ b/LoopLibCore/sources/pdfsavepolicy.h @@ -50,12 +50,23 @@ struct LOOPLIBCORESHARED_EXPORT PDFOperationSavePolicy static PDFOperationSavePolicy saveAsNewArtifact(QString rationale = {}); QJsonObject toJson() const; + + /// True when nobody declared this policy. The conservative default keeps + /// an unclassified operation from being appended, but it is not a + /// declaration: the registry-wide test rejects it, so a forgotten override + /// is a visible defect instead of a silent fallback. + bool isUndeclared() const; + + /// The conservative, explicitly-undeclared policy. Single source of the + /// default; `PDFRepairOperation::savePolicy()` and `isUndeclared()` both + /// resolve to it. + static PDFOperationSavePolicy undeclared(); }; LOOPLIBCORESHARED_EXPORT const char* getPDFSaveModeName(PDFSaveMode mode); LOOPLIBCORESHARED_EXPORT PDFOperationSavePolicy mergePDFSavePolicies(const PDFOperationSavePolicy& first, - const PDFOperationSavePolicy& second); + const PDFOperationSavePolicy& second); -} // namespace pdf +} // namespace pdf -#endif // PDFSAVEPOLICY_H +#endif // PDFSAVEPOLICY_H diff --git a/UnitTests/tst_repairoperationtest.cpp b/UnitTests/tst_repairoperationtest.cpp index ee10a62db..13cc20b21 100644 --- a/UnitTests/tst_repairoperationtest.cpp +++ b/UnitTests/tst_repairoperationtest.cpp @@ -61,6 +61,7 @@ class RepairOperationTest : public QObject private slots: void builtInOperations_areRegistered(); void builtInOperations_declareSavePolicies(); + void everyRegisteredOperationDeclaresItsSavePolicy(); void analyze_doesNotMutateSource(); void unsupportedPrecondition_preventsApply(); void failedOperation_discardsCandidate(); @@ -118,6 +119,32 @@ void RepairOperationTest::builtInOperations_declareSavePolicies() QVERIFY(transaction.savePolicy().invalidatesSignatures); } +void RepairOperationTest::everyRegisteredOperationDeclaresItsSavePolicy() +{ + const pdf::PDFRepairRegistry& registry = pdf::PDFRepairRegistry::instance(); + const QStringList ids = registry.operationIds(); + QVERIFY2(ids.size() >= 7, qPrintable(QString::number(ids.size()))); + for (const QString& id : ids) + { + const pdf::PDFRepairOperation* operation = registry.find(id); + QVERIFY2(operation != nullptr, qPrintable(id)); + const pdf::PDFOperationSavePolicy policy = operation->savePolicy(); + QVERIFY2(!policy.isUndeclared(), qPrintable(id)); + QVERIFY2(!policy.rationale.isEmpty(), qPrintable(id)); + const QJsonObject descriptor = operation->descriptor(); + QCOMPARE(descriptor.value(QStringLiteral("save_policy")).toObject().value(QStringLiteral("mode")).toString(), + QString::fromLatin1(pdf::getPDFSaveModeName(policy.mode))); + } + + // The conservative default is safe for the writer but it is not a + // declaration, and it must never be reported as one. + const pdf::PDFOperationSavePolicy undeclared = pdf::PDFOperationSavePolicy::undeclared(); + QVERIFY(undeclared.isUndeclared()); + QCOMPARE(QString::fromLatin1(pdf::getPDFSaveModeName(undeclared.mode)), QStringLiteral("save-as-new-artifact")); + QVERIFY(undeclared.invalidatesSignatures); + QVERIFY(!pdf::PDFOperationSavePolicy::incrementalAppend(QStringLiteral("ordinary edit")).isUndeclared()); +} + void RepairOperationTest::analyze_doesNotMutateSource() { pdf::PDFDocumentBuilder builder; From 366a97229b95f90155a6a7efd9eb0719e26609d9 Mon Sep 17 00:00:00 2001 From: mberrys Date: Sun, 13 Sep 2026 23:50:35 -0700 Subject: [PATCH 02/26] chore(policy): run the save-policy suites in the core gate lane (#239) --- agent-policy.json | 2 ++ 1 file changed, 2 insertions(+) diff --git a/agent-policy.json b/agent-policy.json index 66bf5afe8..5e0aed3e2 100644 --- a/agent-policy.json +++ b/agent-policy.json @@ -126,6 +126,8 @@ "UnitTestsPreflightProfileResolver", "UnitTestsPreflightVerdict", "UnitTestsProcessingBudget", + "UnitTestsRedactVerifier", + "UnitTestsRepairOperation", "UnitTestsRevisionStress", "UnitTestsSafeFileWriter", "UnitTestsSchemaEvolution", From 3e217848f6a4c82b746009dc8fdaa6ec90da25be Mon Sep 17 00:00:00 2001 From: mberrys Date: Sun, 13 Sep 2026 23:59:27 -0700 Subject: [PATCH 03/26] feat(core): add save-policy strength comparison and one refusal message (#239) --- LoopLibCore/sources/pdfsavepolicy.cpp | 47 +++++++++++++++++++++++++++ LoopLibCore/sources/pdfsavepolicy.h | 13 ++++++++ UnitTests/tst_incrementalsavetest.cpp | 42 ++++++++++++++++++++++++ 3 files changed, 102 insertions(+) diff --git a/LoopLibCore/sources/pdfsavepolicy.cpp b/LoopLibCore/sources/pdfsavepolicy.cpp index 9488cbb69..70c03b973 100644 --- a/LoopLibCore/sources/pdfsavepolicy.cpp +++ b/LoopLibCore/sources/pdfsavepolicy.cpp @@ -24,6 +24,8 @@ #include +#include + namespace pdf { @@ -110,4 +112,49 @@ PDFOperationSavePolicy mergePDFSavePolicies(const PDFOperationSavePolicy& first, return result; } +bool savePolicyIsWeaker(const PDFOperationSavePolicy& candidate, const PDFOperationSavePolicy& required) +{ + if (static_cast(candidate.mode) < static_cast(required.mode)) + { + return true; + } + // A stronger mode is never weaker, whatever it claims about signatures and + // reversibility: only a same-mode request can understate those. + if (candidate.mode != required.mode) + { + return false; + } + if (required.invalidatesSignatures && !candidate.invalidatesSignatures) + { + return true; + } + return !required.reversibleInSession && candidate.reversibleInSession; +} + +QString savePolicyWeakenedMessage(const PDFOperationSavePolicy& candidate, + const PDFOperationSavePolicy& required) +{ + const bool sameMode = candidate.mode == required.mode; + QStringList reasons; + if (static_cast(candidate.mode) < static_cast(required.mode)) + { + reasons.append(QStringLiteral("mode '%1' is weaker than the operation-declared '%2'") + .arg(QString::fromLatin1(getPDFSaveModeName(candidate.mode)), + QString::fromLatin1(getPDFSaveModeName(required.mode)))); + } + if (sameMode && required.invalidatesSignatures && !candidate.invalidatesSignatures) + { + reasons.append(QStringLiteral("signature invalidation is not declared but the operation invalidates signatures")); + } + if (sameMode && !required.reversibleInSession && candidate.reversibleInSession) + { + reasons.append(QStringLiteral("session reversibility is claimed but the operation is not reversible")); + } + if (reasons.isEmpty()) + { + return {}; + } + return QStringLiteral("Refused save policy: %1.").arg(reasons.join(QStringLiteral("; "))); +} + } // namespace pdf diff --git a/LoopLibCore/sources/pdfsavepolicy.h b/LoopLibCore/sources/pdfsavepolicy.h index ee87b3b5b..38a44e436 100644 --- a/LoopLibCore/sources/pdfsavepolicy.h +++ b/LoopLibCore/sources/pdfsavepolicy.h @@ -67,6 +67,19 @@ LOOPLIBCORESHARED_EXPORT const char* getPDFSaveModeName(PDFSaveMode mode); LOOPLIBCORESHARED_EXPORT PDFOperationSavePolicy mergePDFSavePolicies(const PDFOperationSavePolicy& first, const PDFOperationSavePolicy& second); +/// True when \p candidate asks for less persistence safety than \p required: +/// a weaker mode, or - at the same mode - an unstated signature loss or a +/// claimed reversibility the operation does not have. Stricter policies are +/// allowed. +LOOPLIBCORESHARED_EXPORT bool savePolicyIsWeaker(const PDFOperationSavePolicy& candidate, + const PDFOperationSavePolicy& required); + +/// The single refusal message for a weakened request, naming every weakening +/// reason the predicate finds. Empty when the request is not weaker, so +/// callers can use it as both the reason and the predicate. +LOOPLIBCORESHARED_EXPORT QString savePolicyWeakenedMessage(const PDFOperationSavePolicy& candidate, + const PDFOperationSavePolicy& required); + } // namespace pdf #endif // PDFSAVEPOLICY_H diff --git a/UnitTests/tst_incrementalsavetest.cpp b/UnitTests/tst_incrementalsavetest.cpp index 8f7b8b19d..50d03132e 100644 --- a/UnitTests/tst_incrementalsavetest.cpp +++ b/UnitTests/tst_incrementalsavetest.cpp @@ -23,6 +23,7 @@ #include "pdfdocumentbuilder.h" #include "pdfdocumentreader.h" #include "pdfdocumentwriter.h" +#include "pdfrepairoperation.h" #include #include @@ -69,6 +70,7 @@ private slots: void signedPdfIncrementalSave_preservesSignedPrefix(); void explicitPoliciesCannotBeDowngradedToIncremental(); void unclassifiedAndRedactionPoliciesCannotSilentIncrementalAppend(); + void policyStrengthRejectsWeakerRequests(); void fileOverloadReportsWhatItDid(); }; @@ -335,6 +337,46 @@ void IncrementalSaveTest::unclassifiedAndRedactionPoliciesCannotSilentIncrementa QCOMPARE(mergedUnclassified.mode, pdf::PDFSaveMode::SaveAsNewArtifact); } +void IncrementalSaveTest::policyStrengthRejectsWeakerRequests() +{ + const pdf::PDFOperationSavePolicy incremental = pdf::PDFOperationSavePolicy::incrementalAppend(QStringLiteral("ordinary edit")); + const pdf::PDFOperationSavePolicy full = pdf::PDFOperationSavePolicy::fullRewrite(QStringLiteral("redaction")); + const pdf::PDFOperationSavePolicy newArtifact = pdf::PDFOperationSavePolicy::saveAsNewArtifact(QStringLiteral("production correction")); + + QVERIFY(pdf::savePolicyIsWeaker(incremental, full)); + QVERIFY(pdf::savePolicyIsWeaker(full, newArtifact)); + QVERIFY(!pdf::savePolicyIsWeaker(newArtifact, full)); + QVERIFY(!pdf::savePolicyIsWeaker(full, full)); + QVERIFY(!pdf::savePolicyIsWeaker(incremental, incremental)); + + // Same mode, hidden consequence: a caller may not claim less impact than + // the operation declares. + pdf::PDFOperationSavePolicy hidesSignatureLoss = pdf::PDFOperationSavePolicy::fullRewrite(QStringLiteral("caller copy")); + hidesSignatureLoss.invalidatesSignatures = false; + QVERIFY(pdf::savePolicyIsWeaker(hidesSignatureLoss, full)); + pdf::PDFOperationSavePolicy claimsReversible = pdf::PDFOperationSavePolicy::fullRewrite(QStringLiteral("caller copy")); + claimsReversible.reversibleInSession = true; + QVERIFY(pdf::savePolicyIsWeaker(claimsReversible, full)); + + // Stricter than required is allowed. + QVERIFY(!pdf::savePolicyIsWeaker(newArtifact, incremental)); + + // The undeclared default is never weaker than any declared policy, so it + // can stay the transaction default without changing behaviour. + for (const QString& id : pdf::PDFRepairRegistry::instance().operationIds()) + { + QVERIFY2(!pdf::savePolicyIsWeaker(pdf::PDFOperationSavePolicy::undeclared(), + pdf::PDFRepairRegistry::instance().find(id)->savePolicy()), + qPrintable(id)); + } + + QCOMPARE(pdf::savePolicyWeakenedMessage(incremental, full), + QStringLiteral("Refused save policy: mode 'incremental-append' is weaker than the operation-declared 'full-rewrite'.")); + QCOMPARE(pdf::savePolicyWeakenedMessage(hidesSignatureLoss, full), + QStringLiteral("Refused save policy: signature invalidation is not declared but the operation invalidates signatures.")); + QVERIFY(pdf::savePolicyWeakenedMessage(newArtifact, incremental).isEmpty()); +} + void IncrementalSaveTest::fileOverloadReportsWhatItDid() { const QByteArray originalData = writeDocument(createDocument()); From d0e23dcdc1ec3106b17ef077b181b70066b1de95 Mon Sep 17 00:00:00 2001 From: mberrys Date: Mon, 14 Sep 2026 00:03:13 -0700 Subject: [PATCH 04/26] feat(core): refuse a weakened save policy at the transaction boundary (#239) --- LoopLibCore/sources/pdfrepairoperation.cpp | 41 ++++++++++++++++++++++ LoopLibCore/sources/pdfrepairoperation.h | 12 +++++++ UnitTests/tst_repairoperationtest.cpp | 28 +++++++++++++++ 3 files changed, 81 insertions(+) diff --git a/LoopLibCore/sources/pdfrepairoperation.cpp b/LoopLibCore/sources/pdfrepairoperation.cpp index ce9fe815b..389001e00 100644 --- a/LoopLibCore/sources/pdfrepairoperation.cpp +++ b/LoopLibCore/sources/pdfrepairoperation.cpp @@ -373,6 +373,13 @@ PDFOperationResult PDFRepairTransaction::add(const PDFRepairOperation* operation PDFOperationResult PDFRepairTransaction::analyze() { + const PDFOperationResult savePolicyRefusal = refuseWeakenedSavePolicy(); + if (!savePolicyRefusal) + { + m_status = PDFRepairStatus::Failed; + return savePolicyRefusal; + } + m_plans.clear(); m_results.clear(); m_analyzed = true; @@ -416,6 +423,13 @@ PDFOperationResult PDFRepairTransaction::analyze() PDFOperationResult PDFRepairTransaction::apply() { + const PDFOperationResult savePolicyRefusal = refuseWeakenedSavePolicy(); + if (!savePolicyRefusal) + { + m_status = PDFRepairStatus::Failed; + return savePolicyRefusal; + } + if (!m_analyzed) { const PDFOperationResult analysisResult = analyze(); @@ -505,6 +519,33 @@ PDFOperationSavePolicy PDFRepairTransaction::savePolicy() const return result; } +PDFOperationResult PDFRepairTransaction::refuseWeakenedSavePolicy() const +{ + if (m_savePolicyRefused || + (m_hasRequestedSavePolicy && savePolicyIsWeaker(m_requestedSavePolicy, savePolicy()))) + { + return PDFOperationResult(savePolicyWeakenedMessage(m_requestedSavePolicy, savePolicy())); + } + return PDFOperationResult(true); +} + +PDFOperationResult PDFRepairTransaction::setRequestedSavePolicy(const PDFOperationSavePolicy& policy) +{ + const PDFOperationSavePolicy required = savePolicy(); + if (savePolicyIsWeaker(policy, required)) + { + // The refused request is kept only so every later refusal names the + // same request; the effective policy stays the declared one. + m_requestedSavePolicy = policy; + m_savePolicyRefused = true; + m_status = PDFRepairStatus::Failed; + return PDFOperationResult(savePolicyWeakenedMessage(policy, required)); + } + m_requestedSavePolicy = policy; + m_hasRequestedSavePolicy = true; + return PDFOperationResult(true); +} + PDFRepairExpectedChanges PDFRepairTransaction::expectedChanges() const { PDFRepairExpectedChanges expected; diff --git a/LoopLibCore/sources/pdfrepairoperation.h b/LoopLibCore/sources/pdfrepairoperation.h index 886fe296a..35d97d12c 100644 --- a/LoopLibCore/sources/pdfrepairoperation.h +++ b/LoopLibCore/sources/pdfrepairoperation.h @@ -287,6 +287,11 @@ class LOOPLIBCORESHARED_EXPORT PDFRepairTransaction const QList& plans() const { return m_plans; } const QList& results() const { return m_results; } PDFOperationSavePolicy savePolicy() const; + /// Declares the save policy the caller is asking for. Stricter than the + /// operation-declared policy is allowed; weaker is refused here, before any + /// analysis or mutation, so a surface cannot talk an operation out of its + /// persistence requirement. + PDFOperationResult setRequestedSavePolicy(const PDFOperationSavePolicy& policy); PDFRepairStatus status() const { return m_status; } private: @@ -298,6 +303,10 @@ class LOOPLIBCORESHARED_EXPORT PDFRepairTransaction PDFRepairExpectedChanges expectedChanges() const; QVector affectedPages() const; + /// The single refusal check shared by analyze(), apply() and + /// serializeCandidate(): a refused request stays refused for the life of + /// the transaction, so a caller cannot retry past it. + PDFOperationResult refuseWeakenedSavePolicy() const; const PDFDocument* m_source = nullptr; PDFRepairTransactionOptions m_options; @@ -308,6 +317,9 @@ class LOOPLIBCORESHARED_EXPORT PDFRepairTransaction PDFRepairStatus m_status = PDFRepairStatus::Planned; bool m_analyzed = false; bool m_hasCandidate = false; + bool m_hasRequestedSavePolicy = false; + bool m_savePolicyRefused = false; + PDFOperationSavePolicy m_requestedSavePolicy; }; QString pdfRepairStatusName(PDFRepairStatus status); diff --git a/UnitTests/tst_repairoperationtest.cpp b/UnitTests/tst_repairoperationtest.cpp index 13cc20b21..e39449c64 100644 --- a/UnitTests/tst_repairoperationtest.cpp +++ b/UnitTests/tst_repairoperationtest.cpp @@ -62,6 +62,7 @@ private slots: void builtInOperations_areRegistered(); void builtInOperations_declareSavePolicies(); void everyRegisteredOperationDeclaresItsSavePolicy(); + void transactionRejectsAWeakenedSavePolicyBeforeMutation(); void analyze_doesNotMutateSource(); void unsupportedPrecondition_preventsApply(); void failedOperation_discardsCandidate(); @@ -145,6 +146,33 @@ void RepairOperationTest::everyRegisteredOperationDeclaresItsSavePolicy() QVERIFY(!pdf::PDFOperationSavePolicy::incrementalAppend(QStringLiteral("ordinary edit")).isUndeclared()); } +void RepairOperationTest::transactionRejectsAWeakenedSavePolicyBeforeMutation() +{ + pdf::PDFDocumentBuilder builder; + builder.appendPage(QRectF(0, 0, 100, 100)); + const pdf::PDFDocument source = builder.build(); + + pdf::PDFRepairTransaction transaction(source); + QVERIFY(transaction.add(pdf::PDFRepairRegistry::instance().find(QStringLiteral("add-bleed")), + QJsonObject{ { QStringLiteral("bleed_mm"), 3.0 }, + { QStringLiteral("force"), true } })); + // add-bleed declares save-as-new-artifact; asking for an append is weaker. + const pdf::PDFOperationResult weakened = transaction.setRequestedSavePolicy( + pdf::PDFOperationSavePolicy::incrementalAppend(QStringLiteral("caller wants an append"))); + QVERIFY(!weakened); + QCOMPARE(weakened.getErrorMessage(), + QStringLiteral("Refused save policy: mode 'incremental-append' is weaker than the operation-declared 'save-as-new-artifact'.")); + QCOMPARE(transaction.status(), pdf::PDFRepairStatus::Failed); + QVERIFY(!transaction.analyze()); + QVERIFY(!transaction.apply()); + + // Stricter than declared is accepted and does not change the declared policy. + pdf::PDFRepairTransaction stricter(source); + QVERIFY(stricter.add(pdf::PDFRepairRegistry::instance().find(QStringLiteral("production.validate-wide-format")), QJsonObject{})); + QVERIFY(stricter.setRequestedSavePolicy(pdf::PDFOperationSavePolicy::fullRewrite(QStringLiteral("caller wants a rewrite")))); + QCOMPARE(stricter.savePolicy().mode, pdf::PDFSaveMode::IncrementalAppend); +} + void RepairOperationTest::analyze_doesNotMutateSource() { pdf::PDFDocumentBuilder builder; From 14f7577501794ca320c530d7aaa998dcee5e6abb Mon Sep 17 00:00:00 2001 From: mberrys Date: Mon, 14 Sep 2026 00:04:24 -0700 Subject: [PATCH 05/26] feat(core): enforce the effective save policy on the candidate write path (#239) --- LoopLibCore/sources/pdfrepairoperation.cpp | 5 +++++ UnitTests/tst_repairoperationtest.cpp | 19 +++++++++++++++++++ 2 files changed, 24 insertions(+) diff --git a/LoopLibCore/sources/pdfrepairoperation.cpp b/LoopLibCore/sources/pdfrepairoperation.cpp index 389001e00..b6f6a667b 100644 --- a/LoopLibCore/sources/pdfrepairoperation.cpp +++ b/LoopLibCore/sources/pdfrepairoperation.cpp @@ -500,6 +500,11 @@ PDFOperationResult PDFRepairTransaction::serializeCandidate(const QString& candi { return PDFOperationResult(QStringLiteral("Repair transaction has no candidate.")); } + const PDFOperationResult savePolicyRefusal = refuseWeakenedSavePolicy(); + if (!savePolicyRefusal) + { + return savePolicyRefusal; + } return PDFRepairDiffEngine::buildSerializedCandidate( m_candidate, [](PDFDocument*) diff --git a/UnitTests/tst_repairoperationtest.cpp b/UnitTests/tst_repairoperationtest.cpp index e39449c64..bd86d4db1 100644 --- a/UnitTests/tst_repairoperationtest.cpp +++ b/UnitTests/tst_repairoperationtest.cpp @@ -171,6 +171,25 @@ void RepairOperationTest::transactionRejectsAWeakenedSavePolicyBeforeMutation() QVERIFY(stricter.add(pdf::PDFRepairRegistry::instance().find(QStringLiteral("production.validate-wide-format")), QJsonObject{})); QVERIFY(stricter.setRequestedSavePolicy(pdf::PDFOperationSavePolicy::fullRewrite(QStringLiteral("caller wants a rewrite")))); QCOMPARE(stricter.savePolicy().mode, pdf::PDFSaveMode::IncrementalAppend); + + // The candidate write path applies the same rule: a request refused after + // the mutation still cannot produce a candidate artifact. + pdf::PDFRepairTransaction written(source); + QVERIFY(written.add(pdf::PDFRepairRegistry::instance().find(QStringLiteral("add-bleed")), + QJsonObject{ { QStringLiteral("bleed_mm"), 3.0 }, + { QStringLiteral("force"), true } })); + QVERIFY(written.analyze()); + QVERIFY(written.apply()); + QVERIFY(!written.setRequestedSavePolicy(pdf::PDFOperationSavePolicy::incrementalAppend(QStringLiteral("caller wants an append")))); + + QTemporaryDir directory; + QVERIFY(directory.isValid()); + pdf::PDFDocument reopenedCandidate; + const pdf::PDFOperationResult serialized = written.serializeCandidate( + directory.filePath(QStringLiteral("candidate.pdf")), &reopenedCandidate); + QVERIFY(!serialized); + QCOMPARE(serialized.getErrorMessage(), + QStringLiteral("Refused save policy: mode 'incremental-append' is weaker than the operation-declared 'save-as-new-artifact'.")); } void RepairOperationTest::analyze_doesNotMutateSource() From 9a7177213c4172a61d5c7e8c3a7e05b4d01844a2 Mon Sep 17 00:00:00 2001 From: mberrys Date: Mon, 14 Sep 2026 00:20:01 -0700 Subject: [PATCH 06/26] feat(core): refuse candidate saves that would overwrite the trusted source (#239) --- LoopLibCore/sources/pdfdocumentwriter.cpp | 25 +++++ LoopLibCore/sources/pdfdocumentwriter.h | 6 ++ LoopLibCore/sources/pdfrepairoperation.cpp | 17 ++++ LoopLibCore/sources/pdfrepairoperation.h | 4 + LoopLibCore/sources/pdfsavepolicy.h | 13 +++ UnitTests/tst_repairoperationtest.cpp | 108 +++++++++++++++++++++ 6 files changed, 173 insertions(+) diff --git a/LoopLibCore/sources/pdfdocumentwriter.cpp b/LoopLibCore/sources/pdfdocumentwriter.cpp index aa7f63e6e..a9f996737 100644 --- a/LoopLibCore/sources/pdfdocumentwriter.cpp +++ b/LoopLibCore/sources/pdfdocumentwriter.cpp @@ -27,6 +27,7 @@ #include #include #include +#include #include #include "pdfdbgheap.h" @@ -865,4 +866,28 @@ QByteArray PDFDocumentWriter::getSerializedObject(const PDFObject& object) return buffer.data(); } +PDFOperationResult validateSaveRequest(const PDFSaveRequest& request) +{ + if (request.requestedExplicitly && savePolicyIsWeaker(request.requested, request.required)) + { + return PDFOperationResult(savePolicyWeakenedMessage(request.requested, request.required)); + } + + if (request.appendInPlace || request.sourcePath.isEmpty() || request.outputPath.isEmpty()) + { + return PDFOperationResult(true); + } + + const QFileInfo sourceInfo(request.sourcePath); + const QFileInfo outputInfo(request.outputPath); + if (sourceInfo.exists() && outputInfo.exists() && + sourceInfo.canonicalFilePath() == outputInfo.canonicalFilePath()) + { + return PDFOperationResult( + QStringLiteral("Refused save: '%1' is the trusted input artifact; write the candidate to a new path.") + .arg(sourceInfo.fileName())); + } + return PDFOperationResult(true); +} + } // namespace pdf diff --git a/LoopLibCore/sources/pdfdocumentwriter.h b/LoopLibCore/sources/pdfdocumentwriter.h index fcff360fe..c504b772d 100644 --- a/LoopLibCore/sources/pdfdocumentwriter.h +++ b/LoopLibCore/sources/pdfdocumentwriter.h @@ -169,6 +169,12 @@ class LOOPLIBCORESHARED_EXPORT PDFDocumentWriter const PDFOperationControl* m_operationControl = nullptr; }; +/// Refuses a save request that would weaken the operation-declared policy or +/// overwrite the trusted input artifact. Declared here because the writer is +/// the save boundary; implemented once so the transaction and every CLI write +/// path share the same rule. +LOOPLIBCORESHARED_EXPORT PDFOperationResult validateSaveRequest(const PDFSaveRequest& request); + } // namespace pdf #endif // PDFDOCUMENTWRITER_H diff --git a/LoopLibCore/sources/pdfrepairoperation.cpp b/LoopLibCore/sources/pdfrepairoperation.cpp index b6f6a667b..bd1030c6e 100644 --- a/LoopLibCore/sources/pdfrepairoperation.cpp +++ b/LoopLibCore/sources/pdfrepairoperation.cpp @@ -21,6 +21,7 @@ // SOFTWARE. #include "pdfrepairoperation.h" +#include "pdfdocumentwriter.h" #include #include @@ -505,6 +506,22 @@ PDFOperationResult PDFRepairTransaction::serializeCandidate(const QString& candi { return savePolicyRefusal; } + + const PDFOperationSavePolicy effective = + m_hasRequestedSavePolicy ? m_requestedSavePolicy : savePolicy(); + PDFSaveRequest request; + request.sourcePath = m_options.sourcePath; + request.outputPath = candidatePath; + request.required = savePolicy(); + request.requested = effective; + request.requestedExplicitly = m_hasRequestedSavePolicy; + request.appendInPlace = effective.mode == PDFSaveMode::IncrementalAppend; + const PDFOperationResult saveRequestRefusal = validateSaveRequest(request); + if (!saveRequestRefusal) + { + return saveRequestRefusal; + } + return PDFRepairDiffEngine::buildSerializedCandidate( m_candidate, [](PDFDocument*) diff --git a/LoopLibCore/sources/pdfrepairoperation.h b/LoopLibCore/sources/pdfrepairoperation.h index 35d97d12c..41abb39cf 100644 --- a/LoopLibCore/sources/pdfrepairoperation.h +++ b/LoopLibCore/sources/pdfrepairoperation.h @@ -263,6 +263,10 @@ struct LOOPLIBCORESHARED_EXPORT PDFRepairTransactionOptions bool failOnIncompleteValidation = true; int maxOperations = 100; const PDFOperationControl* operationControl = nullptr; + /// The trusted input the caller received. When set, a candidate write to + /// this path is refused; an empty value only disables that path check, not + /// the policy check. + QString sourcePath; }; class LOOPLIBCORESHARED_EXPORT PDFRepairTransaction diff --git a/LoopLibCore/sources/pdfsavepolicy.h b/LoopLibCore/sources/pdfsavepolicy.h index 38a44e436..c4732d457 100644 --- a/LoopLibCore/sources/pdfsavepolicy.h +++ b/LoopLibCore/sources/pdfsavepolicy.h @@ -80,6 +80,19 @@ LOOPLIBCORESHARED_EXPORT bool savePolicyIsWeaker(const PDFOperationSavePolicy& c LOOPLIBCORESHARED_EXPORT QString savePolicyWeakenedMessage(const PDFOperationSavePolicy& candidate, const PDFOperationSavePolicy& required); +/// Everything a save path needs to check before it touches the filesystem. +struct LOOPLIBCORESHARED_EXPORT PDFSaveRequest +{ + QString sourcePath; + QString outputPath; + PDFOperationSavePolicy required; + PDFOperationSavePolicy requested; + bool requestedExplicitly = false; + /// True only when the caller asked for an in-place incremental append, the + /// single case where writing over the source path is the intent. + bool appendInPlace = false; +}; + } // namespace pdf #endif // PDFSAVEPOLICY_H diff --git a/UnitTests/tst_repairoperationtest.cpp b/UnitTests/tst_repairoperationtest.cpp index bd86d4db1..0f4b17afe 100644 --- a/UnitTests/tst_repairoperationtest.cpp +++ b/UnitTests/tst_repairoperationtest.cpp @@ -21,9 +21,14 @@ // SOFTWARE. #include "pdfdocumentbuilder.h" +#include "pdfdocumentwriter.h" #include "pdfrepairoperation.h" #include "pdfstandardconversion.h" +#include +#include +#include +#include #include #include #include @@ -52,6 +57,17 @@ class FailingRepair final : public pdf::PDFRepairOperation } }; +/// Serializes \p document into the bytes a candidate write would produce, so a +/// slot can assert on what a save path did or did not touch. +QByteArray writeSerializedBytes(const pdf::PDFDocument& document) +{ + pdf::PDFDocumentWriter writer(nullptr); + QBuffer buffer; + buffer.open(QIODevice::WriteOnly); + const pdf::PDFOperationResult result = writer.write(&buffer, &document); + return result ? buffer.data() : QByteArray(); +} + } // namespace class RepairOperationTest : public QObject @@ -63,6 +79,8 @@ private slots: void builtInOperations_declareSavePolicies(); void everyRegisteredOperationDeclaresItsSavePolicy(); void transactionRejectsAWeakenedSavePolicyBeforeMutation(); + void saveRequestRefusesToWriteOverTheTrustedSource(); + void candidateSaveRefusesToOverwriteTheSourceOnDisk(); void analyze_doesNotMutateSource(); void unsupportedPrecondition_preventsApply(); void failedOperation_discardsCandidate(); @@ -192,6 +210,96 @@ void RepairOperationTest::transactionRejectsAWeakenedSavePolicyBeforeMutation() QStringLiteral("Refused save policy: mode 'incremental-append' is weaker than the operation-declared 'save-as-new-artifact'.")); } +void RepairOperationTest::saveRequestRefusesToWriteOverTheTrustedSource() +{ + // The guard compares real files: create both paths so the check has + // something to resolve. + QTemporaryDir directory; + QVERIFY(directory.isValid()); + const QString sourcePath = directory.filePath(QStringLiteral("received.pdf")); + const QString candidatePath = directory.filePath(QStringLiteral("received-candidate.pdf")); + for (const QString& path : { sourcePath, candidatePath }) + { + QFile file(path); + QVERIFY(file.open(QIODevice::WriteOnly)); + QVERIFY(file.write(QByteArrayLiteral("%PDF-1.7\n%%EOF\n")) > 0); + file.close(); + } + + const pdf::PDFOperationSavePolicy required = pdf::PDFOperationSavePolicy::saveAsNewArtifact(QStringLiteral("production correction")); + pdf::PDFSaveRequest request; + request.sourcePath = sourcePath; + request.outputPath = sourcePath; + request.required = required; + request.requested = required; + request.requestedExplicitly = true; + const pdf::PDFOperationResult refused = pdf::validateSaveRequest(request); + QVERIFY(!refused); + QCOMPARE(refused.getErrorMessage(), + QStringLiteral("Refused save: 'received.pdf' is the trusted input artifact; write the candidate to a new path.")); + + // A distinct output path is fine, and an in-place append is the point of + // that mode. + request.outputPath = candidatePath; + QVERIFY(pdf::validateSaveRequest(request)); + + pdf::PDFSaveRequest append; + append.sourcePath = sourcePath; + append.outputPath = sourcePath; + append.required = pdf::PDFOperationSavePolicy::incrementalAppend(QStringLiteral("annotation edit")); + append.requested = append.required; + append.requestedExplicitly = true; + append.appendInPlace = true; + QVERIFY(pdf::validateSaveRequest(append)); + + // A weakened request is refused by the same call. + pdf::PDFSaveRequest weakened = request; + weakened.requested = pdf::PDFOperationSavePolicy::incrementalAppend(QStringLiteral("caller wants an append")); + QCOMPARE(pdf::validateSaveRequest(weakened).getErrorMessage(), + QStringLiteral("Refused save policy: mode 'incremental-append' is weaker than the operation-declared 'save-as-new-artifact'.")); +} + +void RepairOperationTest::candidateSaveRefusesToOverwriteTheSourceOnDisk() +{ + QTemporaryDir directory; + QVERIFY(directory.isValid()); + const QString sourcePath = directory.filePath(QStringLiteral("received.pdf")); + + pdf::PDFDocumentBuilder builder; + builder.appendPage(QRectF(0, 0, 100, 100)); + const pdf::PDFDocument source = builder.build(); + + const QByteArray sourceBytes = writeSerializedBytes(source); + QFile sourceFile(sourcePath); + QVERIFY(sourceFile.open(QIODevice::WriteOnly)); + QCOMPARE(sourceFile.write(sourceBytes), qint64(sourceBytes.size())); + sourceFile.close(); + const QByteArray digestBefore = QCryptographicHash::hash(sourceBytes, QCryptographicHash::Sha256); + + pdf::PDFRepairTransactionOptions options; + options.sourcePath = sourcePath; + pdf::PDFRepairTransaction transaction(source, options); + QVERIFY(transaction.add(pdf::PDFRepairRegistry::instance().find(QStringLiteral("add-bleed")), + QJsonObject{ { QStringLiteral("bleed_mm"), 3.0 }, + { QStringLiteral("force"), true } })); + QVERIFY(transaction.analyze()); + QVERIFY(transaction.apply()); + + // The reopened candidate is a required out-parameter of the write API, and + // passing none would fail the call for an unrelated reason: this has to be + // a real refusal to write over the source, not a failed call. + pdf::PDFDocument reopenedCandidate; + const pdf::PDFOperationResult refusedWrite = transaction.serializeCandidate(sourcePath, &reopenedCandidate); + QVERIFY(!refusedWrite); + QCOMPARE(refusedWrite.getErrorMessage(), + QStringLiteral("Refused save: 'received.pdf' is the trusted input artifact; write the candidate to a new path.")); + QFile untouched(sourcePath); + QVERIFY(untouched.open(QIODevice::ReadOnly)); + const QByteArray digestAfter = QCryptographicHash::hash(untouched.readAll(), QCryptographicHash::Sha256); + untouched.close(); + QCOMPARE(digestAfter, digestBefore); +} + void RepairOperationTest::analyze_doesNotMutateSource() { pdf::PDFDocumentBuilder builder; From f4fad3b871cec0010ade99d082c494a66d616b93 Mon Sep 17 00:00:00 2001 From: mberrys Date: Mon, 14 Sep 2026 00:23:16 -0700 Subject: [PATCH 07/26] test(core): pin source bytes across success, cancel, and failure (#239) Pinned invariant: no analyze, apply, or candidate-serialize path writes to PDFRepairTransactionOptions::sourcePath. The slot passed without a production change. --- UnitTests/tst_repairoperationtest.cpp | 70 +++++++++++++++++++++++++++ 1 file changed, 70 insertions(+) diff --git a/UnitTests/tst_repairoperationtest.cpp b/UnitTests/tst_repairoperationtest.cpp index 0f4b17afe..29b10b434 100644 --- a/UnitTests/tst_repairoperationtest.cpp +++ b/UnitTests/tst_repairoperationtest.cpp @@ -81,6 +81,7 @@ private slots: void transactionRejectsAWeakenedSavePolicyBeforeMutation(); void saveRequestRefusesToWriteOverTheTrustedSource(); void candidateSaveRefusesToOverwriteTheSourceOnDisk(); + void sourceBytesSurviveSuccessCancelAndFailure(); void analyze_doesNotMutateSource(); void unsupportedPrecondition_preventsApply(); void failedOperation_discardsCandidate(); @@ -300,6 +301,75 @@ void RepairOperationTest::candidateSaveRefusesToOverwriteTheSourceOnDisk() QCOMPARE(digestAfter, digestBefore); } +void RepairOperationTest::sourceBytesSurviveSuccessCancelAndFailure() +{ + QTemporaryDir directory; + QVERIFY(directory.isValid()); + const QString sourcePath = directory.filePath(QStringLiteral("received.pdf")); + const QString candidatePath = directory.filePath(QStringLiteral("candidate.pdf")); + + pdf::PDFDocumentBuilder builder; + builder.appendPage(QRectF(0, 0, 100, 100)); + const pdf::PDFDocument source = builder.build(); + const QByteArray sourceBytes = writeSerializedBytes(source); + QFile sourceFile(sourcePath); + QVERIFY(sourceFile.open(QIODevice::WriteOnly)); + QCOMPARE(sourceFile.write(sourceBytes), qint64(sourceBytes.size())); + sourceFile.close(); + const QByteArray digest = QCryptographicHash::hash(sourceBytes, QCryptographicHash::Sha256); + + const auto sourceDigestNow = [&sourcePath]() + { + QFile file(sourcePath); + if (!file.open(QIODevice::ReadOnly)) + { + return QByteArray(); + } + const QByteArray digest = QCryptographicHash::hash(file.readAll(), QCryptographicHash::Sha256); + file.close(); + return digest; + }; + + pdf::PDFRepairTransactionOptions options; + options.sourcePath = sourcePath; + const QJsonObject parameters{ { QStringLiteral("bleed_mm"), 3.0 }, { QStringLiteral("force"), true } }; + + // success + { + pdf::PDFRepairTransaction transaction(source, options); + QVERIFY(transaction.add(pdf::PDFRepairRegistry::instance().find(QStringLiteral("add-bleed")), parameters)); + QVERIFY(transaction.analyze()); + QVERIFY(transaction.apply()); + pdf::PDFDocument reopenedCandidate; + const pdf::PDFOperationResult serialized = transaction.serializeCandidate(candidatePath, &reopenedCandidate); + QVERIFY2(serialized, qPrintable(serialized.getErrorMessage())); + QCOMPARE(sourceDigestNow(), digest); + } + + // cancel: analyze, then stop without applying + { + pdf::PDFRepairTransaction transaction(source, options); + QVERIFY(transaction.add(pdf::PDFRepairRegistry::instance().find(QStringLiteral("add-bleed")), parameters)); + QVERIFY(transaction.analyze()); + QCOMPARE(transaction.status(), pdf::PDFRepairStatus::Planned); + QCOMPARE(sourceDigestNow(), digest); + } + + // failure: an operation whose precondition is unsupported (rgb-to-cmyk + // needs a document with color images) is refused before any mutation. A + // failing operation is reported as Unsupported rather than as a failed + // analyze(), so the failure leg pins the status as well as the bytes. + { + pdf::PDFRepairTransaction transaction(source, options); + QVERIFY(transaction.add(pdf::PDFRepairRegistry::instance().find(QStringLiteral("rgb-to-cmyk")), QJsonObject())); + QVERIFY(transaction.analyze()); + QCOMPARE(transaction.status(), pdf::PDFRepairStatus::Unsupported); + QVERIFY(!transaction.apply()); + QVERIFY(transaction.status() != pdf::PDFRepairStatus::Applied); + QCOMPARE(sourceDigestNow(), digest); + } +} + void RepairOperationTest::analyze_doesNotMutateSource() { pdf::PDFDocumentBuilder builder; From 9ae47921e3919b31827a5c16de315d1ffe2a355f Mon Sep 17 00:00:00 2001 From: mberrys Date: Mon, 14 Sep 2026 00:24:54 -0700 Subject: [PATCH 08/26] test(core): pin the received input as a read-only artifact identity (#239) Pinned invariant: PDFArtifactStore publishes an imported file at its digest-addressed path with write bits cleared, so the received input is a distinct, read-only identity. --- UnitTests/tst_operationhistorytest.cpp | 35 ++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/UnitTests/tst_operationhistorytest.cpp b/UnitTests/tst_operationhistorytest.cpp index 63bcae44b..2e5eca731 100644 --- a/UnitTests/tst_operationhistorytest.cpp +++ b/UnitTests/tst_operationhistorytest.cpp @@ -26,6 +26,7 @@ #include #include #include +#include #include #include #include @@ -45,6 +46,7 @@ class OperationHistoryTest final : public QObject private slots: void canonicalJsonIsStableAndRedacted(); void artifactStoreStreamsAndDetectsTampering(); + void importedInputIsReadOnlyAndDigestAddressed(); void lifecycleApprovalAndRollbackResolution(); void rollbackPointsRetentionAndAtomicity(); void externalPayloadTamperingCompromisesChain(); @@ -813,5 +815,38 @@ void OperationHistoryTest::schemaMigratedEventAppendedOnRewrite() QCOMPARE(events.first().resultSummary.value(QStringLiteral("to_version")).toString(), QStringLiteral("3.0")); } +void OperationHistoryTest::importedInputIsReadOnlyAndDigestAddressed() +{ + QTemporaryDir directory; + QVERIFY(directory.isValid()); + const QByteArray inboxBytes = QByteArrayLiteral("%PDF-1.7\n%%EOF\n"); + const QString inboxPath = directory.filePath(QStringLiteral("received.pdf")); + QFile inbox(inboxPath); + QVERIFY(inbox.open(QIODevice::WriteOnly)); + QVERIFY(inbox.write(inboxBytes) > 0); + inbox.close(); + const QByteArray digest = QCryptographicHash::hash(inboxBytes, QCryptographicHash::Sha256); + + pdf::PDFArtifactStore store(directory.filePath(QStringLiteral("store"))); + const pdf::PDFArtifactStoreResult imported = store.importFile(inboxPath, {}); + QVERIFY(imported.success); + QVERIFY(imported.artifact.sha256 == QString::fromLatin1(digest.toHex())); + QVERIFY(store.contains(imported.artifact)); + QVERIFY(store.verify(imported.artifact)); + + const QString stored = store.pathFor(imported.artifact); + // QFileInfo::isReadOnly() was removed in Qt 6; !isWritable() is its + // documented replacement. + QVERIFY(!QFileInfo(stored).isWritable()); + QFile::Permissions permissions = QFile::permissions(stored); + QVERIFY(permissions.testFlag(QFile::ReadOwner)); + QVERIFY(!permissions.testFlag(QFile::WriteOwner)); + + // The imported input is a separate identity from the received file. + QVERIFY(QFileInfo(stored).canonicalFilePath() != QFileInfo(inboxPath).canonicalFilePath()); + // Importing is a read: the received file itself is neither moved nor truncated. + QCOMPARE(QFile(inboxPath).size(), qint64(inboxBytes.size())); +} + QTEST_MAIN(OperationHistoryTest) #include "tst_operationhistorytest.moc" From 42e8ee1835152ae692ac945d629fb29d7a796b33 Mon Sep 17 00:00:00 2001 From: mberrys Date: Mon, 14 Sep 2026 00:29:55 -0700 Subject: [PATCH 09/26] test(core): pin that no non-incremental operation is ever appended (#239) Pinned invariant: the slot passed without a production change. Its non-vacuity comes from three assertions inside it - the registry holds at least 7 operations, at least one of them declines the append path, and downsample-images declares full rewrite. A test-side mutation that inverted the loop's else-branch expectation failed on add-bleed, proving the branch is reached against a signed source. --- UnitTests/tst_repairoperationtest.cpp | 50 +++++++++++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/UnitTests/tst_repairoperationtest.cpp b/UnitTests/tst_repairoperationtest.cpp index 29b10b434..3fec5e8b4 100644 --- a/UnitTests/tst_repairoperationtest.cpp +++ b/UnitTests/tst_repairoperationtest.cpp @@ -25,6 +25,8 @@ #include "pdfrepairoperation.h" #include "pdfstandardconversion.h" +#include + #include #include #include @@ -78,6 +80,7 @@ private slots: void builtInOperations_areRegistered(); void builtInOperations_declareSavePolicies(); void everyRegisteredOperationDeclaresItsSavePolicy(); + void noNonIncrementalOperationCanBeAppendedToASignedSource(); void transactionRejectsAWeakenedSavePolicyBeforeMutation(); void saveRequestRefusesToWriteOverTheTrustedSource(); void candidateSaveRefusesToOverwriteTheSourceOnDisk(); @@ -164,6 +167,53 @@ void RepairOperationTest::everyRegisteredOperationDeclaresItsSavePolicy() QVERIFY(undeclared.invalidatesSignatures); QVERIFY(!pdf::PDFOperationSavePolicy::incrementalAppend(QStringLiteral("ordinary edit")).isUndeclared()); } +void RepairOperationTest::noNonIncrementalOperationCanBeAppendedToASignedSource() +{ + const pdf::PDFRepairRegistry& registry = pdf::PDFRepairRegistry::instance(); + const QStringList ids = registry.operationIds(); + // Guards that keep the loop below from passing on an empty or + // one-sided registry. + QVERIFY2(ids.size() >= 7, qPrintable(QString::number(ids.size()))); + QVERIFY2(std::any_of(ids.cbegin(), ids.cend(), + [®istry](const QString& id) + { + return registry.find(id)->savePolicy().mode != pdf::PDFSaveMode::IncrementalAppend; + }), + "no registered operation declines the append path"); + + // A signature dictionary is enough to make the writer's incremental path + // eligible; whether the *operation* may use it is the policy's decision. + pdf::PDFDocumentBuilder builder; + builder.appendPage(QRectF(0, 0, 200, 200)); + pdf::PDFDictionary signature; + signature.addEntry(pdf::PDFInplaceOrMemoryString("Type"), pdf::PDFObject::createName("Sig")); + builder.addObject(pdf::PDFObject::createDictionary(std::make_shared(std::move(signature)))); + const pdf::PDFDocument signedSource = builder.build(); + + QCOMPARE(pdf::PDFDocumentWriter::getRecommendedWriteMode(&signedSource, + pdf::PDFOperationSavePolicy::incrementalAppend(QStringLiteral("ordinary edit")), + false), + pdf::PDFDocumentWriter::WriteMode::Incremental); + + for (const QString& id : ids) + { + const pdf::PDFOperationSavePolicy declared = registry.find(id)->savePolicy(); + const pdf::PDFDocumentWriter::WriteMode mode = + pdf::PDFDocumentWriter::getRecommendedWriteMode(&signedSource, declared, false); + if (declared.mode == pdf::PDFSaveMode::IncrementalAppend) + { + QCOMPARE(mode, pdf::PDFDocumentWriter::WriteMode::Incremental); + } + else + { + QVERIFY2(mode == pdf::PDFDocumentWriter::WriteMode::FullRewrite, qPrintable(id)); + } + } + + // The only destructive registered operation declares full rewrite. + QCOMPARE(registry.find(QStringLiteral("downsample-images"))->savePolicy().mode, + pdf::PDFSaveMode::FullRewrite); +} void RepairOperationTest::transactionRejectsAWeakenedSavePolicyBeforeMutation() { From ea8389237d2a58424428b72992f5d8bdc64202bd Mon Sep 17 00:00:00 2001 From: mberrys Date: Mon, 14 Sep 2026 00:37:19 -0700 Subject: [PATCH 10/26] feat(cli): refuse an in-place redaction of the trusted input (#239) document.redact now runs pdf::validateSaveRequest with a fullRewrite("redaction removes prior content") requirement before it reads the document, so the refusal does not depend on how much work the redaction would have done, and it emits save-policy.refused with ProcessingFailure (4) instead of silently overwriting the caller's input. Also reformats PdfTool/pdftoolredact.cpp: the file was already clang-format dirty at HEAD (exit 74), and the changed-file format gate checks touched files. --- PdfTool/pdftoolredact.cpp | 57 ++++++++++++++++++++----------- UnitTests/tst_pdftoolcontract.cpp | 36 +++++++++++++++++++ docs/PDFTOOL_CLI_CONTRACT.md | 15 ++++++++ 3 files changed, 88 insertions(+), 20 deletions(-) diff --git a/PdfTool/pdftoolredact.cpp b/PdfTool/pdftoolredact.cpp index fdafc1472..281c5029d 100644 --- a/PdfTool/pdftoolredact.cpp +++ b/PdfTool/pdftoolredact.cpp @@ -36,18 +36,18 @@ QString PDFToolRedact::getStandardString(PDFToolAbstractApplication::StandardStr { switch (standardString) { - case Command: - return "redact"; + case Command: + return "redact"; - case Name: - return PDFToolTranslationContext::tr("Redact"); + case Name: + return PDFToolTranslationContext::tr("Redact"); - case Description: - return PDFToolTranslationContext::tr("Create a redacted document from the original document."); + case Description: + return PDFToolTranslationContext::tr("Create a redacted document from the original document."); - default: - Q_ASSERT(false); - break; + default: + Q_ASSERT(false); + break; } return QString(); @@ -61,6 +61,25 @@ PDFToolExitCode PDFToolRedact::execute(const PDFToolOptions& options) return PDFToolExitCode::InvalidInvocation; } + // Redaction removes prior content, so it declares a full rewrite and never + // persists over the input it was handed. Rejected before the document is + // read: the refusal cannot depend on how much work the redaction would do. + pdf::PDFSaveRequest saveRequest; + saveRequest.sourcePath = options.document; + saveRequest.outputPath = options.redactedDocument; + saveRequest.required = pdf::PDFOperationSavePolicy::fullRewrite(QStringLiteral("redaction removes prior content")); + saveRequest.requested = saveRequest.required; + saveRequest.requestedExplicitly = true; + saveRequest.appendInPlace = false; + const pdf::PDFOperationResult saveValidation = pdf::validateSaveRequest(saveRequest); + if (!saveValidation) + { + reportDiagnostic(options, PDFToolDiagnosticSeverity::Error, QStringLiteral("save-policy.refused"), + saveValidation.getErrorMessage(), + QJsonObject{ { QStringLiteral("path"), options.redactedDocument } }); + return PDFToolExitCode::ProcessingFailure; + } + pdf::PDFDocument document; QByteArray sourceData; if (!readDocument(options, document, &sourceData, false)) @@ -77,21 +96,21 @@ PDFToolExitCode PDFToolRedact::execute(const PDFToolOptions& options) { if (options.executionContext) { - options.executionContext->setData(QJsonObject{{QStringLiteral("operation"), QStringLiteral("redact")}, {QStringLiteral("dry_run"), options.destructiveDryRun}}); + options.executionContext->setData(QJsonObject{ { QStringLiteral("operation"), QStringLiteral("redact") }, { QStringLiteral("dry_run"), options.destructiveDryRun } }); } } else if (options.destructiveReport) { PDFConsole::writeText(PDFToolTranslationContext::tr("Would redact '%1' to '%2'.") - .arg(options.document, options.redactedDocument), - options.outputCodec); + .arg(options.document, options.redactedDocument), + options.outputCodec); } if (options.destructiveDryRun) { if (options.executionContext) { - options.executionContext->addOutput({QStringLiteral("file"), QStringLiteral("primary"), options.redactedDocument, QStringLiteral("planned")}); + options.executionContext->addOutput({ QStringLiteral("file"), QStringLiteral("primary"), options.redactedDocument, QStringLiteral("planned") }); } return PDFToolExitCode::Success; } @@ -125,18 +144,16 @@ PDFToolExitCode PDFToolRedact::execute(const PDFToolOptions& options) pdf::PDFOperationResult result = writer.write(options.redactedDocument, &redactedDocument, true); if (!result) { - reportDiagnostic(options, PDFToolDiagnosticSeverity::Error, QStringLiteral("output.write-failed"), PDFToolTranslationContext::tr("Failed to write redacted document. %1").arg(result.getErrorMessage()), QJsonObject{{QStringLiteral("path"), options.redactedDocument}}); + reportDiagnostic(options, PDFToolDiagnosticSeverity::Error, QStringLiteral("output.write-failed"), PDFToolTranslationContext::tr("Failed to write redacted document. %1").arg(result.getErrorMessage()), QJsonObject{ { QStringLiteral("path"), options.redactedDocument } }); return PDFToolExitCode::ProcessingFailure; } if (options.executionContext) { - options.executionContext->addOutput({ - QStringLiteral("file"), - QStringLiteral("primary"), - options.redactedDocument, - QStringLiteral("written") - }); + options.executionContext->addOutput({ QStringLiteral("file"), + QStringLiteral("primary"), + options.redactedDocument, + QStringLiteral("written") }); } return PDFToolExitCode::Success; diff --git a/UnitTests/tst_pdftoolcontract.cpp b/UnitTests/tst_pdftoolcontract.cpp index 43026370c..076586f5c 100644 --- a/UnitTests/tst_pdftoolcontract.cpp +++ b/UnitTests/tst_pdftoolcontract.cpp @@ -23,6 +23,7 @@ #include "processoutputcapture.h" #include +#include #include #include #include @@ -109,6 +110,7 @@ private slots: void fetchTextFailIfEmptyKeepsSuccessWhenTextExists(); void preflightRejectsNonJsonOutput(); void preflightKeepsNestedReportBoundary(); + void redactRefusesToWriteOverItsOwnInput(); }; void PdfToolContractTest::helpIsWrapped() @@ -316,6 +318,40 @@ void PdfToolContractTest::preflightKeepsNestedReportBoundary() QVERIFY(run.json.value(QStringLiteral("data")).toObject().value(QStringLiteral("report")).isUndefined()); } +void PdfToolContractTest::redactRefusesToWriteOverItsOwnInput() +{ + // Redaction removes prior content, so the command must refuse to persist + // its result over the trusted input the caller handed it. + QTemporaryDir directory; + QVERIFY(directory.isValid()); + const QString inputPath = directory.filePath(QStringLiteral("received.pdf")); + const QString fixture = + QDir(QStringLiteral(LOOP_PREFLIGHT_SOURCE_DIR)).filePath(QStringLiteral("testdata/fixtures/color-rgb.pdf")); + QVERIFY2(QFile::copy(fixture, inputPath), qPrintable(fixture)); + + const ToolRun run = runPdfTool({ QStringLiteral("redact"), + QStringLiteral("--console-format"), QStringLiteral("json"), + inputPath, inputPath }); + + verifyEnvelope(run, 4, QStringLiteral("redact")); + const QJsonObject diagnostic = findDiagnostic(run, QStringLiteral("save-policy.refused")); + QVERIFY2(!diagnostic.isEmpty(), qPrintable(QString::fromUtf8(run.stdoutData))); + QCOMPARE(diagnostic.value(QStringLiteral("severity")).toString(), QStringLiteral("error")); + QVERIFY(diagnostic.value(QStringLiteral("message")).toString().contains(QStringLiteral("trusted input artifact"))); + QCOMPARE(diagnostic.value(QStringLiteral("context")).toObject().value(QStringLiteral("path")).toString(), inputPath); + QVERIFY(run.json.value(QStringLiteral("outputs")).toArray().isEmpty()); + QVERIFY(QFile(inputPath).exists()); + + // The refusal must be about writing over the input, not about redaction: + // the same document and the same caller still produce the artifact when + // the output is a different path. + const ToolRun legitimate = runPdfTool({ QStringLiteral("redact"), + QStringLiteral("--console-format"), QStringLiteral("json"), + inputPath, directory.filePath(QStringLiteral("redacted.pdf")) }); + QCOMPARE(legitimate.exitCode, 0); + QVERIFY(findDiagnostic(legitimate, QStringLiteral("save-policy.refused")).isEmpty()); +} + } // namespace QTEST_MAIN(PdfToolContractTest) diff --git a/docs/PDFTOOL_CLI_CONTRACT.md b/docs/PDFTOOL_CLI_CONTRACT.md index b9a387e33..cf70850f3 100644 --- a/docs/PDFTOOL_CLI_CONTRACT.md +++ b/docs/PDFTOOL_CLI_CONTRACT.md @@ -140,6 +140,21 @@ In JSON mode, handled errors and warnings are captured in `diagnostics` and are **not** additionally written to stderr. In text/XML/HTML mode the existing human-facing stderr behavior is preserved. +### Save-policy refusals + +A command that declares a save mode also declares what that mode has to +guarantee. `redact` removes prior content, so its result is always a full +rewrite and never an append: the guard runs before the document is read, so an +incompatible request is rejected before any content is touched. + +| Command | `code` | Exit | Guard | `message` | +|---|---|---|---|---| +| `redact` | `save-policy.refused` | `4 processing-failure` | Redaction removes prior content, so the output must be a full rewrite written to a path other than the trusted input. | `Refused save: '' is the trusted input artifact; write the candidate to a new path.` | + +`context` carries the refused output path as `path`. The diagnostic is an +`error`, the run records no output, and the input file is left byte-identical: +`PdfTool redact received.pdf received.pdf` is never a successful invocation. + ### Empty results Extraction commands (`fetch-images`, `fetch-text`, `attachments`) complete From 17b647b5425e12c2bf0118e7046a543e59df1e45 Mon Sep 17 00:00:00 2001 From: mberrys Date: Mon, 14 Sep 2026 00:43:38 -0700 Subject: [PATCH 11/26] chore(qualification): add a signed-fixture generator for the incremental-save proof (#239) --- scripts/qualification/make_signed_fixture.py | 111 +++++++++++++++++++ 1 file changed, 111 insertions(+) create mode 100644 scripts/qualification/make_signed_fixture.py diff --git a/scripts/qualification/make_signed_fixture.py b/scripts/qualification/make_signed_fixture.py new file mode 100644 index 000000000..c4e7221a8 --- /dev/null +++ b/scripts/qualification/make_signed_fixture.py @@ -0,0 +1,111 @@ +#!/usr/bin/env python3 +"""Generate the committed signed-PDF fixture used by UnitTestsIncrementalSave. + +This is a developer/CI tool, not a shipped dependency. The generated PDF and its +manifest are committed; the private key is created in a temporary directory and +is never written into the repository. Regeneration is not byte-reproducible (the +signing time and the CMS bytes differ per run), so the manifest digest must be +re-recorded whenever the fixture is regenerated. + +Run it with a throwaway interpreter, never the host one: + + python -m venv "$LOCALAPPDATA/Temp/loop-fixture-venv" + "$LOCALAPPDATA/Temp/loop-fixture-venv/Scripts/python" -m pip install "pyhanko==0.37.0" cryptography + "$LOCALAPPDATA/Temp/loop-fixture-venv/Scripts/python" scripts/qualification/make_signed_fixture.py \ + --output UnitTests/testdata/signatures/signed-incremental-base.pdf \ + --manifest UnitTests/testdata/signatures/manifest.json +""" + +from __future__ import annotations + +import argparse +import datetime +import hashlib +import json +import tempfile +from pathlib import Path + +from cryptography import x509 +from cryptography.hazmat.primitives import hashes, serialization +from cryptography.hazmat.primitives.asymmetric import rsa +from cryptography.x509.oid import NameOID +from pyhanko.pdf_utils.generic import StreamObject +from pyhanko.pdf_utils.writer import PageObject, PdfFileWriter +from pyhanko.sign import signers +from pyhanko.sign.fields import SigFieldSpec + +FIELD_NAME = "LoopSignature" +BLANK_PAGE_MEDIA_BOX = (0, 0, 595, 842) + + +def build_key_pair(directory: Path) -> tuple[Path, Path, str, str]: + key = rsa.generate_private_key(public_exponent=65537, key_size=2048) + subject = issuer = x509.Name([x509.NameAttribute(NameOID.COMMON_NAME, "Loop Save Policy Fixture"), + x509.NameAttribute(NameOID.ORGANIZATION_NAME, "Studio Berry")]) + now = datetime.datetime.now(datetime.timezone.utc) + certificate = ( + x509.CertificateBuilder() + .subject_name(subject) + .issuer_name(issuer) + .public_key(key.public_key()) + .serial_number(x509.random_serial_number()) + .not_valid_before(now - datetime.timedelta(days=1)) + .not_valid_after(now + datetime.timedelta(days=3650)) + .sign(key, hashes.SHA256()) + ) + key_path = directory / "fixture-key.pem" + cert_path = directory / "fixture-cert.pem" + key_path.write_bytes( + key.private_bytes(serialization.Encoding.PEM, + serialization.PrivateFormat.PKCS8, + serialization.NoEncryption()) + ) + cert_path.write_bytes(certificate.public_bytes(serialization.Encoding.PEM)) + fingerprint = certificate.fingerprint(hashes.SHA256()).hex() + return key_path, cert_path, fingerprint, certificate.subject.rfc4514_string() + + +def main() -> int: + parser = argparse.ArgumentParser() + parser.add_argument("--output", required=True, type=Path) + parser.add_argument("--manifest", required=True, type=Path) + arguments = parser.parse_args() + + with tempfile.TemporaryDirectory() as temporary: + key_path, cert_path, fingerprint, subject = build_key_pair(Path(temporary)) + signer = signers.SimpleSigner.load(str(key_path), str(cert_path), ca_chain_files=None, key_passphrase=None) + writer = PdfFileWriter() + writer.insert_page(PageObject(writer.add_object(StreamObject(stream_data=b"")), BLANK_PAGE_MEDIA_BOX)) + output = arguments.output + output.parent.mkdir(parents=True, exist_ok=True) + with output.open("wb") as output_stream: + signers.PdfSigner( + signers.PdfSignatureMetadata(field_name=FIELD_NAME, md_algorithm="sha256"), + signer=signer, + new_field_spec=SigFieldSpec(FIELD_NAME, on_page=0, box=(36, 36, 236, 96)), + ).sign_pdf(writer, output=output_stream) + + payload = output.read_bytes() + manifest = { + "fixture": output.name, + "purpose": "prove that an incremental append preserves a real signature byte range", + "provenance": "generated in-repo by scripts/qualification/make_signed_fixture.py; not third-party", + "license": "same as the repository (MIT)", + "generator": "pyhanko 0.37.0 + cryptography", + "signature_field": FIELD_NAME, + "certificate_subject": subject, + "certificate_sha256_fingerprint": fingerprint, + "private_key": "ephemeral, created in a temporary directory, never committed", + "byte_reproducible": False, + "sha256": hashlib.sha256(payload).hexdigest(), + "bytes": len(payload), + "regenerated_utc": datetime.datetime.now(datetime.timezone.utc).isoformat(), + "expected_validator_result": {"structural": "passed", "signature": "passed"}, + } + arguments.manifest.write_text(json.dumps(manifest, indent=2) + "\n", encoding="utf-8") + print(f"{output} {manifest['sha256']} {manifest['bytes']}") + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) From 6eca835cea7178b0e55d65dbf65377559b2d1d31 Mon Sep 17 00:00:00 2001 From: mberrys Date: Mon, 14 Sep 2026 00:43:43 -0700 Subject: [PATCH 12/26] test(core): add a real signed incremental-save fixture with provenance (#239) --- UnitTests/testdata/signatures/manifest.json | 19 ++++++++++++++++++ .../signatures/signed-incremental-base.pdf | Bin 0 -> 7219 bytes 2 files changed, 19 insertions(+) create mode 100644 UnitTests/testdata/signatures/manifest.json create mode 100644 UnitTests/testdata/signatures/signed-incremental-base.pdf diff --git a/UnitTests/testdata/signatures/manifest.json b/UnitTests/testdata/signatures/manifest.json new file mode 100644 index 000000000..915d1af4d --- /dev/null +++ b/UnitTests/testdata/signatures/manifest.json @@ -0,0 +1,19 @@ +{ + "fixture": "signed-incremental-base.pdf", + "purpose": "prove that an incremental append preserves a real signature byte range", + "provenance": "generated in-repo by scripts/qualification/make_signed_fixture.py; not third-party", + "license": "same as the repository (MIT)", + "generator": "pyhanko 0.37.0 + cryptography", + "signature_field": "LoopSignature", + "certificate_subject": "O=Studio Berry,CN=Loop Save Policy Fixture", + "certificate_sha256_fingerprint": "829d7fe116133bbf2e282ee780f145517a9aac42a79b68d9abec74c17e0811c8", + "private_key": "ephemeral, created in a temporary directory, never committed", + "byte_reproducible": false, + "sha256": "b5bb03596f88ffdc3503b8ebf60a3d6d6ffa2936dab91db07935ad522245101d", + "bytes": 7219, + "regenerated_utc": "2026-09-14T07:42:27.907123+00:00", + "expected_validator_result": { + "structural": "passed", + "signature": "passed" + } +} diff --git a/UnitTests/testdata/signatures/signed-incremental-base.pdf b/UnitTests/testdata/signatures/signed-incremental-base.pdf new file mode 100644 index 0000000000000000000000000000000000000000..25f0308500d543bcd61066948ce362b49dfb0708 GIT binary patch literal 7219 zcmeHMOOISf6=r2wSNQ?3qBcej!nf=FkSt5}OrqGycxL1vC6Tz(eW%@yr+d=f4r4Z; zNNf;)AQBrES+Rm8D-!lB_z|qwBUa(7+uifB69JnD;c2_O>(;HRbH4MP^HtZqwU^2r z)|%$lm!E(6`#=A#VT6*!(Ua!x-DY(7{In;d*sZ&#i)l03>!$sRaNMxpjQn`HD2wF@ zQA7LQy{4Z}>cHB6b%J4BGm496^EF}3=!E9Qdc2i7y$-mlrKID+pn*cJo zeSfhy#rJu)ezxpi-{J1-+Rf!pojYSS4u zIN#xd+~JB6!vHo_)fT@RS1dJm&8W^<8+Vv8K)pMCv!6{LZ#&-t!)9YYpFZso@zKG$ zKY2tHsPbU8TFvHDxDGC8ia!6z<<^7l?4s|2NY&*X&|CW`h7(RJ9j?=0l+V_09IU%_ zKOh#m@ki4o$U1rBV7L%Hp{;I4<8JuGG8N1U%+Ouf@NofOGYZ|RANnsqI}c;M51!@u zcrk%%Wc1!_?&qu7?gD0>Em!OKxLcAM=lk8I#^CJ0M|c81u11U0aPC*@wl;2T`tiF* zPe7JSRH})KnKvosjIH-?GwWmWF@gP;AOMMJTiK2mK8-@kk55SRF{!nJBBFR>uxEVI z04FLOM-6C&RMa65t&)y%vYa-e)y@&ES`LbwkSk5ziaN1rxiQ4FT3f`wbykuwVZ3$9 z5UDX<5pG(=ctfPa5=6LGGD{?DB^DW9YYSXPCvB;*0JF7}5g}Ts;Q((m1 z8_8|U%tcGKoQjKiIc<4em9|W2!eENqHX0^dCx>~roxJ7F5X@JXLy3jcNRQ2zG18<} zVs~A;)=F6&#gK;Vn!6cy?O{0$L@fuE2TOLXot?Y(;00z$=0?Gk&Yg#JBE+7uE6Z|4v4&meHv2?l>Kz$SS21V*p6SzJm$KXYc z3yd*gNT6vbs8Ct%ZJCq`~4T%!=S?nxt(Uy87kS+=VXOy-cQKUb5d(NIp{v=$=(7r z?l~#5vs&gqph~|^4xf<2AClXLvy*-T%#N5Q{Ndxj@Ijv*LQBnUOK!y zIv>Kr3}YWc!+tuXWUX^3cYyDJJbY0$s|xJOjrptx+P4G|F(T1Y-)bi1S%IjWjAz`?bWFf)TjSaJ*m@%M2I_ z2(&e|2wWNF zd3M5&%R97-6hpMinOkjThOMVuf}=W*&6*oIA*cLztyh(PWmT z2iJ5i)+!MbPmwz6JujL|#as!_*_Z=xwM_FOgJI~S+S8C|(IBX@fpZzMVA2_5l5kN- z9*T-K1n|nclvE1ZdQ%w1M`7AK#e>qqg=nIV+*&JmGAS7+gDfhPoQ$%iL?3;&NvTu} zYz@j=7nrsggn|c5B3BUZ zQ)N>hUY?m-4&hE#R8|(?If?Q@6VplaT;L{;oJO9krMLh~i%~LX3FwNecrg*EV@@a+ z9CjNbF($M^vPpbJJTxYiCL;$?P`Cndkf>m#juAjrqg?_h9*uoTSciJVdS;^L71yw+HKwCNB zP<13S#nTJ|`a)5~!03dKAi!Xm&^DQ@L}8f*W`hPz0hF@{UjgT+Ly#fxB zIV)H!Mk|EeG89Xsaqujt0xSv=2Nel88t%1XNfnQQD2Ljh^6220G$!Z*o*Ju>5j-oT zlS$6%5O|1Ouux(EUyv&EGR2HC22L;}4K6#UBUQ*c zSQ!#nNkJ}0k&dVx+{Zx&K$bxRz#av5emhn0ztNwUtKgt@j0dCyNc@7nIx3)~U!#*4 z^D<3FVpjFNrY=y^tNOtQZN8R!4*J7}1Vw1RNxeP$rrbr+q##!YDvY5}nj9eB8sZW0gGg9tC&JDQ#6y=L z5q<25d~wVwWIc3GqCja?qP3uaL$n|z8DT;-F(~CZdI+gX&~F973WyxG?-c51JT_4Q zLI`z}C0L0aC^mQ8O4uq;cq#U)*a0B)2xfE+VO1O|mvqSly;j1>wZ%*rq^>U;Vv8;1vR|5O{^a{{aFxqYls4 z{eCx}_T>F~_^m9_`1OM4A?`!+pZx6}NemAx%IxX7Uy_l>H6 zgT*A(6Z(H|lF{CBF?lwInYT}$zuC<{S`bVR`L7FVdS#V*__sR^dk;@k7@n96&x*FE z=llI}J(J%!jb|V0KJ_@ehs*nm1s-T&_b z&+q?b^2Zy~Km7cc;`6`#>9fDyf9 Date: Mon, 14 Sep 2026 00:50:10 -0700 Subject: [PATCH 13/26] test(core): prove a signed byte range survives an incremental edit (#239) --- UnitTests/CMakeLists.txt | 6 +- UnitTests/tst_incrementalsavetest.cpp | 87 +++++++++++++++++++++++++++ 2 files changed, 92 insertions(+), 1 deletion(-) diff --git a/UnitTests/CMakeLists.txt b/UnitTests/CMakeLists.txt index da4875bfb..ad0f7a596 100644 --- a/UnitTests/CMakeLists.txt +++ b/UnitTests/CMakeLists.txt @@ -46,7 +46,11 @@ set_target_properties(UnitTestsIncrementalSave PROPERTIES LIBRARY_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/${LOOP_INSTALL_LIB_DIR} RUNTIME_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/${LOOP_INSTALL_BIN_DIR} ) -add_test(UnitTestsIncrementalSave "${CMAKE_BINARY_DIR}/${LOOP_INSTALL_BIN_DIR}/UnitTestsIncrementalSave") +add_test(NAME UnitTestsIncrementalSave COMMAND UnitTestsIncrementalSave) +# The signed fixture lives in the source tree, not the ctest working directory. +set_tests_properties(UnitTestsIncrementalSave PROPERTIES + ENVIRONMENT "LOOP_FIXTURE_DATA_DIR=${CMAKE_CURRENT_SOURCE_DIR}/testdata" +) add_executable(UnitTestsProcessingBudget tst_processingbudgettest.cpp diff --git a/UnitTests/tst_incrementalsavetest.cpp b/UnitTests/tst_incrementalsavetest.cpp index 50d03132e..7ae2a915e 100644 --- a/UnitTests/tst_incrementalsavetest.cpp +++ b/UnitTests/tst_incrementalsavetest.cpp @@ -29,7 +29,9 @@ #include #include #include +#include #include +#include namespace { @@ -72,6 +74,7 @@ private slots: void unclassifiedAndRedactionPoliciesCannotSilentIncrementalAppend(); void policyStrengthRejectsWeakerRequests(); void fileOverloadReportsWhatItDid(); + void signedFixtureIncrementalEditPreservesTheSignedByteRange(); }; namespace @@ -121,6 +124,48 @@ QByteArray writeDocument(const pdf::PDFDocument& document) return buffer.data(); } +/// The committed signed fixture, located relative to LOOP_FIXTURE_DATA_DIR when +/// it is set (the CMake test definition sets it to the source tree) and +/// relative to the working directory otherwise. +QByteArray readFixtureBytes(const QString& fileName) +{ + const QString root = QString::fromUtf8(qgetenv("LOOP_FIXTURE_DATA_DIR")); + const QString path = root.isEmpty() + ? QStringLiteral("testdata/signatures/%1").arg(fileName) + : QStringLiteral("%1/signatures/%2").arg(root, fileName); + QFile file(path); + if (!file.open(QIODevice::ReadOnly)) + { + return {}; + } + const QByteArray data = file.readAll(); + file.close(); + return data; +} + +/// The two intervals a /ByteRange covers, as (start, length) pairs. +QVector> parseByteRange(const QByteArray& data) +{ + QVector> result; + const qsizetype marker = data.indexOf("/ByteRange"); + if (marker < 0) + { + return result; + } + const qsizetype open = data.indexOf('[', marker); + const qsizetype close = data.indexOf(']', open); + if (open < 0 || close < 0) + { + return result; + } + const QList numbers = data.mid(open + 1, close - open - 1).simplified().split(' '); + for (qsizetype index = 0; index + 1 < numbers.size(); index += 2) + { + result.append({ numbers.at(index).toLongLong(), numbers.at(index + 1).toLongLong() }); + } + return result; +} + } // namespace void IncrementalSaveTest::preservesOriginalPrefixAndChangedObjects() @@ -422,6 +467,48 @@ void IncrementalSaveTest::fileOverloadReportsWhatItDid() } } +void IncrementalSaveTest::signedFixtureIncrementalEditPreservesTheSignedByteRange() +{ + const QByteArray originalData = readFixtureBytes(QStringLiteral("signed-incremental-base.pdf")); + QVERIFY2(!originalData.isEmpty(), "signed fixture missing; see UnitTests/testdata/signatures/manifest.json"); + QVERIFY(originalData.contains("/ByteRange")); + QVERIFY(originalData.contains("/Contents")); + + const pdf::PDFDocument original = readDocument(originalData); + const pdf::PDFDocumentPointer modified = createModifiedDocument(original); + QVERIFY(modified); + + pdf::PDFDocumentWriter writer(nullptr); + QBuffer output; + output.open(QIODevice::WriteOnly); + QVERIFY(writer.writeIncremental(&output, originalData, &original, modified.data())); + + // 1. The original bytes, and therefore the signed byte range, are intact. + QCOMPARE(output.data().left(originalData.size()), originalData); + + // 2. The signature dictionary is untouched: same /ByteRange intervals and + // the same /Contents payload, which is what a verifier digests. + const auto originalRanges = parseByteRange(originalData); + const auto outputRanges = parseByteRange(output.data()); + QCOMPARE(outputRanges.size(), originalRanges.size()); + for (qsizetype index = 0; index < originalRanges.size(); ++index) + { + QCOMPARE(outputRanges.at(index), originalRanges.at(index)); + } + const auto contentsOf = [](const QByteArray& data) + { + const qsizetype marker = data.indexOf("/Contents"); + const qsizetype open = data.indexOf('<', marker); + const qsizetype close = data.indexOf('>', open); + return data.mid(open, close - open + 1); + }; + QCOMPARE(contentsOf(output.data()), contentsOf(originalData)); + + // 3. The append really is an append: new xref pointing at the old one. + QVERIFY(output.data().mid(originalData.size()).contains("/Prev")); + QVERIFY(output.data().size() > originalData.size()); +} + QTEST_MAIN(IncrementalSaveTest) #include "tst_incrementalsavetest.moc" From 27d369b56232f76026cc6ee77490d2b1598103ad Mon Sep 17 00:00:00 2001 From: mberrys Date: Mon, 14 Sep 2026 00:55:26 -0700 Subject: [PATCH 14/26] test(core): pin that incremental cost follows changed data (#239) --- UnitTests/tst_incrementalsavetest.cpp | 32 +++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/UnitTests/tst_incrementalsavetest.cpp b/UnitTests/tst_incrementalsavetest.cpp index 7ae2a915e..9ddd48927 100644 --- a/UnitTests/tst_incrementalsavetest.cpp +++ b/UnitTests/tst_incrementalsavetest.cpp @@ -75,6 +75,7 @@ private slots: void policyStrengthRejectsWeakerRequests(); void fileOverloadReportsWhatItDid(); void signedFixtureIncrementalEditPreservesTheSignedByteRange(); + void appendCostScalesWithChangedDataNotFileSize(); }; namespace @@ -509,6 +510,37 @@ void IncrementalSaveTest::signedFixtureIncrementalEditPreservesTheSignedByteRang QVERIFY(output.data().size() > originalData.size()); } +void IncrementalSaveTest::appendCostScalesWithChangedDataNotFileSize() +{ + const auto appendedBytesForPages = [](int pages) + { + pdf::PDFDocumentBuilder builder; + for (int index = 0; index < pages; ++index) + { + builder.appendPage(QRectF(0, 0, 595, 842)); + } + const pdf::PDFDocument original = builder.build(); + const QByteArray originalData = writeDocument(original); + const pdf::PDFDocumentPointer modified = createModifiedDocument(original); + pdf::PDFDocumentWriter writer(nullptr); + QBuffer output; + output.open(QIODevice::WriteOnly); + if (!writer.writeIncremental(&output, originalData, &original, modified.data())) + { + return QPair{ -1, -1 }; + } + return QPair{ output.data().size() - originalData.size(), originalData.size() }; + }; + + const QPair small = appendedBytesForPages(1); + const QPair large = appendedBytesForPages(60); + QVERIFY(small.first > 0); + QVERIFY(large.first > 0); + QVERIFY(large.second > small.second * 5); // the file really is much bigger + QVERIFY(large.first < large.second / 10); // the append is not proportional to size + QVERIFY(large.first < small.first * 4); // and it stays in the same order of magnitude +} + QTEST_MAIN(IncrementalSaveTest) #include "tst_incrementalsavetest.moc" From d6d568d339a8a27ae932588b8fc85275ea3f1451 Mon Sep 17 00:00:00 2001 From: mberrys Date: Mon, 14 Sep 2026 00:56:36 -0700 Subject: [PATCH 15/26] test(core): emit the signed incremental artifact for independent validation (#239) --- UnitTests/tst_incrementalsavetest.cpp | 31 +++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/UnitTests/tst_incrementalsavetest.cpp b/UnitTests/tst_incrementalsavetest.cpp index 9ddd48927..0275dcf1a 100644 --- a/UnitTests/tst_incrementalsavetest.cpp +++ b/UnitTests/tst_incrementalsavetest.cpp @@ -27,8 +27,12 @@ #include #include +#include #include +#include #include +#include +#include #include #include #include @@ -505,6 +509,33 @@ void IncrementalSaveTest::signedFixtureIncrementalEditPreservesTheSignedByteRang }; QCOMPARE(contentsOf(output.data()), contentsOf(originalData)); + const QString evidenceDirectory = QString::fromUtf8(qgetenv("LOOP_SAVE_POLICY_EVIDENCE_DIR")); + if (!evidenceDirectory.isEmpty()) + { + QDir().mkpath(evidenceDirectory); + QFile artifact(QStringLiteral("%1/incremental-with-signature.pdf").arg(evidenceDirectory)); + QVERIFY(artifact.open(QIODevice::WriteOnly)); + QCOMPARE(artifact.write(output.data()), qint64(output.data().size())); + artifact.close(); + + QJsonObject evidence{ + { QStringLiteral("schema"), QStringLiteral("loop.save-policy-incremental-evidence") }, + { QStringLiteral("schema_version"), 1 }, + { QStringLiteral("source_fixture"), QStringLiteral("UnitTests/testdata/signatures/signed-incremental-base.pdf") }, + { QStringLiteral("source_sha256"), QString::fromLatin1(QCryptographicHash::hash(originalData, QCryptographicHash::Sha256).toHex()) }, + { QStringLiteral("artifact_sha256"), QString::fromLatin1(QCryptographicHash::hash(output.data(), QCryptographicHash::Sha256).toHex()) }, + { QStringLiteral("source_bytes"), qint64(originalData.size()) }, + { QStringLiteral("artifact_bytes"), qint64(output.data().size()) }, + { QStringLiteral("appended_bytes"), qint64(output.data().size() - originalData.size()) }, + { QStringLiteral("original_prefix_preserved"), output.data().left(originalData.size()) == originalData }, + { QStringLiteral("byte_range_preserved"), true } + }; + QFile evidenceFile(QStringLiteral("%1/incremental-with-signature.json").arg(evidenceDirectory)); + QVERIFY(evidenceFile.open(QIODevice::WriteOnly)); + QCOMPARE(evidenceFile.write(QJsonDocument(evidence).toJson(QJsonDocument::Indented)), qint64(QJsonDocument(evidence).toJson(QJsonDocument::Indented).size())); + evidenceFile.close(); + } + // 3. The append really is an append: new xref pointing at the old one. QVERIFY(output.data().mid(originalData.size()).contains("/Prev")); QVERIFY(output.data().size() > originalData.size()); From b3c3fbb643e2fbb39befde8754b38e9480f4e060 Mon Sep 17 00:00:00 2001 From: mberrys Date: Mon, 14 Sep 2026 01:10:26 -0700 Subject: [PATCH 16/26] feat(cli): honour the declared save policy in add-bleed and rgb-to-cmyk (#239) add-bleed and rgb-to-cmyk wrote their candidate to the path the caller named with no policy guard, so `add-bleed received.pdf --output received.pdf --overwrite` rewrote the trusted input even though both operations declare saveAsNewArtifact. Move the request building and the refusal into one shared PDFToolAbstractApplication::validateOperationSaveRequest, route redact's inline block through it, and call it from add-bleed and rgb-to-cmyk with the policy their registry entry declares. A missing registry entry keeps the command working through the explicitly-undeclared policy. The guard runs before the document is read and before validateDestructiveOutput, so the refusal is not conditional on --overwrite and also covers --dry-run and --report. PdfTool/pdftoolrgbtocmyk.cpp was already dirty at HEAD for clang-format (case-labels, brace lists, namespace comments); formatted in this commit, no semantic change. --- PdfTool/pdftoolabstractapplication.cpp | 29 ++++++++++ PdfTool/pdftoolabstractapplication.h | 13 +++++ PdfTool/pdftooladdbleed.cpp | 14 +++++ PdfTool/pdftoolredact.cpp | 19 ++----- PdfTool/pdftoolrgbtocmyk.cpp | 78 +++++++++++++++++--------- docs/PDFTOOL_CLI_CONTRACT.md | 19 ++++++- 6 files changed, 129 insertions(+), 43 deletions(-) diff --git a/PdfTool/pdftoolabstractapplication.cpp b/PdfTool/pdftoolabstractapplication.cpp index e8a136632..8673459d7 100644 --- a/PdfTool/pdftoolabstractapplication.cpp +++ b/PdfTool/pdftoolabstractapplication.cpp @@ -22,6 +22,7 @@ #include "pdftoolabstractapplication.h" #include "pdfdocumentreader.h" +#include "pdfdocumentwriter.h" #include "pdfsafefilewriter.h" #include "pdfutils.h" #include "ocrsidecarprotocol.h" @@ -2674,4 +2675,32 @@ PDFToolExitCode PDFToolAbstractApplication::validateDestructiveOutputs(const PDF return PDFToolExitCode::Success; } +PDFToolExitCode PDFToolAbstractApplication::validateOperationSaveRequest(const PDFToolOptions& options, + const QString& sourcePath, + const QString& outputPath, + const pdf::PDFOperationSavePolicy& required, + bool appendInPlace) const +{ + pdf::PDFSaveRequest request; + request.sourcePath = sourcePath; + request.outputPath = outputPath; + request.required = required; + request.requested = required; + request.requestedExplicitly = true; + request.appendInPlace = appendInPlace; + + const pdf::PDFOperationResult validation = pdf::validateSaveRequest(request); + if (!validation) + { + // The Core message is the pinned contract text; translating it would + // break the contract, so it is reported verbatim. + reportDiagnostic(options, PDFToolDiagnosticSeverity::Error, QStringLiteral("save-policy.refused"), + validation.getErrorMessage(), + QJsonObject{ { QStringLiteral("path"), outputPath } }); + return PDFToolExitCode::ProcessingFailure; + } + + return PDFToolExitCode::Success; +} + } // pdftool diff --git a/PdfTool/pdftoolabstractapplication.h b/PdfTool/pdftoolabstractapplication.h index 6069ad5ee..0f1112b3d 100644 --- a/PdfTool/pdftoolabstractapplication.h +++ b/PdfTool/pdftoolabstractapplication.h @@ -38,6 +38,7 @@ #include "pdfrgbtocmykfixup.h" #include "pdfrepairdiff.h" #include "pdfrepairoperation.h" +#include "pdfsavepolicy.h" #include "pdfactionlist.h" #include @@ -489,6 +490,18 @@ class PDFToolAbstractApplication /// Returns PDFToolExitCode::Success when every write may proceed; otherwise an /// error value. PDFToolExitCode validateDestructiveOutputs(const PDFToolOptions& options, const QStringList& outputPaths) const; + + /// Holds the write to \p outputPath to the operation-declared \p required + /// policy. A request that would weaken it, or that would write over the + /// trusted input \p sourcePath unless \p appendInPlace is set, is reported as + /// a `save-policy.refused` error and answered with + /// PDFToolExitCode::ProcessingFailure. Returns PDFToolExitCode::Success when + /// the write may proceed. + PDFToolExitCode validateOperationSaveRequest(const PDFToolOptions& options, + const QString& sourcePath, + const QString& outputPath, + const pdf::PDFOperationSavePolicy& required, + bool appendInPlace = false) const; }; /// This class stores information about all applications available. Application diff --git a/PdfTool/pdftooladdbleed.cpp b/PdfTool/pdftooladdbleed.cpp index 372b23478..bf44d0d2f 100644 --- a/PdfTool/pdftooladdbleed.cpp +++ b/PdfTool/pdftooladdbleed.cpp @@ -262,6 +262,20 @@ PDFToolExitCode PDFToolAddBleed::execute(const PDFToolOptions& options) return PDFToolExitCode::InvalidInvocation; } + // The operation, not the command, declares what may happen to the trusted + // input. A missing declaration must not delete the command, so the + // explicitly-undeclared policy still protects the input path. + const pdf::PDFRepairOperation* const declaredOperation = pdf::PDFRepairRegistry::instance().find(QStringLiteral("add-bleed")); + const pdf::PDFOperationSavePolicy declaredPolicy = declaredOperation ? declaredOperation->savePolicy() : pdf::PDFOperationSavePolicy::undeclared(); + if (const PDFToolExitCode refused = validateOperationSaveRequest(options, + options.document, + options.addBleedOutputDocument, + declaredPolicy); + refused != PDFToolExitCode::Success) + { + return refused; + } + pdf::PDFDocument document; QByteArray sourceData; if (!readDocument(options, document, &sourceData, false)) diff --git a/PdfTool/pdftoolredact.cpp b/PdfTool/pdftoolredact.cpp index 281c5029d..e320c0637 100644 --- a/PdfTool/pdftoolredact.cpp +++ b/PdfTool/pdftoolredact.cpp @@ -64,20 +64,13 @@ PDFToolExitCode PDFToolRedact::execute(const PDFToolOptions& options) // Redaction removes prior content, so it declares a full rewrite and never // persists over the input it was handed. Rejected before the document is // read: the refusal cannot depend on how much work the redaction would do. - pdf::PDFSaveRequest saveRequest; - saveRequest.sourcePath = options.document; - saveRequest.outputPath = options.redactedDocument; - saveRequest.required = pdf::PDFOperationSavePolicy::fullRewrite(QStringLiteral("redaction removes prior content")); - saveRequest.requested = saveRequest.required; - saveRequest.requestedExplicitly = true; - saveRequest.appendInPlace = false; - const pdf::PDFOperationResult saveValidation = pdf::validateSaveRequest(saveRequest); - if (!saveValidation) + if (const PDFToolExitCode refused = validateOperationSaveRequest(options, + options.document, + options.redactedDocument, + pdf::PDFOperationSavePolicy::fullRewrite(QStringLiteral("redaction removes prior content"))); + refused != PDFToolExitCode::Success) { - reportDiagnostic(options, PDFToolDiagnosticSeverity::Error, QStringLiteral("save-policy.refused"), - saveValidation.getErrorMessage(), - QJsonObject{ { QStringLiteral("path"), options.redactedDocument } }); - return PDFToolExitCode::ProcessingFailure; + return refused; } pdf::PDFDocument document; diff --git a/PdfTool/pdftoolrgbtocmyk.cpp b/PdfTool/pdftoolrgbtocmyk.cpp index 0a46b8813..ec27c5170 100644 --- a/PdfTool/pdftoolrgbtocmyk.cpp +++ b/PdfTool/pdftoolrgbtocmyk.cpp @@ -65,14 +65,22 @@ QString objectKindName(pdf::PDFRgbToCmykObjectKind kind) { switch (kind) { - case pdf::PDFRgbToCmykObjectKind::VectorPaint: return QStringLiteral("vector-paint"); - case pdf::PDFRgbToCmykObjectKind::Image: return QStringLiteral("image"); - case pdf::PDFRgbToCmykObjectKind::InlineImage: return QStringLiteral("inline-image"); - case pdf::PDFRgbToCmykObjectKind::Form: return QStringLiteral("form"); - case pdf::PDFRgbToCmykObjectKind::AnnotationAppearance: return QStringLiteral("annotation-appearance"); - case pdf::PDFRgbToCmykObjectKind::IndexedPalette: return QStringLiteral("indexed-palette"); - case pdf::PDFRgbToCmykObjectKind::Shading: return QStringLiteral("shading"); - case pdf::PDFRgbToCmykObjectKind::Pattern: return QStringLiteral("pattern"); + case pdf::PDFRgbToCmykObjectKind::VectorPaint: + return QStringLiteral("vector-paint"); + case pdf::PDFRgbToCmykObjectKind::Image: + return QStringLiteral("image"); + case pdf::PDFRgbToCmykObjectKind::InlineImage: + return QStringLiteral("inline-image"); + case pdf::PDFRgbToCmykObjectKind::Form: + return QStringLiteral("form"); + case pdf::PDFRgbToCmykObjectKind::AnnotationAppearance: + return QStringLiteral("annotation-appearance"); + case pdf::PDFRgbToCmykObjectKind::IndexedPalette: + return QStringLiteral("indexed-palette"); + case pdf::PDFRgbToCmykObjectKind::Shading: + return QStringLiteral("shading"); + case pdf::PDFRgbToCmykObjectKind::Pattern: + return QStringLiteral("pattern"); } return QStringLiteral("unknown"); } @@ -83,20 +91,18 @@ QJsonObject reportObject(const pdf::PDFRgbToCmykSettings& settings, QJsonObject result; result.insert(QStringLiteral("command"), QStringLiteral("rgb-to-cmyk")); result.insert(QStringLiteral("target_profile"), QJsonObject{ - { QStringLiteral("name"), settings.targetProfileName }, - { QStringLiteral("bytes"), settings.targetIccData.size() } - }); + { QStringLiteral("name"), settings.targetProfileName }, + { QStringLiteral("bytes"), settings.targetIccData.size() } }); result.insert(QStringLiteral("intent"), int(settings.intent)); result.insert(QStringLiteral("black_point_compensation"), settings.blackPointCompensation); result.insert(QStringLiteral("converted"), QJsonObject{ - { QStringLiteral("vector_paints"), report.vectorPaintsConverted }, - { QStringLiteral("images"), report.imagesConverted }, - { QStringLiteral("indexed_palettes"), report.indexedPalettesConverted }, - { QStringLiteral("forms"), report.formsVisited }, - { QStringLiteral("annotation_appearances"), report.annotationAppearancesVisited } - }); + { QStringLiteral("vector_paints"), report.vectorPaintsConverted }, + { QStringLiteral("images"), report.imagesConverted }, + { QStringLiteral("indexed_palettes"), report.indexedPalettesConverted }, + { QStringLiteral("forms"), report.formsVisited }, + { QStringLiteral("annotation_appearances"), report.annotationAppearancesVisited } }); result.insert(QStringLiteral("unsupported"), [&report] - { + { QJsonArray values; for (const pdf::PDFRgbToCmykUnsupportedItem& item : report.unsupported) { @@ -106,23 +112,26 @@ QJsonObject reportObject(const pdf::PDFRgbToCmykSettings& settings, { QStringLiteral("reason"), item.reason } }); } - return values; - }()); + return values; }()); result.insert(QStringLiteral("output_intent_changed"), report.outputIntentChanged); result.insert(QStringLiteral("postflight_passed"), report.postflightPassed); return result; } -} // namespace +} // namespace QString PDFToolRgbToCmyk::getStandardString(StandardString standardString) const { switch (standardString) { - case Command: return QStringLiteral("rgb-to-cmyk"); - case Name: return PDFToolTranslationContext::tr("RGB to CMYK"); - case Description: return PDFToolTranslationContext::tr("Convert RGB PDF content through LittleCMS to a selected CMYK output condition."); - default: break; + case Command: + return QStringLiteral("rgb-to-cmyk"); + case Name: + return PDFToolTranslationContext::tr("RGB to CMYK"); + case Description: + return PDFToolTranslationContext::tr("Convert RGB PDF content through LittleCMS to a selected CMYK output condition."); + default: + break; } return QString(); } @@ -152,7 +161,8 @@ PDFToolExitCode PDFToolRgbToCmyk::execute(const PDFToolOptions& options) settings.targetProfileName = QFileInfo(settings.targetProfileName).completeBaseName(); const QString sourceProfilePath = options.rgbToCmykSettings.fallbackRgbIccId.isEmpty() - ? QString() : QString::fromUtf8(options.rgbToCmykSettings.fallbackRgbIccId); + ? QString() + : QString::fromUtf8(options.rgbToCmykSettings.fallbackRgbIccId); if (!sourceProfilePath.isEmpty()) { if (!readProfile(sourceProfilePath, &settings.fallbackRgbIccData, &profileError)) @@ -162,6 +172,20 @@ PDFToolExitCode PDFToolRgbToCmyk::execute(const PDFToolOptions& options) } } + // The operation, not the command, declares what may happen to the trusted + // input. A missing declaration must not delete the command, so the + // explicitly-undeclared policy still protects the input path. + const pdf::PDFRepairOperation* const declaredOperation = pdf::PDFRepairRegistry::instance().find(QStringLiteral("rgb-to-cmyk")); + const pdf::PDFOperationSavePolicy declaredPolicy = declaredOperation ? declaredOperation->savePolicy() : pdf::PDFOperationSavePolicy::undeclared(); + if (const PDFToolExitCode refused = validateOperationSaveRequest(options, + options.document, + options.rgbToCmykOutputDocument, + declaredPolicy); + refused != PDFToolExitCode::Success) + { + return refused; + } + pdf::PDFDocument document; if (!readDocument(options, document, nullptr, false)) { @@ -236,4 +260,4 @@ PDFToolAbstractApplication::Options PDFToolRgbToCmyk::getOptionsFlags() const return ConsoleFormat | OpenDocument | PageSelector | ColorManagementSystem | RgbToCmyk | DestructiveWrite; } -} // namespace pdftool +} // namespace pdftool diff --git a/docs/PDFTOOL_CLI_CONTRACT.md b/docs/PDFTOOL_CLI_CONTRACT.md index cf70850f3..fe83d7d5d 100644 --- a/docs/PDFTOOL_CLI_CONTRACT.md +++ b/docs/PDFTOOL_CLI_CONTRACT.md @@ -144,16 +144,29 @@ human-facing stderr behavior is preserved. A command that declares a save mode also declares what that mode has to guarantee. `redact` removes prior content, so its result is always a full -rewrite and never an append: the guard runs before the document is read, so an -incompatible request is rejected before any content is touched. +rewrite and never an append. `add-bleed` and `rgb-to-cmyk` are corrective +commands: they take the policy from their own operation registration +(`PDFRepairRegistry::instance().find()->savePolicy()`) instead of repeating +the rationale in the CLI, so the declaration is the single authority and the +command cannot talk the operation out of it. The guard runs before the document +is read, so an incompatible request is rejected before any content is touched. | Command | `code` | Exit | Guard | `message` | |---|---|---|---|---| | `redact` | `save-policy.refused` | `4 processing-failure` | Redaction removes prior content, so the output must be a full rewrite written to a path other than the trusted input. | `Refused save: '' is the trusted input artifact; write the candidate to a new path.` | +| `add-bleed` | `save-policy.refused` | `4 processing-failure` | The operation declares `saveAsNewArtifact` ("bleed correction must preserve the trusted source"), so the candidate must be written to a path other than the trusted input. | `Refused save: '' is the trusted input artifact; write the candidate to a new path.` | +| `rgb-to-cmyk` | `save-policy.refused` | `4 processing-failure` | The operation declares `saveAsNewArtifact` ("color conversion creates a production candidate"), so the candidate must be written to a path other than the trusted input. | `Refused save: '' is the trusted input artifact; write the candidate to a new path.` | `context` carries the refused output path as `path`. The diagnostic is an `error`, the run records no output, and the input file is left byte-identical: -`PdfTool redact received.pdf received.pdf` is never a successful invocation. +`PdfTool redact received.pdf received.pdf`, +`PdfTool add-bleed received.pdf --output received.pdf`, and +`PdfTool rgb-to-cmyk received.pdf --output received.pdf --target-profile ` +are never successful invocations. + +The refusal is not conditional on `--overwrite`, which only authorises replacing +an existing candidate, and it applies in `--dry-run` and `--report` mode too: an +invocation that can never succeed must not be reported as a plan. ### Empty results From c99475550bbdf6c43f902bae8739bdc90753338b Mon Sep 17 00:00:00 2001 From: mberrys Date: Mon, 14 Sep 2026 01:10:40 -0700 Subject: [PATCH 17/26] test(cli): pin the corrective-command save-policy refusals (#239) Cover add-bleed and rgb-to-cmyk the way redact is covered: the input path passed as its own output is refused with exit 4 and `save-policy.refused`, the Core message is the pinned trusted-input text, no output is recorded and the input stays byte-identical. The positive control in each slot re-runs the same invocation against a distinct output path and requires exit 0, so the guard cannot be satisfied by refusing the command outright. Both slots fail when the add-bleed guard is deleted (verified by mutation). --- UnitTests/tst_pdftoolcontract.cpp | 104 ++++++++++++++++++++++++++++++ 1 file changed, 104 insertions(+) diff --git a/UnitTests/tst_pdftoolcontract.cpp b/UnitTests/tst_pdftoolcontract.cpp index 076586f5c..d286533ed 100644 --- a/UnitTests/tst_pdftoolcontract.cpp +++ b/UnitTests/tst_pdftoolcontract.cpp @@ -22,6 +22,7 @@ #include "processoutputcapture.h" +#include #include #include #include @@ -111,6 +112,8 @@ private slots: void preflightRejectsNonJsonOutput(); void preflightKeepsNestedReportBoundary(); void redactRefusesToWriteOverItsOwnInput(); + void addBleedRefusesToWriteOverItsOwnInput(); + void rgbToCmykRefusesToWriteOverItsOwnInput(); }; void PdfToolContractTest::helpIsWrapped() @@ -243,6 +246,19 @@ QJsonObject findDiagnostic(const ToolRun& run, const QString& code) return QJsonObject(); } +QByteArray fileDigest(const QString& path) +{ + QFile file(path); + if (!file.open(QIODevice::ReadOnly)) + { + return QByteArray(); + } + + QCryptographicHash hash(QCryptographicHash::Sha256); + hash.addData(&file); + return hash.result(); +} + } // namespace void PdfToolContractTest::fetchImagesOnVectorOnlyDocumentNotesEmptyResult() @@ -352,6 +368,94 @@ void PdfToolContractTest::redactRefusesToWriteOverItsOwnInput() QVERIFY(findDiagnostic(legitimate, QStringLiteral("save-policy.refused")).isEmpty()); } +void PdfToolContractTest::addBleedRefusesToWriteOverItsOwnInput() +{ + // add-bleed declares saveAsNewArtifact("bleed correction must preserve the + // trusted source"), so a corrective run may not hand the input back as its + // own output. + QTemporaryDir directory; + QVERIFY(directory.isValid()); + const QString inputPath = directory.filePath(QStringLiteral("received.pdf")); + const QString fixture = + QDir(QStringLiteral(LOOP_PREFLIGHT_SOURCE_DIR)).filePath(QStringLiteral("testdata/fixtures/color-rgb.pdf")); + QVERIFY2(QFile::copy(fixture, inputPath), qPrintable(fixture)); + const QByteArray inputDigest = fileDigest(inputPath); + QVERIFY(!inputDigest.isEmpty()); + + const ToolRun run = runPdfTool({ QStringLiteral("add-bleed"), + QStringLiteral("--console-format"), QStringLiteral("json"), + QStringLiteral("--overwrite"), + inputPath, + QStringLiteral("--output"), inputPath }); + + verifyEnvelope(run, 4, QStringLiteral("add-bleed")); + const QJsonObject diagnostic = findDiagnostic(run, QStringLiteral("save-policy.refused")); + QVERIFY2(!diagnostic.isEmpty(), qPrintable(QString::fromUtf8(run.stdoutData))); + QCOMPARE(diagnostic.value(QStringLiteral("severity")).toString(), QStringLiteral("error")); + QVERIFY(diagnostic.value(QStringLiteral("message")).toString().contains(QStringLiteral("trusted input artifact"))); + QCOMPARE(diagnostic.value(QStringLiteral("context")).toObject().value(QStringLiteral("path")).toString(), inputPath); + QVERIFY(run.json.value(QStringLiteral("outputs")).toArray().isEmpty()); + QCOMPARE(fileDigest(inputPath), inputDigest); + + // The refusal has to be about the destination, not about add-bleed: the + // same document and the same caller still produce a candidate at a distinct + // path. + const QString candidatePath = directory.filePath(QStringLiteral("candidate.pdf")); + const ToolRun legitimate = runPdfTool({ QStringLiteral("add-bleed"), + QStringLiteral("--console-format"), QStringLiteral("json"), + inputPath, + QStringLiteral("--output"), candidatePath }); + QCOMPARE(legitimate.exitCode, 0); + QVERIFY(findDiagnostic(legitimate, QStringLiteral("save-policy.refused")).isEmpty()); + QVERIFY(QFile(candidatePath).exists()); +} + +void PdfToolContractTest::rgbToCmykRefusesToWriteOverItsOwnInput() +{ + // rgb-to-cmyk declares saveAsNewArtifact("color conversion creates a + // production candidate"), so the candidate may not replace the input even + // when the caller asks for --overwrite. + const QString profilePath = QFINDTESTDATA("testdata/synthetic-cmyk.icc"); + QVERIFY2(!profilePath.isEmpty() && QFile::exists(profilePath), qPrintable(profilePath)); + + QTemporaryDir directory; + QVERIFY(directory.isValid()); + const QString inputPath = directory.filePath(QStringLiteral("received.pdf")); + const QString fixture = + QDir(QStringLiteral(LOOP_PREFLIGHT_SOURCE_DIR)).filePath(QStringLiteral("testdata/fixtures/output-intent-rgb.pdf")); + QVERIFY2(QFile::copy(fixture, inputPath), qPrintable(fixture)); + const QByteArray inputDigest = fileDigest(inputPath); + QVERIFY(!inputDigest.isEmpty()); + + const ToolRun run = runPdfTool({ QStringLiteral("rgb-to-cmyk"), + QStringLiteral("--console-format"), QStringLiteral("json"), + QStringLiteral("--overwrite"), + inputPath, + QStringLiteral("--output"), inputPath, + QStringLiteral("--target-profile"), profilePath }); + + verifyEnvelope(run, 4, QStringLiteral("rgb-to-cmyk")); + const QJsonObject diagnostic = findDiagnostic(run, QStringLiteral("save-policy.refused")); + QVERIFY2(!diagnostic.isEmpty(), qPrintable(QString::fromUtf8(run.stdoutData))); + QCOMPARE(diagnostic.value(QStringLiteral("severity")).toString(), QStringLiteral("error")); + QVERIFY(diagnostic.value(QStringLiteral("message")).toString().contains(QStringLiteral("trusted input artifact"))); + QCOMPARE(diagnostic.value(QStringLiteral("context")).toObject().value(QStringLiteral("path")).toString(), inputPath); + QVERIFY(run.json.value(QStringLiteral("outputs")).toArray().isEmpty()); + QCOMPARE(fileDigest(inputPath), inputDigest); + + // The refusal has to be about the destination, not about the conversion: + // the same document and the same caller still convert at a distinct path. + const QString candidatePath = directory.filePath(QStringLiteral("candidate.pdf")); + const ToolRun legitimate = runPdfTool({ QStringLiteral("rgb-to-cmyk"), + QStringLiteral("--console-format"), QStringLiteral("json"), + inputPath, + QStringLiteral("--output"), candidatePath, + QStringLiteral("--target-profile"), profilePath }); + QCOMPARE(legitimate.exitCode, 0); + QVERIFY(findDiagnostic(legitimate, QStringLiteral("save-policy.refused")).isEmpty()); + QVERIFY(QFile(candidatePath).exists()); +} + } // namespace QTEST_MAIN(PdfToolContractTest) From 14fe3300c38aca10a10f43b4dcf212e00bdf96ec Mon Sep 17 00:00:00 2001 From: mberrys Date: Mon, 14 Sep 2026 01:20:15 -0700 Subject: [PATCH 18/26] ci: validate the signed incremental artifact with qpdf and pdfsig (#239) The reusable-linux.yml job now installs qpdf and poppler-utils, runs UnitTestsIncrementalSave with LOOP_SAVE_POLICY_EVIDENCE_DIR set so the signed incremental artifact is emitted, runs scripts/qualification/run_independent_validators.py with --claim structural and --claim signature, asserts inline that the run is passed and that every per-claim status is passed, and uploads the evidence JSON as the save-policy-independent-validation workflow artifact. The passing run is produced by the reusable-linux.yml job (step "Prove signed incremental save with independent validators") and is pending the first dispatch; CI cannot commit the evidence itself. This commit therefore records the honest local run at docs/evidence/session-15-save-policy/independent-validation-local.json (status incomplete, reason_code validator-not-installed for both claims), which never counts as a pass per docs/INDEPENDENT_VALIDATION.md. docs/INCREMENTAL_SAVE.md no longer claims the repository carries no signing fixture: the fixture, its generator, the byte-range and cost slots, and the CI-owned structural/signature claims are named. --- .github/workflows/reusable-linux.yml | 38 ++++++++++++++++++ docs/INCREMENTAL_SAVE.md | 23 +++++++++-- .../independent-validation-local.json | 39 +++++++++++++++++++ 3 files changed, 97 insertions(+), 3 deletions(-) create mode 100644 docs/evidence/session-15-save-policy/independent-validation-local.json diff --git a/.github/workflows/reusable-linux.yml b/.github/workflows/reusable-linux.yml index a3d4fccf1..318d88649 100644 --- a/.github/workflows/reusable-linux.yml +++ b/.github/workflows/reusable-linux.yml @@ -408,6 +408,44 @@ jobs: run: | ctest --output-on-failure + - name: Install independent validators + if: ${{ !inputs.fast }} + run: | + sudo apt update + sudo apt install -y qpdf poppler-utils + + - name: Prove signed incremental save with independent validators + if: ${{ !inputs.fast }} + working-directory: loop + env: + QT_QPA_PLATFORM: offscreen + run: | + set -euo pipefail + export LOOP_SAVE_POLICY_EVIDENCE_DIR="$PWD/evidence/save-policy" + rm -rf "$LOOP_SAVE_POLICY_EVIDENCE_DIR" + ctest --test-dir build -R '^UnitTestsIncrementalSave$' --output-on-failure + mkdir -p docs/evidence/session-15-save-policy + python3 scripts/qualification/run_independent_validators.py \ + --input "$LOOP_SAVE_POLICY_EVIDENCE_DIR/incremental-with-signature.pdf" \ + --output docs/evidence/session-15-save-policy/independent-validation-linux.json \ + --candidate-sha "$(git rev-parse HEAD)" \ + --claim structural --claim signature + python3 - <<'PY' + import json + data = json.load(open("docs/evidence/session-15-save-policy/independent-validation-linux.json")) + assert data["status"] == "passed", data["status"] + assert all(v["status"] == "passed" for v in data["validators"]), data["validators"] + print("independent validation evidence: passed", [v["claim"] for v in data["validators"]]) + PY + + - name: Upload save-policy independent validation evidence + if: ${{ !inputs.fast }} + uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4 + with: + name: save-policy-independent-validation + path: loop/docs/evidence/session-15-save-policy/independent-validation-linux.json + if-no-files-found: error + - name: Run preflight corpus gate if: ${{ !inputs.fast }} working-directory: loop/build diff --git a/docs/INCREMENTAL_SAVE.md b/docs/INCREMENTAL_SAVE.md index b8b465176..42805a058 100644 --- a/docs/INCREMENTAL_SAVE.md +++ b/docs/INCREMENTAL_SAVE.md @@ -53,6 +53,23 @@ outside this save path. The focused `UnitTestsIncrementalSave` target checks prefix preservation, changed-object visibility after reopening, source-byte mismatch refusal, and -the write-mode policy. Signed-fixture validation and veraPDF validation remain -external release/CI checks because this repository does not carry a signing -fixture or the veraPDF runtime. +the write-mode policy. A real signing fixture is carried in the repository at +`UnitTests/testdata/signatures/signed-incremental-base.pdf`, generated by +`scripts/qualification/make_signed_fixture.py` (the private key stays in a +temporary directory; only the signed PDF and its `manifest.json` are +committed). The byte-range claim is pinned by +`IncrementalSaveTest::signedFixtureIncrementalEditPreservesTheSignedByteRange` +and the cost claim by +`IncrementalSaveTest::appendCostScalesWithChangedDataNotFileSize`. + +The structural and signature claims over the artifact that test emits are made +by external validators, not by Loop's own parser: the `reusable-linux.yml` job +installs `qpdf` and `poppler-utils`, runs `UnitTestsIncrementalSave` with +`LOOP_SAVE_POLICY_EVIDENCE_DIR` set, runs +`scripts/qualification/run_independent_validators.py` with `--claim structural +--claim signature`, and stores the evidence under +`docs/evidence/session-15-save-policy/`. A run on a host without those tools +records `incomplete` with `reason_code: validator-not-installed`, which never +counts as a pass (see [INDEPENDENT_VALIDATION.md](INDEPENDENT_VALIDATION.md)). +veraPDF conformance (`--claim standards`) remains an external release check +because this repository does not carry the veraPDF runtime. diff --git a/docs/evidence/session-15-save-policy/independent-validation-local.json b/docs/evidence/session-15-save-policy/independent-validation-local.json new file mode 100644 index 000000000..2155bfb2b --- /dev/null +++ b/docs/evidence/session-15-save-policy/independent-validation-local.json @@ -0,0 +1,39 @@ +{ + "schema": "loop.independent-validation-evidence", + "schema_version": 1, + "status": "incomplete", + "input": { + "path": "C:\\Users\\micha\\AppData\\Local\\Temp\\save-policy-evidence\\incremental-with-signature.pdf", + "bytes": 7624, + "sha256": "fc573865e74766af2fac4d24fd053ea7232fbfa42d784ea6e81ac09d62917ff5" + }, + "validators": [ + { + "claim": "structural", + "program": "qpdf", + "program_path": null, + "configured_arguments": [ + "--check", + "{input}" + ], + "status": "incomplete", + "reason_code": "validator-not-installed" + }, + { + "claim": "signature", + "program": "pdfsig", + "program_path": null, + "configured_arguments": [ + "{input}" + ], + "status": "incomplete", + "reason_code": "validator-not-installed" + } + ], + "platform": { + "system": "Windows", + "release": "10", + "machine": "AMD64" + }, + "candidate_sha": "c99475550bbdf6c43f902bae8739bdc90753338b" +} From 351c2c79cc4186587b398e543d7160abfd45a891 Mon Sep 17 00:00:00 2001 From: mberrys Date: Mon, 14 Sep 2026 01:24:33 -0700 Subject: [PATCH 19/26] test(core): pin that no save path records an approved output (#239) Pinned invariant, not a defect fix: nothing in the tree sets an approval kind or a rollback approved-output flag today, so the new OperationHistoryTest::noSavePathProducesAnApprovedOutputRecord slot passes on first run and guards the 0.3.0-B approval workflow (LOUPE-53). Before this commit the suite had 16 slots and none of them named the invariant. Two plan snippets were wrong against the code and are corrected here (the assertions are stronger than the plan's, not weaker): - PDFOperationHistoryEvent has no operationId member (it belongs to PDFOperationHistoryExecution, pdfoperationhistory.h:100), so that line is dropped; the operation identity stays pinned on PDFRollbackPoint::operationId, which does exist. - QVERIFY(!approval.isValid()) is false: PDFApprovalRecord::isValid() means "well formed", and PDFOperationHistoryStore::appendEvent refuses any event whose approval is invalid (pdfoperationhistorystore.cpp:381), so the default record must be valid. The pin is therefore kind == None plus every approval field empty, plus a positive control that a record claiming a Human decision without actor, decision or decision time is invalid. The gate that keeps a default record out of the rollback path is isValid() && kind != None (pdfoperationhistorystore.cpp:790). Mutation evidence for non-vacuity: flipping PDFRollbackPoint::approvedOutput's default to true fails exactly this slot ('!point.approvedOutput' returned FALSE, 16 passed / 1 failed) and nothing else; reverted, the suite is 17 passed, 0 failed, 0 skipped. --- UnitTests/tst_operationhistorytest.cpp | 44 ++++++++++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/UnitTests/tst_operationhistorytest.cpp b/UnitTests/tst_operationhistorytest.cpp index 2e5eca731..a35c5b35e 100644 --- a/UnitTests/tst_operationhistorytest.cpp +++ b/UnitTests/tst_operationhistorytest.cpp @@ -47,6 +47,7 @@ private slots: void canonicalJsonIsStableAndRedacted(); void artifactStoreStreamsAndDetectsTampering(); void importedInputIsReadOnlyAndDigestAddressed(); + void noSavePathProducesAnApprovedOutputRecord(); void lifecycleApprovalAndRollbackResolution(); void rollbackPointsRetentionAndAtomicity(); void externalPayloadTamperingCompromisesChain(); @@ -848,5 +849,48 @@ void OperationHistoryTest::importedInputIsReadOnlyAndDigestAddressed() QCOMPARE(QFile(inboxPath).size(), qint64(inboxBytes.size())); } +void OperationHistoryTest::noSavePathProducesAnApprovedOutputRecord() +{ + // Pinned invariant: no save path records an approval. Nothing in this tree + // ever sets an approval kind or the rollback approved-output flag. + // PDFApprovalRecord::isValid() means "well formed", not "carries an + // approval": appendEvent rejects an event whose approval is invalid + // (pdfoperationhistorystore.cpp:381), so the default record is valid and is + // separated from a real approval by its None kind and empty payload + // (pdfoperationhistorystore.cpp:790). + const pdf::PDFApprovalRecord unapproved; + QVERIFY(unapproved.isValid()); + QCOMPARE(unapproved.kind, pdf::PDFApprovalKind::None); + QVERIFY(unapproved.actorId.isEmpty()); + QVERIFY(unapproved.decision.isEmpty()); + QVERIFY(unapproved.policyId.isEmpty()); + QVERIFY(unapproved.rationale.isEmpty()); + QVERIFY(unapproved.evidenceSha256.isEmpty()); + QVERIFY(!unapproved.decidedUtc.isValid()); + + pdf::PDFOperationHistoryEvent planned; + planned.status = pdf::PDFOperationHistoryStatus::Planned; + QCOMPARE(planned.approval.kind, pdf::PDFApprovalKind::None); + QCOMPARE(planned.approval.toJson(), unapproved.toJson()); + + // A record that claims an approval decision but carries no actor, decision + // or decision time is not a valid approval: one cannot be fabricated. + pdf::PDFApprovalRecord claimed; + claimed.kind = pdf::PDFApprovalKind::Human; + QVERIFY(!claimed.isValid()); + + pdf::PDFRollbackPoint point; + point.operationId = QStringLiteral("add-bleed"); + point.planSummary = QStringLiteral("planned candidate"); + QVERIFY(!point.approvedOutput); + QVERIFY(!point.toJson().value(QStringLiteral("approvedOutput")).toBool()); + + // A candidate artifact is not an approved output merely because it exists. + pdf::PDFRollbackPoint candidate = point; + candidate.isOriginalInput = false; + QVERIFY(!candidate.approvedOutput); + QVERIFY(!candidate.toJson().value(QStringLiteral("approvedOutput")).toBool()); +} + QTEST_MAIN(OperationHistoryTest) #include "tst_operationhistorytest.moc" From 37984f0471cf0383c475916c2404a5912c6acce1 Mon Sep 17 00:00:00 2001 From: mberrys Date: Mon, 14 Sep 2026 01:25:25 -0700 Subject: [PATCH 20/26] docs: state the true state of editor crash recovery (#239) docs/EDITOR_RECOVERY.md described a recovery service that is not in the tree. It now states that 2a19e2c1 deleted LoupeLibGui/pdfrecoverymanager.{h,cpp}; that the rest of the document is the contract to restore, not shipped behaviour; that UnitTests/tst_recoverytest.cpp compiles against a header that does not exist and is registered in no CMake target, so it neither compiles nor runs; that the approval half of the 0.3.0-A criterion is pinned by UnitTestsOperationHistory::noSavePathProducesAnApprovedOutputRecord; and that the restore half is tracked by #575. UnitTests/tst_recoverytest.cpp gains a // NOT COMPILED: header comment under the license block so the orphan cannot be mistaken for a running test. The file is kept as the specification and is deliberately still not added to a CMake target. python scripts/ci/check_source_integrity.py: "Source integrity policy passed." (exit 0). --- UnitTests/tst_recoverytest.cpp | 6 ++++++ docs/EDITOR_RECOVERY.md | 31 ++++++++++++++++++++++++------- 2 files changed, 30 insertions(+), 7 deletions(-) diff --git a/UnitTests/tst_recoverytest.cpp b/UnitTests/tst_recoverytest.cpp index b36b9676c..9b776ad74 100644 --- a/UnitTests/tst_recoverytest.cpp +++ b/UnitTests/tst_recoverytest.cpp @@ -20,6 +20,12 @@ // OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE // SOFTWARE. +// NOT COMPILED: this file specifies the source-identity and policy-clamp +// behaviour of the Editor recovery service, which does not exist in this tree +// (deleted with the Widgets libraries in 2a19e2c1) and is registered in no +// CMake target. Keep it as the specification; restore it together with +// LoopEditor's recovery manager. See docs/EDITOR_RECOVERY.md. + #include "pdfrecoverymanager.h" #include diff --git a/docs/EDITOR_RECOVERY.md b/docs/EDITOR_RECOVERY.md index 756491ebb..95b85f771 100644 --- a/docs/EDITOR_RECOVERY.md +++ b/docs/EDITOR_RECOVERY.md @@ -1,8 +1,22 @@ # Editor crash/session recovery -Loop protects unsaved Editor work with a private, bounded recovery store. The -store is owned by `PDFRecoveryManager` in the Editor/Core recovery boundary and is attached to the -single-document Editor session. +**The recovery service described below does not exist in this tree.** It was +deleted by `2a19e2c1` ("delete Widgets libraries and plugin pack for Session 05 +Issue 17"), which removed `LoupeLibGui/pdfrecoverymanager.{h,cpp}`. Everything +below is the contract to restore, not a description of shipped code. + +`UnitTests/tst_recoverytest.cpp` is kept as the specification of the +source-identity and policy-clamp behaviour, but it includes +`pdfrecoverymanager.h`, a header that exists nowhere in the tree, and it is +registered in no CMake target. It therefore is not compiled and does not run. + +The 0.3.0-A requirement ("crash recovery restores workspace/revision state +without presenting the recovered file as an approved production artifact") is +only half reachable today: the approval half is pinned by +`UnitTestsOperationHistory::noSavePathProducesAnApprovedOutputRecord` (no save +path records an approval or an approved output, so a recovered file cannot be +presented as approved), and the restore half is tracked by +[#575](https://github.com/studio-berry/loop/issues/575). ## Safety contract @@ -40,7 +54,10 @@ Retention defaults to 14 days, 20 sessions, and 2 GiB. Cleanup runs after classification and excludes active sessions. Invalid/stale candidates can be discarded from the startup dialog without being opened. -`UnitTestsRecovery` covers source replacement/missing classification and policy -clamping. The service boundaries are deterministic and ready for injected fake -clock/filesystem crash-point tests; process-kill GUI coverage belongs with the -GUI/E2E harness tracked separately. +No running test covers recovery: the `RecoveryTest` slots +`sourceIdentityDetectsReplacement` and `policyClampsUnsafeValues` are the +specification for source replacement/missing classification and policy clamping, +but their file is in no CMake target and does not compile against the current +tree. Restoring the service means restoring the manager and wiring that test up +in the same change, including the process-kill GUI coverage that belongs with +the GUI/E2E harness. From 56060a971bd464e73d330f8d4dbcd262781ee98a Mon Sep 17 00:00:00 2001 From: mberrys Date: Mon, 14 Sep 2026 01:27:52 -0700 Subject: [PATCH 21/26] docs: document operation-owned save policy strength and its proofs (#239) --- docs/INCREMENTAL_SAVE.md | 30 ++++++++++++++++++++++++++++-- docs/INDEPENDENT_VALIDATION.md | 9 +++++++++ 2 files changed, 37 insertions(+), 2 deletions(-) diff --git a/docs/INCREMENTAL_SAVE.md b/docs/INCREMENTAL_SAVE.md index 42805a058..e68c9db6b 100644 --- a/docs/INCREMENTAL_SAVE.md +++ b/docs/INCREMENTAL_SAVE.md @@ -31,14 +31,40 @@ operation defaults to new-artifact rather than being silently appended. - Redaction, sanitization, and other destructive operations must use the full writer. The redaction verifier continues to reject a redacted output that contains `/Prev`. -- A new-artifact policy cannot overwrite the trusted source; the Editor routes - it to Save As and rejects an attempt to use the source path as the output. +- A new-artifact policy cannot overwrite the trusted source: Core refuses a candidate + write whose output resolves to the transaction's `sourcePath`, and PdfTool refuses + the same path for the corrective commands it drives. - If the source cannot be read again, the interactive save is refused rather than risking a full rewrite of a signed or revisioned source. - A changed signature dictionary, removed object slot, source-byte mismatch, or encryption-mode change causes incremental save to fail and requires a full rewrite or an explicit user-facing recovery path. +### Policy strength + +The declared policy is a floor, not a hint. `savePolicyIsWeaker()` compares a +caller's request with the operation's declaration: a weaker mode, an unstated +signature loss, or a claimed reversibility the operation does not have. A +strictly stronger mode is never weaker, so asking for more safety is always +allowed. `PDFRepairTransaction::setRequestedSavePolicy()` accepts a stricter +request and refuses a weaker one before `analyze()`, `apply()` or +`serializeCandidate()` do any work, and the refusal is remembered so a caller +cannot retry past it. `pdf::validateSaveRequest()` applies the same rule at the +save boundary and additionally refuses a candidate whose output resolves to +`PDFRepairTransactionOptions::sourcePath`. PdfTool reports the refusal as +diagnostic `save-policy.refused` with exit code 4. + +These guarantees are pinned by name, not by convention: +`everyRegisteredOperationDeclaresItsSavePolicy` (no registered operation may +rely on the undeclared default), +`transactionRejectsAWeakenedSavePolicyBeforeMutation`, +`saveRequestRefusesToWriteOverTheTrustedSource`, +`candidateSaveRefusesToOverwriteTheSourceOnDisk`, +`sourceBytesSurviveSuccessCancelAndFailure`, +`noNonIncrementalOperationCanBeAppendedToASignedSource`, and, for the +corrective CLI commands, `addBleedRefusesToWriteOverItsOwnInput` and +`rgbToCmykRefusesToWriteOverItsOwnInput`. + The editor content-save path preserves object numbers and does not run the storage-shrinking optimizer before the controller chooses its write mode. This is required for changed-object detection and signature coverage preservation. diff --git a/docs/INDEPENDENT_VALIDATION.md b/docs/INDEPENDENT_VALIDATION.md index cfeddee60..60d87419f 100644 --- a/docs/INDEPENDENT_VALIDATION.md +++ b/docs/INDEPENDENT_VALIDATION.md @@ -32,6 +32,15 @@ timeouts, invocation failures, and absent signatures are `incomplete`; a nonzero validator exit is `rejected`. `incomplete` never qualifies as PASS, and self-only Loop checks do not satisfy this gate. +The save-policy claim is produced by `UnitTestsIncrementalSave`: when +`LOOP_SAVE_POLICY_EVIDENCE_DIR` is set it writes +`incremental-with-signature.pdf` — a genuinely signed document that keeps the +original signed byte range across an incremental append — plus a JSON record of +the source and artifact digests. `reusable-linux.yml` runs `--claim structural +--claim signature` over that artifact and stores the evidence under +`docs/evidence/session-15-save-policy/`. A host without `qpdf` and `pdfsig` +records `reason_code: validator-not-installed` and stays `incomplete`. + The conversion fixture triad remains the source-level oracle in `loop-preflight/testdata/conversion/manifest.json`. Any real PDF added for a platform qualification run must record provenance, license, digest, expected From 55ec58fcf1e016b1d8b7eea68be816140caa1255 Mon Sep 17 00:00:00 2001 From: mberrys Date: Mon, 14 Sep 2026 01:27:52 -0700 Subject: [PATCH 22/26] chore(changelog): record the save-policy strength change (#239) --- changes/feat-0.3.0-239-save-policy-strength.md | 10 ++++++++++ 1 file changed, 10 insertions(+) create mode 100644 changes/feat-0.3.0-239-save-policy-strength.md diff --git a/changes/feat-0.3.0-239-save-policy-strength.md b/changes/feat-0.3.0-239-save-policy-strength.md new file mode 100644 index 000000000..69d91be4f --- /dev/null +++ b/changes/feat-0.3.0-239-save-policy-strength.md @@ -0,0 +1,10 @@ +Category: changed +Audience: developers and operators +Breaking-Change: yes +Summary: The save mode an operation declares is now a floor at the Core transaction and write +boundary. `PDFRepairTransaction::setRequestedSavePolicy()` accepts a stricter policy but refuses a +weaker one before analysis or mutation, and `pdf::validateSaveRequest()` refuses any candidate write +whose output resolves to the trusted input file. `redact`, `add-bleed` and `rgb-to-cmyk` therefore +exit 4 with `save-policy.refused` when the output path is the input path — those in-place invocations +previously succeeded with `--overwrite`, so write the candidate to a new path. Redaction also +declares a full rewrite, so it can never produce an incremental update. From dfbf73e3f108d32eaf5c7237aaf6f524f325ca52 Mon Sep 17 00:00:00 2001 From: mberrys Date: Mon, 14 Sep 2026 01:42:13 -0700 Subject: [PATCH 23/26] fix(docs): keep the recovery record inside the Loop identity contract (#239) --- docs/EDITOR_RECOVERY.md | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/docs/EDITOR_RECOVERY.md b/docs/EDITOR_RECOVERY.md index 95b85f771..38c90479f 100644 --- a/docs/EDITOR_RECOVERY.md +++ b/docs/EDITOR_RECOVERY.md @@ -2,7 +2,10 @@ **The recovery service described below does not exist in this tree.** It was deleted by `2a19e2c1` ("delete Widgets libraries and plugin pack for Session 05 -Issue 17"), which removed `LoupeLibGui/pdfrecoverymanager.{h,cpp}`. Everything +Issue 17"), which removed `pdfrecoverymanager.{h,cpp}` along with the GUI library +that hosted it; the branch name in that commit records the old library name. The +historical path is recoverable without depending on this page: +`git log --all --diff-filter=D --name-only -- '*pdfrecoverymanager*'`. Everything below is the contract to restore, not a description of shipped code. `UnitTests/tst_recoverytest.cpp` is kept as the specification of the From d9543dd129c2209f6c5337d6297bb81512560541 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Wed, 16 Sep 2026 00:41:33 +0000 Subject: [PATCH 24/26] chore(docs): refresh phase5-widgets inventory after dev merge Regenerate docs/generated/phase5-widgets-inventory.json so the Supply Chain Policy check passes after merging origin/dev (CMake target graph drift). Co-authored-by: michael berry --- docs/generated/phase5-widgets-inventory.json | 44 +++++++++++++++++++- 1 file changed, 43 insertions(+), 1 deletion(-) diff --git a/docs/generated/phase5-widgets-inventory.json b/docs/generated/phase5-widgets-inventory.json index ca3e98a7e..066622a6e 100644 --- a/docs/generated/phase5-widgets-inventory.json +++ b/docs/generated/phase5-widgets-inventory.json @@ -90,6 +90,7 @@ "UnitTests/CMakeLists.txt", "UnitTests/CMakeLists.txt", "UnitTests/CMakeLists.txt", + "UnitTests/CMakeLists.txt", "loop-preflight/tools/CMakeLists.txt" ], "shell_ledger": "docs/loop-shell.json", @@ -408,6 +409,7 @@ "UnitTestsOperatorAcceptance", "UnitTestsOverprint", "UnitTestsOverprintRender", + "UnitTestsPageBoxCorpus", "UnitTestsPageMasterExport", "UnitTestsPluginAbi", "UnitTestsPreflightEngine", @@ -2190,6 +2192,46 @@ "widgets_paths": [], "consumers": [] }, + { + "id": "UnitTestsPageBoxCorpus", + "kind": "executable", + "cmake": "UnitTests/CMakeLists.txt", + "profile_enabled": false, + "profile_condition": "qualification target excluded from the product-surface manifest", + "install_rule": false, + "installed_in_profile": false, + "build_only_in_profile": false, + "direct_links": [ + "LoopLibCore", + "Qt6::Core", + "Qt6::Gui", + "Qt6::Test" + ], + "direct_qt_modules": [ + "Core", + "Gui", + "Test" + ], + "transitive_targets": [ + "LoopLibCore" + ], + "transitive_qt_modules": [ + "Sql", + "Svg", + "Xml" + ], + "qt_modules": [ + "Core", + "Gui", + "Sql", + "Svg", + "Test", + "Xml" + ], + "widgets_linkage": "none", + "widgets_paths": [], + "consumers": [] + }, { "id": "UnitTestsPageMasterExport", "kind": "executable", @@ -3241,7 +3283,7 @@ } ], "counts": { - "targets": 74, + "targets": 75, "installed_in_profile": 4, "build_only_in_profile": 2, "widgets_surfaces": 4, From 2046296caefd5fe88ad823a4bd61c2f4b9971ab2 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Wed, 16 Sep 2026 03:56:22 +0000 Subject: [PATCH 25/26] fix(test): unwrap PdfTool envelope in bleed stress preflight UnitTestsBleedStress parsed stdout as a flat preflight report, but PdfTool now emits schema_version 1 envelopes with the report under data.report. Align runPreflight() with OperatorAcceptance and PreflightCorpus helpers so failBleedPreflight sees check_id bleed again. No save-policy change. Co-authored-by: michael berry --- UnitTests/CMakeLists.txt | 2 ++ UnitTests/tst_bleedstresstest.cpp | 20 +++++++++++++++++--- 2 files changed, 19 insertions(+), 3 deletions(-) diff --git a/UnitTests/CMakeLists.txt b/UnitTests/CMakeLists.txt index f92498210..f0057068e 100644 --- a/UnitTests/CMakeLists.txt +++ b/UnitTests/CMakeLists.txt @@ -856,6 +856,8 @@ if(NOT LOOP_BUILD_ONLY_CORE_LIBRARY) PDFTOOL_EXECUTABLE_PATH="$" LOOP_PREFLIGHT_SOURCE_DIR="${CMAKE_SOURCE_DIR}/loop-preflight" ) + target_include_directories(UnitTestsBleedStress PRIVATE + ${CMAKE_SOURCE_DIR}/UnitTests/support/preflight) set_target_properties(UnitTestsBleedStress PROPERTIES WIN32_EXECUTABLE OFF diff --git a/UnitTests/tst_bleedstresstest.cpp b/UnitTests/tst_bleedstresstest.cpp index 31b995deb..b6a0213a8 100644 --- a/UnitTests/tst_bleedstresstest.cpp +++ b/UnitTests/tst_bleedstresstest.cpp @@ -22,6 +22,7 @@ // Stress-tests bleed preflight + add-bleed repair on AI-artwork-like fixtures (MIC-316). +#include "pdftoolenvelopeutils.h" #include "processoutputcapture.h" #include @@ -146,7 +147,14 @@ bool BleedStressTest::runPreflight(const QString& pdfPath, QJsonObject* report, { const QString profilePath = QDir(sourceDir()).filePath(QString::fromLatin1(STRESS_PROFILE)); QByteArray stdOut; - if (!runPdfTool({ QStringLiteral("preflight"), pdfPath, QStringLiteral("--profile"), profilePath }, &stdOut, exitCode)) + if (!runPdfTool({ QStringLiteral("preflight"), + pdfPath, + QStringLiteral("--profile"), + profilePath, + QStringLiteral("--console-format"), + QStringLiteral("json") }, + &stdOut, + exitCode)) { return false; } @@ -158,12 +166,18 @@ bool BleedStressTest::runPreflight(const QString& pdfPath, QJsonObject* report, return false; } + const QJsonObject envelope = document.object(); + if (!pdfplugin::pdftool::isResultEnvelope(envelope, QStringLiteral("preflight"))) + { + return false; + } + if (report) { - *report = document.object(); + *report = pdfplugin::pdftool::reportFromEnvelope(envelope); } - return true; + return !report || !report->isEmpty(); } bool BleedStressTest::runAddBleed(const QString& inputPath, const QString& outputPath, const QString& mode, int* exitCode) const From 3729d73197ab3c9c1ae330936e935128f452b514 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Wed, 16 Sep 2026 03:56:27 +0000 Subject: [PATCH 26/26] docs(changes): note bleed stress envelope test fix in #582 fragment Co-authored-by: michael berry --- changes/feat-0.3.0-239-save-policy-strength.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/changes/feat-0.3.0-239-save-policy-strength.md b/changes/feat-0.3.0-239-save-policy-strength.md index 69d91be4f..0108d0b38 100644 --- a/changes/feat-0.3.0-239-save-policy-strength.md +++ b/changes/feat-0.3.0-239-save-policy-strength.md @@ -7,4 +7,5 @@ weaker one before analysis or mutation, and `pdf::validateSaveRequest()` refuses whose output resolves to the trusted input file. `redact`, `add-bleed` and `rgb-to-cmyk` therefore exit 4 with `save-policy.refused` when the output path is the input path — those in-place invocations previously succeeded with `--overwrite`, so write the candidate to a new path. Redaction also -declares a full rewrite, so it can never produce an incremental update. +declares a full rewrite, so it can never produce an incremental update. Bleed stress tests now +unwrap PdfTool's structured `data.report` envelope so bleed preflight findings are asserted again.