Skip to content

Commit d206212

Browse files
authored
fix(documentai-toolbox): contain split_pdf output to output_path (#18063)
split_pdf builds each output filename from entity.type_, which is read straight from the parsed Document (loaded via from_gcs or from_document_path). Document AI entity types can contain "/" (the fixtures already carry "vat/tax_amount"), and a document whose type_ is set to something like ../../../../tmp/pwned makes os.path.join(output_path, output_filename) write the split PDF outside output_path. The entity type is now flattened before it goes into the filename, so the write stays inside output_path. - [ ] Make sure to open an issue as a [bug/issue](https://github.com/googleapis/google-cloud-python/issues) before writing your code! That way we can discuss the change, evaluate designs, and agree on the general idea - [x] Ensure the tests and linter pass - [x] Code coverage does not decrease (if any source code was changed) - [ ] Appropriate docs were updated (if necessary)
1 parent 102c940 commit d206212

2 files changed

Lines changed: 42 additions & 1 deletion

File tree

packages/google-cloud-documentai-toolbox/google/cloud/documentai_toolbox/wrappers/document.py

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -823,7 +823,17 @@ def split_pdf(self, pdf_path: str, output_path: str) -> List[str]:
823823
input_filename, input_extension = os.path.splitext(os.path.basename(pdf_path))
824824
with Pdf.open(pdf_path) as pdf:
825825
for entity in self.entities:
826-
subdoc_type = entity.type_ or "subdoc"
826+
# Entity types come from the parsed Document and may contain
827+
# path separators (e.g. "vat/tax_amount") or traversal
828+
# sequences. Flatten them so the split file stays inside
829+
# output_path. ":" is also flattened since it is not a valid
830+
# filename character on Windows.
831+
subdoc_type = (
832+
(entity.type_ or "subdoc")
833+
.replace("/", "_")
834+
.replace("\\", "_")
835+
.replace(":", "_")
836+
)
827837
page_range = (
828838
f"pg{entity.start_page + 1}"
829839
if entity.start_page == entity.end_page

packages/google-cloud-documentai-toolbox/tests/unit/test_document.py

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -790,6 +790,37 @@ def test_split_pdf_with_non_splitter(get_bytes_classifier_mock):
790790
get_bytes_classifier_mock.assert_called_once()
791791

792792

793+
@mock.patch("google.cloud.documentai_toolbox.wrappers.document.Pdf")
794+
def test_split_pdf_sanitizes_entity_type_path_traversal(
795+
mock_Pdf, get_bytes_splitter_mock
796+
):
797+
doc = document.Document.from_gcs(
798+
gcs_bucket_name="test-directory", gcs_prefix="documentai/output/123456789/0"
799+
)
800+
mock_output_file = mock.Mock()
801+
mock_Pdf.new.return_value = mock_output_file
802+
803+
# Entity types are read from the parsed Document and may carry path
804+
# separators or traversal sequences.
805+
doc.entities[0].type_ = "../../../../etc:passwd"
806+
807+
output_path = "splitter/output/"
808+
actual = doc.split_pdf(
809+
pdf_path="procurement_multi_document.pdf", output_path=output_path
810+
)
811+
812+
get_bytes_splitter_mock.assert_called_once()
813+
814+
resolved_output = os.path.realpath(output_path)
815+
for call in mock_output_file.save.call_args_list:
816+
written_path = os.path.realpath(call.args[0])
817+
assert written_path == resolved_output or written_path.startswith(
818+
resolved_output + os.sep
819+
)
820+
821+
assert actual[0] == "procurement_multi_document_pg1_.._.._.._.._etc_passwd.pdf"
822+
823+
793824
def test_convert_document_to_annotate_file_response():
794825
doc = document.Document.from_document_path(
795826
document_path="tests/unit/resources/0/toolbox_invoice_test-0.json"

0 commit comments

Comments
 (0)