Redact credentials embedded in provider endpoints, and bound the provider name by graycyrus · Pull Request #1 · graycyrus/opencompany

graycyrus · 2026-09-11T19:23:14Z

Two defects on the inference surface, found by a QA sweep. Branches from
docs/inference-rework (PR tinyhumansai#2262) at 7a7393720 and is a strict
fast-forward of it, so it can also be landed with a plain FF push rather
than merged.

Redact credentials embedded in provider endpoints

An endpoint URL can carry userinfo (http://user:password@host/v1).
Nothing refused one on the way in, and several places interpolated the
stored string on the way out — including the catalog-read failure note
and the base_url field of GET …/inference, which is ScopedCompany
rather than admin.

Two independent mechanisms, because they cover different populations:

Refused wherever an endpoint is accepted —
catalogue::endpoint_has_credentials gates normalize_local_endpoint
(the funnel every stored endpoint passes through) and validate_parts
(the manifest and console-PUT half of the same rule). Same class of
rule as the api_key_secret check already beside it. The draft probe
refuses one too, before the request is made.
Redacted wherever an endpoint is said — catalogue::redact_endpoint
masks the userinfo in every DTO field, operator-facing message and log
line that names an endpoint. Refusal cannot reach a value stored before
the rule existed, or one from a company.toml or
OPENCOMPANY_INFERENCE_URL this host does not own. The endpoint used
to make a request is untouched.

The subtle half is the failure text: reqwest already masks userinfo in
its own Display, so an error quoting the upstream string looks safe —
and then a format! that adds our copy of the URL beside it puts it
straight back. Three separate messages did exactly that; two were found
by the new end-to-end test rather than by reading.

Strict manifest validate() runs only on a first boot with no persisted
record, so the refusal cannot brick a company already running on such an
endpoint. That company is covered by redaction instead.

frontend/src/inference/types.ts claimed no shape in it could carry a
credential. baseUrl could; it now says so.

A long provider name no longer corrupts the secret store

FsSecretStore reads and writes a legacy secret path whose filename is
the un-truncated slug of the whole key, unlike the canonical path, which
is digest-bounded. A key past NAME_MAX made the kernel answer
ENAMETOOLONG rather than ENOENT, and both call sites surfaced that as
a store error:

get turned a plain credential read into a 500.
set's clear path had already written the canonical file before
looking for a legacy file to revoke — so clearing a credential returned
an error with the key already truncated to zero bytes and the provider
row still listed, repairable only by hand-editing the index.

A name too long to be a path component cannot name a file that exists,
so ENAMETOOLONG now reads as absent, exactly like ENOENT. Every other
error kind still propagates. The SQLite and MongoDB secret stores have no
legacy path and were unaffected.

The input side is bounded too: 80 characters, matching
MAX_DISPLAY_NAME_CHARS — the bound this codebase already uses for the
other name a person types and reads back in a list. It also keeps the
derived secret key inside the canonical filename budget, so a provider's
credential file stays the readable %k- form. Held host-side in
check_provider_name and check_slug; the console mirrors it so the
operator is told beside the field rather than after a round trip.

Tests

Cover the reported paths: never stored, not echoed in the catalog-read
failure, redacted in the read DTO and in a provider row; an @ in a path
not mistaken for a credential; a password that itself contains an @;
and the provider name at the bound, past it, and through the clear-path
write that used to fail after truncating the key.

Verified locally

cargo fmt --all -- --check — clean
cargo clippy --all-targets -- -D warnings — clean
cargo test — 5255 lib + 26 integration passed, 0 failed
cargo check --features openhuman,mcp — exit 0, no new warnings
(the two remaining are pre-existing, in vendor/openhuman)
npm run typecheck, typecheck:unit, typecheck:e2e — all three clean
npm test (frontend) — 5463 passed, 0 failed
scripts/ci/assert-design-tokens.sh, assert-feature-lanes.sh,
assert-toolchain-pin.sh — all pass

`Bundle::legacy_secret` slugs the whole key into one path component with no length bound, unlike the canonical `Bundle::secret`, which is digest-truncated. A key past `NAME_MAX` made the kernel answer `ENAMETOOLONG` rather than `ENOENT`, and both callers surfaced that as a store error. `get` turned a plain credential read into a 500. `set`'s clear path had already written the canonical file before it went looking for a legacy file to revoke, so clearing a credential returned an error with the key already truncated to zero bytes and the record still listed. A name too long to be a path component cannot name a file that exists, so `ENAMETOOLONG` reads as absent, exactly like `ENOENT`. Every other error kind still propagates.

… carries a credential Two rules the provider write plane did not hold. The name had no length limit, and `slugify` turns it into the address of a secret (`provider/<slug>/key`). An unbounded name produced an unbounded secret key, which is how a long name reached the filesystem store's legacy path and broke it. Eighty characters, matching `MAX_DISPLAY_NAME_CHARS` — the bound this codebase already uses for the other name a person types and reads back in a list. It also keeps the derived key inside the canonical secret filename budget, so a provider's credential file stays the readable `%k-` form. Enforced on the name in `plan_add` and `edit_provider`, and on the derived slug in `check_slug` — the function that stands between a typed name and a secret address. An endpoint may carry userinfo (`http://user:password@host/v1`), and nothing refused one. `endpoint_refusal` says which of the two reasons an endpoint was rejected for, and the draft probe refuses a credentialed URL before it makes the request rather than after.

An endpoint URL can carry userinfo. Nothing refused one on the way in, and several places interpolated the stored string on the way out — including the catalog-read failure note and the `base_url` field of the company status read, which is a `ScopedCompany` route every console reader calls on every page load. Two independent mechanisms, because already-stored values and values arriving from a manifest or `OPENCOMPANY_INFERENCE_URL` are not covered by a rule applied at the point of entry: - `catalogue::endpoint_has_credentials` refuses such an endpoint wherever one is accepted — `normalize_local_endpoint`, which every stored endpoint passes through, and `validate_parts`, the manifest and console-`PUT` half of the same rule. It is the same class of rule as the existing `api_key_secret` check beside it. - `catalogue::redact_endpoint` masks the userinfo wherever an endpoint is *said* — both catalog DTOs and their failure notes, the status and managed DTOs, the provider list, the no-key advisory and the probe failure reading. The endpoint used to make the request is untouched. The console mirrors both rules so the operator is told beside the field rather than after a round trip, and `types.ts` no longer claims no shape in it can carry a credential without saying which field could.

…path Two more places built a message out of the raw endpoint, both found by the new end-to-end test rather than by reading: - `inference_models::fetch_catalog` writes `request to {url} failed: {error}`. `reqwest` had already masked the userinfo in its own half of that string; our half put it back, and the result is the text the catalog route returns to the console. - `probe::probe_models` does the same into `ProbeFailure::raw`, which reaches a host log — and a log is disk. Tests cover the three reported paths (never stored, not echoed in the catalog-read failure, redacted in the read DTO and in a provider row), an `@` in a path not being mistaken for a credential, a password that itself contains an `@`, and the provider name at the bound, past it, and through the clear-path write that used to fail after truncating the key.

…slots A `base_url` is the fifth place a credential can hide and the only one of them that is read back on a non-admin route. Says which mechanism covers which population, why the failure-text case is the subtle one, and why the refusal cannot brick a company already running on such an endpoint.

`model_unavailable_advice` tells the operator to `GET {models_url}`, built from the company's `base_url`. That sentence reaches the console and gets screenshotted into tickets, so an endpoint carrying userinfo put the credential there too. Redacted inside the function rather than at its two call sites, so a third caller cannot reintroduce it.

`InferenceReadyDto.base_url` reported `OPENCOMPANY_INFERENCE_URL` verbatim, on the reasoning that a URL is not a secret. A URL can carry userinfo, and this is the one endpoint no input rule this workload holds could have kept clean: the deployer sets it, not a tenant.

graycyrus · 2026-09-11T19:26:42Z

Superseded by tinyhumansai#2281, which tracks the same branch against the upstream repo where the team reviews.

graycyrus added 7 commits September 12, 2026 00:47

graycyrus closed this Sep 11, 2026

Navigation Menu

Repository navigation

Redact credentials embedded in provider endpoints, and bound the provider name - #1

Redact credentials embedded in provider endpoints, and bound the provider name#1
graycyrus wants to merge 7 commits into
docs/inference-reworkfrom
fix/inference-credential-safety

graycyrus commented Sep 11, 2026

Uh oh!

graycyrus commented Sep 11, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

graycyrus commented Sep 11, 2026