Skip to content

Add the broad-format Unstructured bridge profile - #207

Open
salmonumbrella wants to merge 1 commit into
kenn-io:mainfrom
salmonumbrella:feat/unstructured-bridge-profile
Open

Add the broad-format Unstructured bridge profile#207
salmonumbrella wants to merge 1 commit into
kenn-io:mainfrom
salmonumbrella:feat/unstructured-bridge-profile

Conversation

@salmonumbrella

@salmonumbrella salmonumbrella commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

What changed

Docbank now has a hardened Unstructured bridge profile for the formats it can currently inspect and bound locally: PDF, PPTX, XLSX, ODS, CSV, EPUB, EML, XML, Markdown, plain text, JPEG, and PNG.

The profile pins deployment and runtime identity, exact-byte disclosure, finite source, evidence, Markdown, result, and artifact limits, and the one allowed structured artifact. The generic bridge enforces those ceilings before upload and across inline or separately fetched results. Provider acceptance alone never expands the advertised format set.

Why

Unstructured supports a broad surface, but “the provider accepts it” is not enough to make the input or output safe durable authority. Docbank should expose the useful intersection it can inspect and bound today, then expand deliberately as local authority improves.

Usage

For library or adapter deployment, build the canonical compatibility profile with unstructured.NewProfile(config). The operator’s bridge implementation must match that identity and run behind docbank-rendition/v1; unsupported upstream formats do not enter the profile automatically.

This PR adds the reference profile, not daemon, API, or CLI registration. That application wiring is intentionally deferred to S1–S3.

Part of #176 (R15). Stacks on #206.

@roborev-ci

roborev-ci Bot commented Aug 25, 2026

Copy link
Copy Markdown

roborev: Combined Review (a26e613)

Code review found two medium-severity issues involving crash recovery and schema/runtime consistency.

Medium

  • document/bridge/client.go:131 — The asynchronous bridge client does not checkpoint its accepted job ID or implement ResumableRenditionProvider. If the daemon exits after submission, the expired provider-phase job lacks a resume handle and is quarantined as operator_required, even though the bridge exposes durable job lookup. Implement RenderResumable, checkpoint the job ID before polling, resume through the existing job endpoint without resubmitting source bytes, and add a crash/checkpoint/resume test.

  • document/bridge/source-evidence-v1.schema.json:12,75 — The published schema conflicts with runtime validation: unit omissions cannot contain the required locator because it is absent while additionalProperties is false, and the schema permits time_range unit kinds rejected by Go validation. Add locator to omission properties, require it conditionally for unit omissions, align unit-kind enums with the Go contract, and add schema-conformance tests.


Reviewers: 2 done | Synthesis: codex, 12s | Total: 22m7s

@salmonumbrella
salmonumbrella force-pushed the feat/unstructured-bridge-profile branch from a26e613 to 7f01d5e Compare August 25, 2026 06:48
@roborev-ci

roborev-ci Bot commented Aug 25, 2026

Copy link
Copy Markdown

roborev: Combined Review (7f01d5e)

Review found five medium-severity issues involving retry hot-spins, duplicate provider submissions, authorization expiry, and unbounded XML parsing.

