Skip to content

PDFCLOUD-5192 Add optional delete step to security samples - #104

Merged
datalogics-tsmith merged 27 commits into
pdfrest:mainfrom
datalogics-dliang:security-optional-delete
Sep 9, 2025
Merged

datalogics-tsmith merged 27 commits into
pdfrest:mainfrom
datalogics-dliang:security-optional-delete

Conversation

@datalogics-dliang

@datalogics-dliang datalogics-dliang commented Sep 2, 2025

Copy link
Copy Markdown
Contributor

PDFCLOUD-5192

  • Adds an optional delete step to security samples, e.g. samples for redaction, watermarking, encrypting, or restricting.
    • cURL is fully completed, and
    • 7 selected samples in the other languages (DotNet, Java, JavaScript, Python, PHP) have been selected to be edited.
  • There are a remaining 95 samples (complex, JSON Payload, and Multipart Payload) that have not yet been edited, but this work has been paused to wait for a testing suite that can test all samples.

* Complex flow examples: redact-preview-and-finalize
* /pdf-with-redacted-text-preview
* /pdf-with-redacted-text-applied
* Complex flow examples: protected-watermark
* /watermarked-pdf-image
* /watermarked-pdf
* Complex flow examples: decrypt-add-reencrypt
* /encrypted-pdf
* /encrypted-pdf-new-password
* /encrypted-pdf-change-password
* /decrypted-pdf - deletes the decrypted outputs
* /decrypted-pdf-with-permissions-password - deletes the decrypted outputs
* /restricted-pdf
* /restricted-pdf-change-password
* /restricted-pdf-add-password-with-open-password
* /restricted-pdf-add-password-and-restrictions
* /unrestricted-pdf
* /unrestricted-pdf-with-open-password
@datalogics-dliang datalogics-dliang changed the title Add optional delete step to security endpoints PDFCLOUD-5192 Add optional delete step to security endpoints Sep 2, 2025
@datalogics-dliang datalogics-dliang changed the title PDFCLOUD-5192 Add optional delete step to security endpoints PDFCLOUD-5192 Add optional delete step to security samples Sep 3, 2025
@datalogics-dliang
datalogics-dliang marked this pull request as ready for review September 4, 2025 14:02
Comment thread AGENTS.md Outdated
* pdf-with-redacted-text-preview
* pdf-with-redacted-text-applied
* watermarked-pdf
* encrypted-pdf
* decrypted-pdf
* restricted-pdf
* unrestricted-pdf
* Work done by Codex
* Adds optional delete step to the above samples in all languages (besides cURL) with relevant comment.
* For unrestricted or decrypted-pdf, delete the outputId. Otherwise, delete the inputId.
* For pdf-with-redacted-text-preview samples, delete both the inputId and outputId as they are both unredacted, but add a warning comment.
* Fixes PDFWithRedactedTextPreview.java with missing bracket
* Fixes remaining java samples with `mvn spotless:apply`
* Wrap whole delete block in parent if so that all variables are in scope
Comment thread cURL/Endpoint Examples/JSON Payload/decrypted-pdf-with-permissions-password.sh Outdated
-H "Accept: application/json" \
-H "Content-Type: multipart/form-data" \
-H "Api-Key: $API_KEY" \
-F "ids=$DECRYPTED_ID, $ADDED_IMAGE_ID" No newline at end of file

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't seem to delete the output from the last curl command where the file was reencrypted

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This just deletes the sensitive files that are decrypted and unrestricted (e.g. the added image file that can still be edited). This PR generally leaves the protected (e.g. encrypted, restricted, watermarked, redacted) files alone.

@datalogics-tsmith

Copy link
Copy Markdown
Contributor

I think the optional delete steps should be commented out. Otherwise they will run by default and users who didn't read the sample may be surprised to find that their files were immediately deleted

# The following code is an optional step to delete unwatermarked and
# watermarked files from pdfRest servers.

