Two had real security consequences:
The bootstrap rejection ran on npm dotenv's parser while process.loadEnvFile
applied the file with Node's own. Two independently maintained dialects meant
the check and the thing it guards could disagree: a name Node accepts but the
checker misses would reach process.env unchecked, and BASH_ENV there runs a
file of the project's choosing on every `bash -c` the bash tool issues. Parse
once with node:util's parseEnv — the same engine loadEnvFile uses — and assign
the entries already checked, which also drops the dotenv dependency.
llm-pi-ai still returned a literal profile.apiKey ahead of everything, and it
registers a settings namespace, so the defect removed from llm-deepseek
survived intact in its design twin. The field is gone from the profile schema,
the resolution path, and the tests.
The rest are consistency and documentation defects the review named:
- verify-config-source-ownership did not scan the Python runtime's bundled
cordis.yml, which still inlined apiKey and baseURL. Both are covered now, and
the line-anchored INLINE_DENY documents that it is a tripwire, not a parser.
- The deny list missed NODE_TLS_REJECT_UNAUTHORIZED, the askpass hooks, the
GIT_CONFIG_* redirections, and PYTHONHOME — all implied by its own stated
rule about what a variable does.
- Snapshot lookups folded case on Windows, where environment names are
case-insensitive and an exact-match Map could miss a higher-ranked layer.
- The credentials note claimed a read-time permission check was "not taken"
while this PR implemented it; the credentials-local README still described
two layers, live process.env reads, dotenv-era limitations, and a renamed
anchor; the llm-deepseek README still advertised the removed literal apiKey;
and web.ts and base.cordis.yml kept personal-overlay wording.
- The ownership note's literal-apiKey claim now names its scope: the
web-search providers keep a literal field but register no settings
namespace, so nothing can shadow a stored credential through them.
Review found five real defects in the configuration-source work, all confirmed
against the code rather than argued:
1. The note claimed --config outranks settings.yaml. It does not: the settings
seam registers a plugin's cordis entry config as the `base` layer and the
user section layers over it, and the seam cannot tell a shipped value from a
--config one. The note now states shipped reality and names --config-replace
as the lever for a deployment that must win. Separately, a literal `apiKey`
in settings outranked both the environment and .credentials.yaml — the field
is removed, so configuration carries a reference and nothing else.
2. DEEPSEEK_SEARCH_BASE_URL was functionally deleted: the shipped inline went
away without the provider learning to read it. It now resolves from the
environment snapshot, as the README always claimed.
3. The bootstrap deny list missed the interpreter start-up hooks. BASH_ENV is
the sharpest: `bash -c` sources it on every bash tool call, so a project
.env could run a file of its choosing before every command. The list now
covers BASH_ENV and its per-language siblings, the Git hook commands, and
the remaining preload and CA variables, organised by what a variable does
rather than which runtime owns it.
4. YAML parse errors quoted the offending source line — which in a credentials
document is the secret — into boot stderr and the watcher's logger. Only the
error code and position are reported now, in credentials-local and
settings-local alike, pinned by a test that asserts the secret is absent.
5. 0600 governed only files the harness wrote. A hand-created 0644 document was
read normally. POSIX now checks the mode before reading contents, at boot
and on every reload; Windows has no mode to inspect and is skipped rather
than faked.
The project a session is launched in is trusted by default, with no prompt and
no stored trust record: it may supply its own endpoint, ordinary variables, and
a key ranked below the managed store. Trust stops at the harness itself — a
discovered file still cannot set DSH_PERMISSION_MODE, PATH, BASH_ENV, or the
rest, because those take effect with no user action, before any turn, outside
the permission policy and the sandbox.
$DSH_HOME/.env had just become an ordinary environment layer, which left the
harness resolving user-facing values from a flattened process.env that could
no longer say where a value came from. A key stored through the web page
stayed shadowed by an older key in the user's own .env. An endpoint could be
redirected by the project: the invoking directory's .env is materialized like
every other layer, and a base URL decides where a resolved API key is sent, so
a DEEPSEEK_BASE_URL written into a model-editable workspace would send the
user's credential — and the prompts carrying their code — to whatever host
that file named.
Give every user-facing value one ordering, with four kinds of source:
explicit for this run per-operation override, CLI argument
> authored by deployment --config / --config-replace
> this launch's shell inherited process environment
> product-managed store settings.yaml, .credentials.yaml
> discovered file $DSH_HOME/.env
> defaults schema default, shipped base, public default
The domains differ only in which tiers exist. The earlier split — credentials
ranking the environment over the managed file while settings ranked over the
environment — was inconsistent: the distinguishing fact is who authored the
source, not the domain.
packages/util/environment owns an immutable snapshot with per-layer
provenance. getFrom(name, sources) searches only the layers a caller names,
and omitting one is a refusal rather than a demotion: the adapters ask for
['process', 'user-env'], so no reordering can let a project file back into a
decision it was excluded from.
isBootstrapOnly rejects, before anything is materialized, any .env setting a
variable that governs how a process launches (PATH, SHELL, NODE_OPTIONS,
LD_PRELOAD), where code or model-visible instructions load from (the whole
DSH_* namespace, HOME, XDG_*), or how the network is reached (proxy and CA
variables). The namespace is denied wholesale so a switch added later cannot
become settable by being forgotten, and there is no opt-out.
verify-config-source-ownership keeps both rules: no unregistered process.env
read under packages/*/*/src (26 allowlisted with reasons), and no apiKey,
baseURL, or headers inlined from the environment in shipped Cordis config —
removing those inlines is what makes the deployment tier meaningful.
$DSH_HOME/.env carried two incompatible jobs. As credentials-local's writable
secret store it could not be hoisted into process.env — hoisting makes every
stored key read as a read-only launch override and blocks rotation from the
TUI and the web page. But its name and dotenv format promise an environment
file, so a DEEPSEEK_BASE_URL sitting beside a working DEEPSEEK_API_KEY in the
same file was silently ignored: only the credential provider read the
document, and it addresses credential references alone.
Split the two jobs into two files.
.credentials.yaml is the provider-managed store: a strict YAML mapping of
CredentialRef to non-empty string, no version field, no wrapper level. Because
it holds credentials and nothing else, a non-mapping root, a non-identifier
key, a non-string value, an empty string, a duplicate key, and malformed YAML
are all rejections rather than skipped entries — loud at boot and at a write,
warn-and-keep-last-good on a live reload. The dotenv physical-line editor
gives way to a patch of the parsed document, so comments and untouched entries
keep their formatting and any string value round-trips, multi-line included.
Writer lock, read-modify-write, atomic 0600 write under a 0700 directory,
watcher, self-write suppression, and quiescent disposal are unchanged.
$DSH_HOME/.env becomes the user's ordinary environment layer. app-boot's new
loadLayeredEnv loads the invoking directory's .env then the Harness home's,
giving user < project < inherited; the home resolves from the inherited
environment first, so a project .env cannot redirect it.
Credential precedence is unchanged: the live environment still wins read-only
over the file, and shadowed writes still reject. Whether a provider-managed
store should instead win over the environment is a separate decision.
No migration: a key already in $DSH_HOME/.env keeps resolving through the new
environment layer, as a read-only env source that shadows the stored one.
Five findings from the #939 review, each reproduced before being fixed.
**Configuration reads are as privileged as writes.** `settings.describe`
returns every exposed namespace's configuration and `credentials.describe`
reports whether an arbitrary environment-variable name is configured and from
where — reconnaissance no anonymous caller should have. Both join
PRIVILEGED_METHODS, so the whole configuration plane is loopback-only until
real authentication exists; `trustedHosts` was never authentication. The model
catalog stays reachable: it carries no endpoints or key state, and a LAN
client's model picker legitimately needs it. Asserted over a real HTTP server,
because the Host header a browser actually sends is what decides this.
**The proxy serves only namespaces a registered model provider addresses.**
The settings seam is general — any plugin may register one — but the Web
configuration plane is the model-provider surface. Without the gate, every
future `settings.register()` would silently become remotely readable and
writable configuration. An unregistered namespace and an unexposed one answer
identically, so no caller can enumerate the registry one probe at a time.
**Path-addressed writes replace the redacted-document rebuild.** The editor
reads the REDACTED descriptor, so rebuilding a section from it and replacing
wholesale deleted every literal secret the wire never returned — reproduced as
`{baseURL, reasoning}` in, stored `apiKey` gone out. `settings.mutate` applies
set/unset ops to the section as it stands at the front of the seam's write
queue, and the client names only fields it can see, so an unseen secret is
untouched by construction rather than by care.
P2s in the same pass: `llm/adapters-updated` now contains async listener
rejections (an uncontained one escaped as unhandledRejection, contradicting
the documented "observer failures are contained"); llm-deepseek's retry-policy
swap uses the atomic `registration.replace` instead of dispose-then-register,
which published `[]` then `["deepseek-official"]` so an observer saw the
provider disappear and come back; and a transport rejection no longer strands
the page in `loading` or a card in `busy`, with removal failures surfaced on
the page banner instead of swallowed.
The policy home's resolve() now stamps readDenyPaths, so every consumer
that pins the resolved shape (bash-sandbox hand-off, tool-fs stamps)
carries it, and three uncovered branches gained real tests: landlock
reporting partial enforcement for a denial it cannot express, the
policy's default under programmatic construction, and both ambient
credential paths in llm-deepseek without a mounted seam.
Four review findings across the seam and both adapters.
registerAdapter now returns a handle carrying replace(providers): the
candidate route set is validated in full before anything moves, so a
route another adapter owns leaves the previous registration intact, and
the swap itself is one synchronous section with no observable gap. pi-ai
uses it instead of dispose-then-register — the old shape dropped every
route when the new set conflicted, and its facts cache could then equal
the registry's, so reverting to a working configuration never re-applied.
Its registration facts are also sorted by provider, so a settings
document that merely reorders keys no longer triggers a swap.
DeepSeek's per-request snapshot now carries the credential facts, and
resolveApiKey receives it instead of re-reading the raw config: a
settings generation the resolver rejects can no longer contribute its
literal key to a request the previous generation's endpoint serves.
pi-ai only defers to the SDK's provider-native discovery when a profile
names no credential at all; a configured apiKeyEnv that misses now fails
with MISSING_CREDENTIAL naming the route and the reference, instead of
handing pi-ai undefined and letting it authenticate with an unrelated
ambient key.
The eager boot-time credential probe is gone: it could run before the
credentials service mounted and reported every failure as a missing key.
The route stays registered and browsable; the first request gives the
accurate error, whose guidance now leads with the credential store and
mentions a literal apiKey last.
The shipped surfaces loaded the harness home's .env into the process
environment before cordis booted. credentials-local then saw every stored
key as an ambient launch override: describe reported source 'env' with
writable false, and set/unset rejected as shadowed — so a key the web page
or TUI stored was unrotatable and undeletable from the next run onward,
and the adapter kept using the value captured at launch.
The home's .env is now the credential provider's own store, read by that
provider alone and hot-reloaded by it. The genuine launch environment and
the invoking directory's .env (loaded by the bin) remain the read-only
ambient layer, so a plain composition without the provider still resolves
keys exactly as before.
Proven by a real restart in the loader composition: store a key through
the seam, dispose the tree, re-boot over the same harness home, and the
entry is still file-sourced and writable — rotating it lands on the very
next request.
ctx.llm gains 'llm/adapters-updated' — a payload-free registry notification
emitted at every topology commit point (adapter routes registering or
disposing, directory entries appearing or withdrawing) with contained
observers and INVARIANT rethrow — plus registerConfigurableProviders/
listConfigurableProviders, the directory of routes an adapter plugin can
activate through configuration. llm-deepseek declares deepseek-official
(whole llm-deepseek section as profile); llm-pi-ai declares the full
installed catalog under providers.<provider> even while dormant, so the
web settings surface can offer every provider before any route exists.
The invariant companion asserts the registry stays readable at each
notification.
The native adapter's route was named deepseek, colliding with pi-ai's
catalog provider of the same name, so the two DeepSeek paths could never
be mounted side by side. The web settings page needs both configurable at
once. Compositions, fixtures, goldens, scaffolding defaults, and docs all
move together (pre-release, no shim); TUI/session-query-spill/
missing-credential goldens re-recorded through their keyless refresh
modes because provider-name length shifts box padding and spill
truncation points.
llm-deepseek gains a Loader+Include composition spec proving external
settings.yaml/.env edits reach the very next request, and a real-API e2e
where only a credentials-local document holds the key. The headless example
pins the first-run missing-credential UX as a keyless stream-json snapshot
(new credentials.cordis.snapshot.yml scenario); runLoaderSmoke learns
expectedExitCode so a designed failure surface can be pinned instead of
masked.
The adapter takes an options thunk and a per-stream credential resolver
instead of frozen construction facts: base URL, catalog, defaults, idle
budget, and the API key re-resolve at each operation, so a settings or
credential change reaches the very next request while in-flight streams
keep the facts they started with. resolveAdapterOptions is the one explicit
resolve step (entry config fails loud at load; a live snapshot failing a
beyond-schema bound keeps the last good options). The plugin layers its
entry config under the optional llm-deepseek settings section and resolves
keys literal-first through ctx.credentials with an ambient env fallback; a
missing key now registers the route, warns, and fails each request with
actionable MISSING_CREDENTIAL instead of failing plugin load. The
registration-captured retry policy re-registers the route in place when it
changes.
Implements the approved simplification Agent Note: sse.ts now pipes the
response body through TextDecoderStream and EventSourceParserStream
(eventsource-parser/stream) and keeps only the DeepSeek protocol shim —
yield each event's data, terminate on [DONE], throw
LlmError('STREAM_CLOSED') on EOF without the sentinel. The SSE
spec-conformance tests are deleted; sse.spec.ts pins only the
[DONE]/STREAM_CLOSED/EOF contract, including the new spec-strict verdict
that an unterminated trailing event is truncation (the old parser
flushed it — a robustness nicety no real provider shape needs).
eventsource-parser@^3.1.0 becomes llm-deepseek's second runtime
dependency (already in the lockfile transitively via the MCP SDK).
Docs: the Agent Note moves proposed/ → implemented/ and is rewritten per
the lifecycle contract; the rejected NIH roll-up note's inbound links
follow. The twin-adapters note, dsh-llm LlmAdapter JSDoc (and its
type-equiv fences), cookbook, group/package READMEs, root AGENTS.md
layout line, sdk-helper comments, and the regenerated config catalog
drop the "hand-rolled fetch + SSE" claim in both languages; all eight
touched pairs re-recorded.
A well-formed provider stream that ends with finish_reason stop and zero
content blocks previously became a successful empty assistant message: the
turn completed silently, and drivers like goal-session counted the no-op
round. Both adapters now map that degenerate completion to a finish
{kind:'error'} with the new canonical EMPTY_RESPONSE code from dsh-llm, and
dsh-llm-retry adds the code to its default retryable set, so the existing
closed-step recovery path retries it and fails loud once the budget is
exhausted.
Covered by adapter unit tests, an llm-retry default-policy test, and a new
authored keyless ACP snapshot (empty-response-retry) with a deterministic
1 ms zero-jitter retry overlay.
17 master merges (GUI host/web stack, compact header, dshweb, fast local
hooks, windows coverage skips, worktree runtime). Conflicts:
- packages/ui/user-interaction/src/index.ts: master extracted the question
types to types.ts; took the re-export and grafted our detail field (the
plan-review payload) into the extracted AskUserQuestionItem.
- generated catalogs regenerated over both sides.
The module graph crossed mermaid's default 500-edge render guard with the
GUI packages plus our plan-mode edges; raised maxEdges in verify-mermaid
(a secure config settable only at initialize).
Note: the remote branch also renamed packages/mode -> packages/plan
(PR #512 plan-mode-simplification, fast-forwarded before this merge).
Master arrivals re-grafted onto the plan-mode surfaces:
- Package invariant runtime contracts: every package owns ./invariant —
added the dsh-mode companion (mode/set payload validation: non-empty
trimmed name), its unit spec, exports/peer wiring, and the mode group in
the tsconfig invariant paths glob.
- Interception waterfalls gained an AbortSignal parameter
(agent/prompt-submit, agent/turn-continuation) and ToolExecutionInput.signal
became required: listeners and test dispatch updated; the exit tool now
forwards exec.signal unconditionally.
- TUI question dialog: master's position/unanswered header layout kept,
the plan-review detail block re-grafted between question and answers.
- LSP capability family arrived: lsp tool joins the catalog pin, dsh-lsp
examples deps beside dsh-mode; lsp-definition snapshot refreshed with
the mode command and modes advertisement.
- docs/architecture.md and packages/README.md regenerated tables re-grafted
(ctx.modes row, mode package row) and condensed within word ceilings.
- examples/acp-agent cordis.yml: kept the deployment-owned plan instructions
(incl. the conversational-agreement sentence) beside master's routed-model
compaction comment; snapshot expected outputs refreshed keyless and noise
fixtures restored.
A TUI run against an unreachable endpoint failed with only 'fetch failed':
undici wraps transport failures in a bare TypeError whose diagnosis lives
on .cause, and every diagnostic seam rendered only error.message. The
readline front door additionally rendered failed turns as pure silence.
- dsh-llm: new errorChain(value) renders the full cause chain and
AggregateError members with circular/hostile-coercion containment.
- llm-deepseek: pre-response transport failures throw LlmError('NETWORK')
naming the endpoint and chaining the fetch TypeError; aborts keep their
DOMException so the loop still classifies them as cancellation.
- agent-loop: durable turn/end error messages and logger warnings render
through errorChain; local renderThrown copies removed.
- ui-stdio: failure turn/end reasons now render ([turn failed <code>],
[turn aborted], [turn rejected], output-token-limit); startup-failure
logs use errorChain.
- ui-tui: agent/error notices and the startup-failure line use errorChain.