Medium

  • Retry queue can hot-spin on ineligible targetscmd/docbank/backfill_retry.go:38
    Retry entries are removed only after success. If a quarantined target becomes ineligible, it is never revisited or removed; after its deadline, waitDelay remains zero, causing daemon backfills to hot-spin and preventing checksum completion. Reconcile entries after each cursor sweep, remove ineligible targets, and enforce a positive delay for empty batches.

  • Docling poll exhaustion can duplicate processing and chargesdocument/docling/client.go:217
    A known pending task is classified as retryable capacity, but Docling has no resume support. The worker clears its snapshot and resubmits while the original task may still complete. Return RenditionErrorAmbiguousSubmission, or durably checkpoint and resume using the task ID.

  • Bridge poll exhaustion can resubmit a live jobdocument/bridge/client.go:162, document/bridge/client.go:185
    Cancellation is best-effort: errors are discarded, and HTTP 200 responses are accepted without confirming cancellation. Return a retryable result only after cancellation or absence is positively confirmed; otherwise return RenditionErrorAmbiguousSubmission. Validate cancellation responses or add durable job resumption.

  • Bridge operations can outlive sealed authorizationdocument/bridge/client.go:145
    Authorization is checked only when Render starts, while uploads, polling, artifact retrieval, and cancellation may continue beyond ExpiresAt. Bound the operation context to the earliest authorization expiry, caller deadline, or total timeout, and revalidate authorization before secret resolution and every outbound request.

  • Nested XML can cause disproportionate memory consumptioninternal/processing/source_metadata.go:337
    Character data is copied into every open ancestor builder, making memory usage proportional to nesting depth multiplied by text size. Deeply nested malformed OOXML or XMP can exhaust daemon memory. Accumulate only recognized fields, enforce per-field and aggregate limits, and terminate parsing with a bounded warning when exceeded.


Reviewers: 2 done | Synthesis: codex, 15s | Total: 38m47s

@salmonumbrella salmonumbrella changed the title Add Unstructured bridge profile Add the broad-format Unstructured bridge profile Aug 25, 2026
@salmonumbrella
salmonumbrella force-pushed the feat/unstructured-bridge-profile branch from 7f01d5e to aa30667 Compare August 25, 2026 10:38
@roborev-ci

roborev-ci Bot commented Aug 25, 2026

Copy link
Copy Markdown

roborev: Combined Review (aa30667)

Medium-severity issues remain in authorization expiry handling, API schema accuracy, XML parsing bounds, and upload memory use.

Medium

  • document/bridge/client.go:145 — Bridge execution is bounded only by totalTimeout and ignores authorization.ExpiresAt. A slow multipart upload may continue disclosing source bytes after authorization expires, with later results rejected as outside the authorized interval. Parse ExpiresAt, use the earlier of expiry and totalTimeout as the operation deadline, and classify expiry consistently before submission, polling, and artifact retrieval.

  • document/bridge/openapi.yaml:175 — The OpenAPI contract defines receipt as an unconstrained object, while the client requires and validates every RenditionReceipt field. A schema-conforming bridge can therefore return a response that Docbank rejects. Define closed RenditionReceipt and RenditionUsage schemas with required fields and bounds, then reference them from CompletedResult.

  • internal/processing/source_metadata.go:337 — Every XML character chunk is copied into every open ancestor builder without limits on nesting depth or accumulated text. Deeply nested XMP or Office property documents can amplify bounded input into excessive memory use and terminate the daemon. Bound XML depth and per-value accumulation, and retain text only for relevant elements.

  • document/marker/client.go:232, document/marker/client.go:494, document/reducto/client.go:307 — Accepted uploads are fully buffered and then copied into multipart buffers; Reducto also clones its buffer. Near configured limits, one document can consume multiple gigabytes of heap and cause an OOM. Stream verified content into multipart requests, use disk-backed spooling, or lower accepted limits to reflect the in-memory implementation.


Reviewers: 2 done | Synthesis: codex, 12s | Total: 23m37s

@salmonumbrella
salmonumbrella force-pushed the feat/unstructured-bridge-profile branch from aa30667 to 4283826 Compare August 25, 2026 11:12
@roborev-ci

roborev-ci Bot commented Aug 25, 2026

Copy link
Copy Markdown

roborev: Combined Review (4283826)

High-severity resource-exhaustion risk and three medium-severity reliability/policy gaps need attention before merge.

High

  • internal/processing/source_metadata.go:337 — Unbounded XML allocation amplification

    Every XML text token is copied into every open ancestor’s builder without limits on nesting depth or aggregate memory. Deeply nested XMP or OOXML can amplify a bounded input into gigabytes of allocations during automatic backfill, potentially crashing the daemon repeatedly on restart.

    Fix: Cap XML depth and accumulated text, collect only relevant elements, and emit a bounded extraction warning when limits are exceeded.

Medium

  • internal/processing/source_metadata.go:174 — Metadata collection can exceed serialization limits

    List entries are not checked against MaxSourceMetadataValueBytes, and the collector does not enforce the 8 MiB aggregate encoded-record limit. Valid embedded metadata can produce a record that MarshalSourceMetadataV1 rejects, leaving the document permanently unprocessed and repeatedly retried.

    Fix: Enforce per-entry, label, and aggregate encoded-size budgets during collection; omit excess metadata with bounded warnings and verify that backfill still publishes a generation.

  • cmd/docbank/daemon.go:184 — Stale retry entries can cause busy-spinning

    Retry entries are removed only after successful processing. If a failed blob is deleted or completed through another path, an empty root scan leaves its ready retry entry behind, causing waitDelay to return zero indefinitely and both backfill loops to spin.

    Fix: Reconcile retries after a complete root-to-end scan, discard entries for targets that no longer exist, and retain a nonzero idle delay when no target is available.

  • document/media/inspect.go:480 — EPUB external-reference checks miss inline CSS

    Inspection checks URL references in XML attributes and standalone .css files but ignores CSS in style attributes and <style> elements. An EPUB containing an external url(...) or @import may therefore receive an eligible capability record despite the external-reference policy.

    Fix: Inspect all CSS-bearing EPUB content, including inline styles, and reject external URLs with CapabilityReasonExternalReference.


Reviewers: 2 done | Synthesis: codex, 15s | Total: 23m34s

@salmonumbrella
salmonumbrella force-pushed the feat/unstructured-bridge-profile branch 7 times, most recently from e35d144 to 0c59dc0 Compare August 25, 2026 12:51
@salmonumbrella
salmonumbrella force-pushed the feat/unstructured-bridge-profile branch from 0c59dc0 to a5f2e3a Compare August 25, 2026 13:10
@roborev-ci

roborev-ci Bot commented Aug 25, 2026

Copy link
Copy Markdown

roborev: Combined Review (a5f2e3a)

Medium-severity issues remain; the security review found no concrete vulnerabilities.

Medium

  • internal/processing/source_metadata.go:269 — PDF XMP is detected by scanning raw bytes for <x:xmpmeta>, allowing unreferenced objects or page-content streams to be treated as authoritative metadata. Resolve XMP through the final cross-reference catalog’s /Metadata entry and process only that verified stream.

  • internal/processing/source_metadata.go:336 — XML text is copied into every open ancestor without depth or allocation limits, enabling quadratic memory growth from deeply nested XMP or OOXML. Bound XML depth and total collected text, and aggregate descendants only for recognized metadata elements.

  • document/bridge/client.go:202, document/bridge/client.go:422, document/bridge/client.go:544 — Some bridge failures are not returned as top-level *RenditionProviderError, so timeouts, truncated artifacts, and malformed provider Markdown become unclassified and quarantine jobs as operator_required. Return top-level classified errors consistently, including the malformed Markdown error directly.

  • internal/processing/source_metadata.go:174 — String-list entries bypass MaxSourceMetadataValueBytes, potentially producing metadata that cannot be marshaled and is retried indefinitely. Enforce the per-entry limit before appending, warn when omitting oversized values, and verify extracted records marshal successfully.

  • cmd/docbank/daemon.go:184, cmd/docbank/daemon.go:247, cmd/docbank/backfill_retry.go:41 — Retry entries persist when failed targets are deleted or no longer missing, causing waitDelay to return zero and trigger a tight database polling loop. Reconcile entries after full scans or enforce a positive minimum delay for stale entries.


Reviewers: 2 done | Synthesis: codex, 11s | Total: 26m13s

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant