Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### Fixed

- Redact Cloudflare API tokens in `DocumentConversionSettings` Debug output,
including pretty formatting, cloned settings and settings retained after build,
independently of enabled document backends.

- Bound DOCX table dimensions and padded cell work before local conversion,
including the default markkit backend. The deprecated Office renderer admits
Markdown bytes before escaping or padding into one capped buffer.
Expand Down
25 changes: 24 additions & 1 deletion crates/fetchkit/src/documents/settings.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
// Decisions: Preserve string-based settings across all feature sets; redact only
// the credential in Debug without changing public storage or build/clone semantics.
//! String-based document conversion settings shared by the CLI, MCP server and
//! Python bindings. Libraries can use [`DocumentConversion`] directly instead.

use std::fmt;

use super::{DocumentConversion, DocumentFormat};

/// Built-in converter names accepted by [`DocumentConversionSettings`].
Expand All @@ -12,7 +16,7 @@ pub const BUILTIN_CONVERTERS: &[&str] = &["pdf-inspector", "markkit", "office",
/// Plain settings resolved into a [`DocumentConversion`].
///
/// Credentials are passed explicitly; nothing is read from the environment.
#[derive(Debug, Default, Clone)]
#[derive(Default, Clone)]
pub struct DocumentConversionSettings {
/// Accepted formats (e.g. `["pdf", "docx"]`). `None` keeps every supported format.
pub formats: Option<Vec<String>>,
Expand All @@ -26,11 +30,30 @@ pub struct DocumentConversionSettings {
/// Cloudflare account id, required when `converters` names `cloudflare`.
pub cloudflare_account_id: Option<String>,
/// Cloudflare API token with Workers AI permission.
/// Redacted in settings `Debug`; direct field access still exposes the value.
pub cloudflare_api_token: Option<String>,
/// Honour proxy environment variables for remote converter uploads.
pub respect_proxy_env: bool,
}

impl fmt::Debug for DocumentConversionSettings {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
// THREAT[TM-LEAK-007]: Never format the original credential or a clone of it.
f.debug_struct("DocumentConversionSettings")
.field("formats", &self.formats)
.field("converters", &self.converters)
.field("allow_remote", &self.allow_remote)
.field("max_document_bytes", &self.max_document_bytes)
.field("cloudflare_account_id", &self.cloudflare_account_id)
.field(
"cloudflare_api_token",
&self.cloudflare_api_token.as_ref().map(|_| "<redacted>"),
)
.field("respect_proxy_env", &self.respect_proxy_env)
.finish()
}
}

impl DocumentConversionSettings {
/// Whether every field is at its default, i.e. nothing to configure.
pub fn is_default(&self) -> bool {
Expand Down
72 changes: 72 additions & 0 deletions crates/fetchkit/tests/document_settings.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
use fetchkit::DocumentConversionSettings;

fn assert_redacted(settings: &DocumentConversionSettings) {
for output in [format!("{settings:?}"), format!("{settings:#?}")] {
assert!(output.contains("<redacted>"));
assert!(!output.contains("dummy-credential"));
for visible in [
"formats",
"pdf",
"docx",
"converters",
"cloudflare",
"allow_remote: true",
"max_document_bytes",
"42",
"cloudflare_account_id",
"test-account",
"cloudflare_api_token",
"respect_proxy_env: true",
] {
assert!(
output.contains(visible),
"missing nonsecret setting: {visible}"
);
}
}
}

#[test]
fn settings_debug_redacts_before_and_after_build_and_clone() {
let settings = DocumentConversionSettings {
formats: Some(vec!["pdf".into(), "docx".into()]),
converters: Some(vec!["cloudflare".into()]),
allow_remote: true,
max_document_bytes: Some(42),
cloudflare_account_id: Some("test-account".into()),
cloudflare_api_token: Some("dummy-credential".into()),
respect_proxy_env: true,
};
assert_redacted(&settings);
let result = settings.build();
#[cfg(feature = "docs-cloudflare")]
assert!(result.is_ok());
#[cfg(not(feature = "docs-cloudflare"))]
assert_eq!(
result.unwrap_err(),
"converter `cloudflare` requires the docs-cloudflare feature"
);
assert_redacted(&settings);
let cloned = settings.clone();
assert_eq!(
cloned.cloudflare_api_token.as_deref(),
Some("dummy-credential")
);
assert_redacted(&cloned);
}

#[test]
fn settings_debug_distinguishes_absent_and_empty_credentials() {
let mut settings = DocumentConversionSettings::default();
for output in [format!("{settings:?}"), format!("{settings:#?}")] {
assert!(output.contains("cloudflare_api_token: None"));
assert!(!output.contains("<redacted>"));
assert!(output.contains("allow_remote: false"));
assert!(output.contains("respect_proxy_env: false"));
}
settings.cloudflare_api_token = Some(String::new());
for output in [format!("{settings:?}"), format!("{settings:#?}")] {
assert!(output.contains("<redacted>"));
assert!(!output.contains("cloudflare_api_token: None"));
}
}
13 changes: 13 additions & 0 deletions docs/hardening.md
Original file line number Diff line number Diff line change
Expand Up @@ -129,3 +129,16 @@ another service:
from `max_body_size`. Lower it, or restrict `formats`, for tighter budgets.
- Local parsing of hostile documents is CPU-bound; isolate the process and apply
outer CPU and memory limits for high-volume untrusted input (TM-DOS-007/008).

`DocumentConversionSettings` redacts `cloudflare_api_token` in compact and pretty
`Debug` output, including cloned settings and settings retained after `build()`.
This works even without the `docs-cloudflare` feature. A present token is shown as
`<redacted>`; an absent token remains `None`. Other settings stay visible.
Converter credentials also use the redacting `SecretString` wrapper.

Redaction applies to these representations. The public settings token field still
contains the original string, and building does not clear it. Avoid logging that
field directly or placing credentials in other settings. Fetchkit does not sanitize
arbitrary application logs or converter response text. Its CLI/MCP and Python
configuration paths do not log settings; embedding applications control their own
diagnostic output and access.
5 changes: 5 additions & 0 deletions knowledge/foundations/tool-contract.md
Original file line number Diff line number Diff line change
Expand Up @@ -360,6 +360,11 @@ By default, Fetchkit blocks connections to private/reserved IP ranges:
(Cloudflare credentials from `CLOUDFLARE_ACCOUNT_ID` / `CLOUDFLARE_API_TOKEN`),
Python `document_formats`, `converters`, `allow_remote_conversion`,
`max_document_bytes`, `cloudflare_account_id`, `cloudflare_api_token`.
- `DocumentConversionSettings::Debug` redacts a present Cloudflare token in compact
and pretty output, including clones and settings retained after `build()`, with
every backend feature configuration. Other settings remain visible; absent tokens
are `None`. The public token field remains `Option<String>`: explicit field
access/formatting is not redacted, and building does not clear it.
- For binary without a matching processor:
- Return metadata (`content_type`, `size`, `filename`, `last_modified`)
- Include `error: "Binary content is not supported. Only textual content (HTML, text, JSON, etc.) can be fetched."`,
Expand Down
4 changes: 4 additions & 0 deletions knowledge/log.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
# Fetchkit Knowledge Update Log

## 2026-10-13

* **Credential redaction**: `DocumentConversionSettings` now redacts present Cloudflare tokens in compact/pretty `Debug`, including clones and after build, independently of backend features. Public string storage remains unchanged; explicit field logging and downstream diagnostic access remain caller responsibilities (TM-LEAK-007).

## 2026-10-12

* **DOCX table budgets**: Both local backends now admit table dimensions and padded cell work before conversion; the deprecated renderer additionally admits exact Markdown bytes before allocation. Tiny offline fixtures cover limits, escaping and normal output under TM-DOS-008.
Expand Down
20 changes: 16 additions & 4 deletions knowledge/security/threat-model.md
Original file line number Diff line number Diff line change
Expand Up @@ -377,7 +377,7 @@ escaping and unchanged normal tables; no resource-exhaustion payload is needed.
| TM-LEAK-004 | User-Agent reveals software version | Info | Default UA `Everruns Fetchkit/1.0` reveals stack; configurable | **BY DESIGN** |
| TM-LEAK-005 | Timing side-channels (connect time reveals network proximity) | Low | 1-second timeout masks some timing; not fully mitigated | **ACCEPTED** |
| TM-LEAK-006 | Fetched documents disclosed to a remote converter | Medium | Remote converters off by default, explicit `allow_remote(true)`, forced off by `hardened()`, backend named in `extraction_method` | MITIGATED |
| TM-LEAK-007 | Remote converter credentials exposed | Medium | Tokens held in `SecretString`, redacted from `Debug`, never echoed in errors or responses | MITIGATED |
| TM-LEAK-007 | Remote converter credentials exposed | Medium | Converter secrets use `SecretString`; settings `Debug` redacts the token; authentication errors omit credentials | MITIGATED |

### Mitigation Details

Expand All @@ -391,9 +391,21 @@ operator configuration, never from fetched content, and the response's
`ConverterClient`, so a host can route them through its own egress boundary.

**TM-LEAK-007 — Converter credential exposure (MITIGATED):**
API tokens and configured header values are stored as `SecretString`, which redacts
itself from `Debug`; authentication failures are reported by status, without the
token or request headers.
Converter API tokens and configured header values are stored as `SecretString`,
which redacts itself from `Debug`; authentication failures are reported by status,
without the token or request headers. `DocumentConversionSettings` retains its public
`Option<String>` token field for configuration compatibility, but its custom `Debug`
formats present tokens (including empty strings) as `<redacted>` and absent tokens
as `None`. This applies to compact/pretty formatting, clones and the original
settings after `build()`, independently of backend features. Noncredential fields
remain visible. Tiny public-API formatting contracts cover these cases.

This is representation redaction, not a global log sanitizer or memory erasure:
callers can still read/format the token field directly, put secrets in other fields,
or expose service-controlled response text. `build(&self)` does not clear settings.
The in-tree CLI/MCP and Python configuration paths do not log settings; exposure
through configuration diagnostics requires an embedding application's logging sink
and diagnostic-reader access. No default production diagnostic leak is established.

**TM-LEAK-001 — Error message detail (MITIGATED):**
Fetchkit's error types (`FetchError`) use generic messages that don't include
Expand Down
Loading