INPUT_PDF_ID=$(jq -r '.inputId' <<< $WATERMARKED_OUTPUT)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The inputId of watermark is an array. We only have one inputid in this case since it is a text watermark but we still need to respect the array

It would need to be
INPUT_PDF_ID=$(jq -r '.inputId[0]' <<< $WATERMARKED_OUTPUT)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed this for cURL but will need to fix this for the rest of the samples in other languages via codex

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed the non-cURL text watermark samples in 7535565

@datalogics-kam

datalogics-kam commented Sep 4, 2025

Copy link
Copy Markdown
Contributor

After I get the other conversions done for the C# samples in #110, I'll also have the AI apply the same changes as shown in the DotNET folder in this PR, and we can figure it out in merge.

Because the structure of the samples changed a little bit, and things like the API key are coming in from an env var.

* Add a toggle to turn the deletion step on or off with the PDFREST_DELETE_SENSITIVE_FILES variable.
* Use PDFREST_DELETE_SENSITIVE_FILES=true to turn on the deletion step.
* Update global README.md and cURL README.md to explain this logic
* Make the delete step optional by adding a local variable which the user can toggle on/off to do the delete step.
* The delete step is off by default.
* Match the rest of the samples and use DELETE_SENSITIVE_FILES.
@datalogics-dliang

Copy link
Copy Markdown
Contributor Author

I think the optional delete steps should be commented out. Otherwise they will run by default and users who didn't read the sample may be surprised to find that their files were immediately deleted

@datalogics-tsmith I added a local toggle in each sample so that the delete step can be turned on or off by setting the toggle to true or false. What do you think of this approach instead of commenting in and out large blocks of code? I kept the toggle as a local variable; a further improvement can be made by using environment variables in the future. I didn't go the environment variable route this time since I found we haven't standardized the way we load them (with some dotenv package or otherwise) across all the existing samples.

@datalogics-kam

datalogics-kam commented Sep 5, 2025

Copy link
Copy Markdown
Contributor

I like the local toggle idea, though I agree it would be also nice to have it read from an env var and default to false. That might be a lot to do for many languages right now, but I may put that in as part of the upgrade in the C# examples, which also got the environment variables and files-from-command line upgrades.

Also, I don't think the new Ruby, Perl, R, or VB.NET samples do anything with sensitive files.

@datalogics-kam

Copy link
Copy Markdown
Contributor

I've got a version with merged in C# samples at #110, and will fix that up again once this is merged.

Also, 5e5cbb1 implements the environment variable to delete sensitive files as well.

// is involved, an explicit delete call can be made to the API.
//
// The following code is an optional step to delete sensitive files
// (unredacted, unencrypted, unrestricted, or unwatermarked) from pdfRest servers.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I really think we should delete all of the files partly just to show that we can and partly since I think people who would be interested in it might still want their files to be deleted even when they are watermarked/encrypted/redacted etc

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll go ahead and work on a change to delete all files involved in the workflow.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As of 84c05ff all cURL security samples and the base security samples in all other languages should be deleting all files in the workflow now (inputs and outputs).

@datalogics-tsmith

Copy link
Copy Markdown
Contributor

@datalogics-tsmith I added a local toggle in each sample so that the delete step can be turned on or off by setting the toggle to true or false. What do you think of this approach instead of commenting in and out large blocks of code? I kept the toggle as a local variable; a further improvement can be made by using environment variables in the future. I didn't go the environment variable route this time since I found we haven't standardized the way we load them (with some dotenv package or otherwise) across all the existing samples.

I'm fine with this as a solution

* Updates comments, pretty-prints main request output.
* encrypted-pdf
* pdf-with-redacted-text-applied
* restricted-pdf
* watermarked-pdf
* These samples previously only deleted the input files.
* decrypted-pdf
* unrestricted-pdf
* pdf-with-redacted-text-preview - comment updated.
* Deletes original input file as well as the input image for adding an image
@datalogics-tsmith
datalogics-tsmith merged commit 91b5347 into pdfrest:main Sep 9, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants