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
28 changes: 27 additions & 1 deletion apps/desktop/src/components/settings/ProviderHeadersEditor.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { useEffect, useRef, useState, type ChangeEvent } from "react";
import { useTranslation } from "react-i18next";
import { APP_VERSION } from "@pi-desktop/shared";
import { APP_VERSION, inspectHeaderValue } from "@pi-desktop/shared";
import {
KeyValueRows,
pairsToRecord,
Expand Down Expand Up @@ -68,6 +68,22 @@ export function ProviderHeadersEditor({

useEffect(() => () => window.clearTimeout(copyTimer.current), []);

// A value the host folds on save, and one it will refuse, are both said next
// to the rows — a fullwidth character is an IME slip, not a mystery. Only
// rows that will actually be persisted count: an unnamed, empty or already
// refused row has nothing to fold, and saying otherwise would read as if the
// whole row were fine.
const headerRows = pairs.map((pair) => {
const header = inspectHeaderValue(pair.value);
const storable =
pair.key.trim() !== "" && header.value !== "" && header.fault === null;
return { header, storable };
});
const foldedHeaderValue = headerRows.some(
(row) => row.storable && row.header.folded,
);
const faultyHeaderValue = headerRows.some((row) => row.header.fault !== null);

const addPreset = (key: string) => {
const preset = HEADER_PRESETS.find((item) => item.key === key);
if (!preset) return;
Expand Down Expand Up @@ -157,6 +173,16 @@ export function ProviderHeadersEditor({
{t("settings.headersImportError")}
</div>
) : null}
{foldedHeaderValue ? (
<div className="provider-setup-header-note" role="status">
{t("settings.headersFullwidthFolded")}
</div>
) : null}
{faultyHeaderValue ? (
<div className="provider-setup-header-error" role="alert">
{t("settings.headersValueNotLatin1")}
</div>
) : null}
<div className="provider-setup-header-list">
<KeyValueRows
pairs={pairs}
Expand Down
8 changes: 8 additions & 0 deletions apps/desktop/src/styles/model-config.css
Original file line number Diff line number Diff line change
Expand Up @@ -482,6 +482,14 @@
line-height: var(--leading-relaxed);
}

/* Quieter than the import error: folding is informational, not a refusal. */
.provider-setup-header-note {
margin: 0;
color: var(--ds-text-muted);
font-size: var(--text-2xs);
line-height: var(--leading-relaxed);
}

/* Pick on the left, review on the right; both stretch to the same height. */
.provider-setup-panes {
flex: 1;
Expand Down
19 changes: 19 additions & 0 deletions apps/desktop/test/provider-form-layout.test.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -356,3 +356,22 @@ test("the vendor account dialog hosts the same panes in the same shell", () => {
assert.doesNotMatch(styles, /\.vendor-account-chosen/);
assert.doesNotMatch(styles, /\.vendor-account-custom-model/);
});

test("Advanced says a fullwidth value folds and a non-Latin-1 value is refused", () => {
// The rule itself lives in @pi-desktop/shared (unit-tested there) and is
// mirrored in host-core; this pins that the editor asks it and renders both
assert.match(headerEditorSource, /import \{ APP_VERSION, inspectHeaderValue \}/);
assert.match(headerEditorSource, /inspectHeaderValue\(pair\.value\)/);
// Only a row that will be persisted may claim it folds: the hint has to
// agree with what the host and the runtime do with the row.
assert.match(headerEditorSource, /pair\.key\.trim\(\) !== ""/);
assert.match(headerEditorSource, /header\.value !== ""/);
assert.match(headerEditorSource, /header\.fault === null/);
assert.match(headerEditorSource, /row\.storable && row\.header\.folded/);
assert.match(headerEditorSource, /provider-setup-header-note/);
assert.match(headerEditorSource, /role="status"/);
assert.match(headerEditorSource, /role="alert"/);
assert.match(headerEditorSource, /settings\.headersFullwidthFolded/);
assert.match(headerEditorSource, /settings\.headersValueNotLatin1/);
assert.match(block(".provider-setup-header-note"), /color: var\(--ds-text-muted\)/);
});
6 changes: 6 additions & 0 deletions crates/host-core/src/config_sync/apply.rs
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,12 @@ fn apply_entity(
object.insert("secretValue".into(), Value::String(secret.clone()));
}
}
// A bundle from a peer on an older build, or a backup taken before the
// header rule was tightened, can carry a value this build refuses. That
// is dropped here — the rule `config_headers` already applies when
// reading a store — so one stale row cannot fail the whole revision,
// which is what the strict write the editor goes through would do.
providers::retain_storable_headers(&mut payload);
let exists = st
.db
.conn()
Expand Down
10 changes: 5 additions & 5 deletions crates/host-core/src/providers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,13 +42,13 @@ pub(crate) use credentials::{
config_reasoning_override, config_value, config_with_headers, config_with_limit,
config_with_oauth_account_label, config_with_reasoning_override,
config_with_thinking_levels_override, ensure_config_object, limit_temperature_value,
limit_u32_value, limits_object, merge_provider_config_overrides, upsert_secret_meta,
LimitOverrides,
limit_u32_value, limits_object, merge_provider_config_overrides, retain_storable_headers,
upsert_secret_meta, LimitOverrides,
};
pub(crate) use validation::{
config_limit_f64, config_limit_u32, normalize_headers_input, normalize_one_header,
normalize_thinking_levels, valid_header_key, validate_model_aliases, MAX_HEADERS,
MAX_MODEL_ALIAS_CHARS,
config_limit_f64, config_limit_u32, fold_fullwidth, header_value_fault,
normalize_headers_input, normalize_one_header, normalize_thinking_levels, storable_headers,
valid_header_key, validate_model_aliases, MAX_HEADERS, MAX_MODEL_ALIAS_CHARS,
};

#[cfg(test)]
Expand Down
55 changes: 40 additions & 15 deletions crates/host-core/src/providers/credentials.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,27 +32,47 @@ pub(crate) fn config_headers(raw: &str) -> Option<BTreeMap<String, String>> {
collected.insert("User-Agent".into(), user_agent.to_string());
}
}
let mut by_lower: BTreeMap<String, (String, String)> = BTreeMap::new();
for (key, value) in collected {
if let Ok(Some((normalized_key, normalized_value))) = normalize_one_header(&key, &value) {
by_lower.insert(
normalized_key.to_ascii_lowercase(),
(normalized_key, normalized_value),
);
}
}
let out: BTreeMap<_, _> = by_lower
.into_iter()
.take(MAX_HEADERS)
.map(|(_, pair)| pair)
.collect();
// A stored map is read, not written: rows this build refuses (a value with
// a character no header can carry, a reserved name) are dropped rather than
// reported, so an older store cannot fail a turn.
let out = storable_headers(&collected);
if out.is_empty() {
None
} else {
Some(out)
}
}

/// Fold and drop the `headers` object of a provider payload before it is
/// deserialized into a write input. A bundle from a peer on an older build, or
/// a backup taken before the header rule was tightened, can carry a value this
/// build refuses; failing the whole sync revision over one stale row is worse
/// than dropping it, and the row is already invisible on read (`config_headers`
/// drops the same cases).
pub(crate) fn retain_storable_headers(payload: &mut serde_json::Value) {
let Some(object) = payload.as_object_mut() else {
return;
};
let Some(headers) = object.get("headers").and_then(serde_json::Value::as_object) else {
return;
};
let raw: BTreeMap<String, String> = headers
.iter()
.filter_map(|(key, value)| Some((key.clone(), value.as_str()?.to_string())))
.collect();
if raw.is_empty() {
return;
}
let storable = storable_headers(&raw);
if storable.is_empty() {
object.remove("headers");
} else {
object.insert(
"headers".into(),
serde_json::to_value(storable).unwrap_or_default(),
);
}
}
/// Set or clear optional headers. An empty map clears them and drops leftover `userAgent`.
pub(crate) fn config_with_headers(raw: &str, headers: &BTreeMap<String, String>) -> Result<String> {
let mut config = ensure_config_object(raw)?;
Expand Down Expand Up @@ -292,6 +312,11 @@ pub(crate) fn upsert_secret_meta(
Ok(())
}

/// Read a provider's API key, folding fullwidth IME input the same way header
/// values are folded. The key is signed into `Authorization` (or `x-api-key`)
/// headers, where a fullwidth character can never be valid, so a key stored
/// before this rule existed still authenticates after an upgrade. Applied on
/// read as well as on write because only this accessor feeds outbound requests.
pub fn get_secret_for_provider(
db: &Database,
secrets: &SecretStore,
Expand All @@ -304,7 +329,7 @@ pub fn get_secret_for_provider(
.optional()?
.flatten();
if let Some(sref) = secret_ref {
secrets.get(&sref)
Ok(secrets.get(&sref)?.map(|value| fold_fullwidth(&value)))
} else {
Ok(None)
}
Expand Down
12 changes: 6 additions & 6 deletions crates/host-core/src/providers/repository.rs
Original file line number Diff line number Diff line change
Expand Up @@ -424,7 +424,9 @@ pub(crate) fn delete_provider_row(db: &Database, secrets: &SecretStore, id: &str
/// `api_key` reference and the row's `secret_ref` change; no field the plugin's
/// manifest owns is touched, so the next load still refreshes the declaration.
///
/// An empty value deletes the stored key and clears `secret_ref`.
/// An empty value deletes the stored key and clears `secret_ref`. A fullwidth
/// value is folded to half-width, the same rule header values follow, because
/// the key is signed into an HTTP header.
pub fn set_provider_secret(
db: &Database,
secrets: &SecretStore,
Expand All @@ -435,12 +437,10 @@ pub fn set_provider_secret(
return Ok(None);
}
let api_key_ref = secret_ref_for_provider(id);
match secret_value
.map(str::trim)
.filter(|value| !value.is_empty())
{
let secret_value = secret_value.map(str::trim).map(fold_fullwidth);
match secret_value.filter(|value| !value.is_empty()) {
Some(value) => {
let backend = secrets.set(&api_key_ref, value)?;
let backend = secrets.set(&api_key_ref, &value)?;
upsert_secret_meta(db, &api_key_ref, id, &backend)?;
db.conn()
.prepare_cached(
Expand Down
Loading
Loading