Skip to content

Add the verified local plain-text rendition provider - #196

Open
salmonumbrella wants to merge 1 commit into
kenn-io:mainfrom
salmonumbrella:feat/plaintext-rendition-provider
Open

Add the verified local plain-text rendition provider#196
salmonumbrella wants to merge 1 commit into
kenn-io:mainfrom
salmonumbrella:feat/plaintext-rendition-provider

Conversation

@salmonumbrella

@salmonumbrella salmonumbrella commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

What changed

Docbank now has a private in-process rendition provider for authorized UTF-8 files. It rereads the supplied bytes, verifies their exact length and SHA-256, and emits one stable provider-neutral evidence unit under an explicit 16 MiB profile cap.

Empty input, invalid UTF-8, NUL bytes, cancellation, oversize input, and byte substitution all fail closed. The provider has no network client, credential binding, hosted consent, retained artifact, or filesystem reopen path.

Why

Plain text should not need a hosted provider or an ambient filesystem path. It still needs the same exact-byte and evidence authority as every other rendition path so local processing cannot become a privileged shortcut.

Usage

For library use, construct the provider with plaintext.New(profile) and call it through document.RenditionProvider with an authorized UTF-8 upload.

This PR does not add daemon, API, or CLI selection. The S1–S3 application-surface work will build the process-local runtime registration around this provider.

Part of #176 (R6). Stacks on #195.

@roborev-ci

roborev-ci Bot commented Aug 24, 2026

Copy link
Copy Markdown

roborev: Combined Review (bc7df15)

Verdict: Changes requested — one high-severity credential-exposure risk and three medium-severity processing issues remain.

High

  • document/bridge/client.go:93bridge.New preserves the supplied HTTP transport, so default clients can honor ProxyFromEnvironment. For permitted http:// origins, an ambient proxy may receive multipart document contents and the bearer credential in plaintext. Use a destination-bound direct transport with proxies disabled and explicit CIDR policy; reject credentialed plaintext profiles unless a trusted-proxy policy is explicitly configured.

Medium

  • cmd/docbank/daemon.go:161 and cmd/docbank/daemon.go:222 — Backfill retry entries are not removed when their targets disappear from missing-work queries. Once an orphaned retry becomes ready, scans can enter a tight SQLite polling loop. Prune absent retries after complete scans or ensure empty scans wait.

  • document/bridge/client.go:174 — Bridge cancellation returns raw context.Canceled or context.DeadlineExceeded, which document.RenderRendition converts into an unclassified provider error. Wrap local context termination as RenditionProviderError with RenditionErrorCanceled, and add integration coverage through RenderRendition.

  • internal/store/processing_migration.go:196 — Migration continually requeues current-version extraction rows already marked failed, repeatedly processing deterministic terminal failures and potentially looping without delay. Treat current-version failures as terminal until the extractor version changes, or persist retry classification and back off only transient failures.


Reviewers: 2 done | Synthesis: codex, 10s | Total: 18m42s

@salmonumbrella salmonumbrella changed the title feat: add verified plain-text rendition provider Add the verified local plain-text rendition provider Aug 25, 2026
@salmonumbrella
salmonumbrella force-pushed the feat/plaintext-rendition-provider branch from bc7df15 to e358f09 Compare August 25, 2026 07:38
@roborev-ci

roborev-ci Bot commented Aug 25, 2026

Copy link
Copy Markdown

roborev: Combined Review (e358f09)

Medium-severity issues remain in bridge artifact validation, XML metadata processing, list-size enforcement, and external schema detection.

Medium

  • document/bridge/client.go:390 — Completed bridge results fetch every artifact before enforcing MaxArtifacts, authorized roles, or MaxTotalResultBytes. A malicious provider could trigger excessive downloads and memory use before validation rejects the result. Preflight the artifact manifest—validating count, roles, media types, hashes, and declared aggregate size—then track actual bytes while resolving artifacts.

  • internal/processing/source_metadata.go:331 — Each XML character chunk is copied into every open ancestor’s builder, causing quadratic allocation for deeply nested metadata XML. Bound XML nesting and cumulative captured text, and retain text only for elements that can produce supported fields.

  • internal/processing/source_metadata.go:176 — String-list entries are not checked against MaxSourceMetadataValueBytes. Oversized entries can pass extraction but fail codec validation during marshaling, causing repeated metadata backfill failures. Apply the per-value limit to every list entry, warn, and omit oversized values.

  • document/media/inspect.go:549 — External-reference detection ignores xsi:noNamespaceSchemaLocation and treats schemaLocation as a single URL instead of namespace/location pairs. XML may therefore retain external schema URLs despite the inspection contract. Recognize noNamespaceSchemaLocation, parse schemaLocation as whitespace-separated pairs, and validate each location fail-closed.


Reviewers: 2 done | Synthesis: codex, 10s | Total: 18m55s

@salmonumbrella
salmonumbrella force-pushed the feat/plaintext-rendition-provider branch from e358f09 to c76fab5 Compare August 25, 2026 10:38
@roborev-ci

roborev-ci Bot commented Aug 25, 2026

Copy link
Copy Markdown

roborev: Combined Review (c76fab5)

Three medium-severity issues require fixes; no high or critical findings were reported.

Medium

  • internal/store/extraction.go:480 — Lexical-head existence is incorrectly used as the migration-state flag. In a fresh vault, successful extractions remain in the legacy cache until restart; once a head exists, deterministic failures may be repeatedly requeued, causing continuous processing. Track migration initialization explicitly, allow the first successful extraction to create a head, and remove terminal failures from the queue unless an extractor-version change makes them retryable.

  • cmd/docbank/backfill_retry.go:38 — Retry entries are removed only when their target succeeds. If a target disappears before retry, a stale ready entry can make waitDelay return zero indefinitely, causing a tight database/CPU loop. Reconcile retry entries against every completed target scan, expire entries not observed, and retain a nonzero idle delay when no work was processed.

  • internal/processing/source_metadata.go:176 — List metadata values are validated for UTF-8 and count, but not for the codec’s per-value size limit. An oversized Received, author, or keyword entry can make canonicalization fail and prevent otherwise valid metadata and warnings from being published. Enforce MaxSourceMetadataValueBytes for every list entry, omit oversized values, and record a value_too_large warning before canonicalization.


Reviewers: 2 done | Synthesis: codex, 10s | Total: 26m33s

@salmonumbrella
salmonumbrella force-pushed the feat/plaintext-rendition-provider branch from c76fab5 to 16c31f6 Compare August 25, 2026 11:12
@roborev-ci

roborev-ci Bot commented Aug 25, 2026

Copy link
Copy Markdown

roborev: Combined Review (16c31f6)

Review verdict: Six medium-severity issues require attention; no Critical or High findings were reported.

Medium

  • Exact publication retries can failinternal/store/processing_catalog.go:313
    Stored artifacts are reloaded with derived MD5 values from blob_checksums, while the original immutable build lacks them. After staging succeeds and a later step fails, retrying the same build can fail reflect.DeepEqual. Exclude derived MD5 data from immutable-build equality or canonicalize both records to persisted fields. Add a retry test after catalog staging.

  • Cancellation errors lose their classificationdocument/bridge/client.go:176
    Several paths return raw context errors, which document.RenderRendition replaces with an “unclassified rendition provider error,” breaking canceled classification and errors.Is compatibility. Wrap cancellation and deadline errors in RenditionProviderError with RenditionErrorCanceled, preserving the cause.

  • Historical versions can expose current or newer metadatainternal/store/source_metadata.go:194
    Historical metadata combines the requested blob’s data with the node’s current name/path and latest provenance selected only by node_id. Tie attachment and provenance facts to the exact content version or blob, exposing the current node location separately if necessary.

  • Active HTML element bodies can survive sanitizationdocument/normalize.go:614
    Each raw HTML span uses a fresh writer, discarding suppression state from opening elements such as <script> and <iframe>. Their tags are removed, but their bodies can remain in Markdown and searchable evidence. Preserve suppression across spans or process complete active elements together, with inline and block coverage.

  • Bridge responses can allocate far beyond the authorized result sizedocument/bridge/client.go:327
    Responses are buffered and unmarshaled up to the global 512 MiB limit before enforcing MaxTotalResultBytes. Derive the envelope limit from the authorization with JSON/base64 overhead accounted for, or stream-decode while enforcing artifact limits.

  • Deeply nested links can exhaust memory or stack spacedocument/normalize.go:935
    Link nesting is unbounded, while flattening and serialization recursively traverse children. Crafted provider HTML can cause resource exhaustion. Enforce a maximum link depth, flatten deeper links, and test deeply nested input.


Reviewers: 2 done | Synthesis: codex, 14s | Total: 18m2s

@salmonumbrella
salmonumbrella force-pushed the feat/plaintext-rendition-provider branch from 16c31f6 to 825cdf2 Compare August 25, 2026 11:39
@roborev-ci

roborev-ci Bot commented Aug 25, 2026

Copy link
Copy Markdown

roborev: Combined Review (825cdf2)

Medium findings identified: publication retries can fail after staging, and oversized string-list metadata can permanently block publication.

Medium

  • internal/store/processing_catalog.go:313 — Publication cannot be retried after a post-staging failure.
    RecordRenditionBlob stores the sanitized Markdown MD5, while the staged artifact retains an empty MD5. On retry, loadRenditionBuild joins the stored MD5 into the record, causing reflect.DeepEqual to reject otherwise identical immutable metadata.
    Fix: Exclude auxiliary MD5 data from immutable build equality, or consistently populate the staged record from verified receipts. Add a retry assertion to the post-catalog-stage failure test.

  • internal/processing/source_metadata.go:176 — Oversized string-list metadata can permanently prevent publication.
    Validation checks UTF-8 and entry count but does not enforce the 64 KiB per-value or 8 MiB encoded limits used by MarshalSourceMetadataV1. An oversized PDF author, email header, or similar list value can therefore make canonicalization fail rather than recording an omission warning.
    Fix: Enforce per-entry and aggregate encoded-size limits while collecting lists, deterministically omit oversized data with a warning, and test an oversized embedded list value through extraction and publication.


Reviewers: 2 done | Synthesis: codex, 11s | Total: 16m8s

@salmonumbrella
salmonumbrella force-pushed the feat/plaintext-rendition-provider branch 4 times, most recently from 633413b to 3f897c7 Compare August 25, 2026 12:37
@salmonumbrella
salmonumbrella force-pushed the feat/plaintext-rendition-provider branch from 3f897c7 to 0887b27 Compare August 25, 2026 12:51
@roborev-ci

roborev-ci Bot commented Aug 25, 2026

Copy link
Copy Markdown

roborev: Combined Review (0887b27)

Rendition and metadata processing have one crash-level defect, one credential-exposure risk, and five medium-severity reliability/resource issues.

High

  • internal/processing/source_metadata.go:330 — The growing stack copies non-zero strings.Builder values. Indented nested XML can trigger slice reallocation followed by a panic, permanently failing the source-metadata worker. Store pointers to elements/builders and add tests using indented nested XMP and OOXML.

  • document/bridge/client.go:546validateOrigin permits non-loopback http:// origins under operator_network; submission can then expose complete source documents and reusable bearer credentials to network observers or proxies. Require HTTPS for non-loopback origins. Restrict any plaintext local-adapter support to destination-pinned loopback transports, and never send credentials over other plaintext connections.

Medium

  • internal/processing/source_metadata.go:331 — Character data is copied into every open ancestor without depth or cumulative-text bounds, allowing deeply nested XMP to cause quadratic CPU and memory consumption. Enforce XML-depth and extraction budgets and accumulate text only for relevant elements.

  • internal/processing/source_metadata.go:176 — String-list collection does not enforce the 64 KiB per-entry or 8 MiB aggregate encoding limits. Oversized embedded values can make canonicalization fail and leave documents in permanent backfill retries. Apply both byte limits during collection and omit oversized values with a warning.

  • document/internal/formatdetect/detect.go:189 — Page counting requires a literal trailer dictionary, rejecting valid PDF 1.5+ files whose /Root is in an /Type /XRef stream dictionary. Resolve the catalog root from the cross-reference stream referenced by startxref and add xref-stream-only page-tree coverage.

  • cmd/docbank/backfill_retry.go:38, cmd/docbank/daemon.go:161 — A due retry makes waitDelay return zero indefinitely when its target is deleted or no longer missing, causing both backfill loops to poll continuously and preventing checksum completion. Reconcile stale retry entries after complete scans and retain a nonzero idle delay for empty scans.

  • document/bridge/client.go:168 — Provider retry delays are truncated to the per-request timeout, while retryable polling errors discard RenditionProviderError.RetryAfter. This can hammer rate-limited providers and exhaust attempts prematurely. Preserve delays from envelopes and classified errors, bounded by the remaining overall operation deadline.


Reviewers: 2 done | Synthesis: codex, 16s | Total: 23m51s

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