Obsidian's source-code review forbids disabling obsidianmd/prefer-window-timers
and obsidianmd/no-global-this (and flags undescribed directive comments). The
0.11.32 review failed on five such disables.
- router.ts, obsidian-api.ts, session-manager.ts: global set/clearInterval and
setTimeout → window.* equivalents.
- debug.ts: drop the globalThis console fallback → window.console.
- tests/setup.ts: alias window to globalThis in the node test env so the
window.* timers/console resolve under Jest (Obsidian always provides window
at runtime).
- session-manager cleanupInterval typed number (window.setInterval's return).
All five eslint-disable directives removed. Lint clean, build green, 336 tests pass.
The dev-dependency bump tightened lint and exposed pre-existing debt.
Resolved without blanket rule suppression:
- no-unnecessary-type-assertion (15): removed redundant assertions
(eslint --fix) across formatters, mcp-server, security, utils, validation.
- no-base-to-string (6): the removed assertions un-masked String(unknown)
calls. All were already guarded (object→JSON.stringify, primitive→String),
so not bugs — narrowed via a typed primitive local, which also satisfies
no-unnecessary-type-assertion (whose receiver String() accepts anything).
- no-deprecated (15): obsidian 1.13.0 deprecates PluginSettingTab.display()
in favor of getSettingDefinitions(). Kept display() as the supported
cross-version fallback and routed internal re-renders through a new
render() method, so no deprecated symbol is referenced and no future
deprecation in main.ts is masked. Full declarative migration: #224.
- no-deprecated (1, ButtonComponent.setWarning): the replacement
setDestructive() requires 1.13.0 > our minAppVersion 1.6.6, so it's kept
with one scoped eslint-disable + comment. Revisit with #224.
- no-unused-vars (2): dropped now-unused http type imports in mcp-server.
- prefer-active-doc (5): document → activeDocument for popout-window
compatibility (main settings DOM queries, image-handler canvas).
Lint clean, build green, 336 tests pass.
The '/' traverse summary reported Math.min(10, getFiles().length) using the
unfiltered file count, while the traversal actually seeds from the
.mcpignore-filtered set (graph-traversal.ts). With exclusions enabled this
overcounted and leaked that hidden files exist via an inflated number. Count
the same filtered set the seed logic uses.
Graph operations (neighbors/backlinks/forwardlinks/traverse/path/
statistics) read metadataCache.resolvedLinks and vault.getFiles()
directly and never consulted the MCPIgnoreManager. With path
exclusions enabled, .mcpignore-ignored paths surfaced as neighbors/
edges of non-ignored queries, and querying an ignored note directly
disclosed its relationships.
- GraphSearchTool rejects an excluded query root (treated as
"File not found", matching ObsidianAPI's read-path behavior).
- GraphTraversal filters excluded paths out of the link primitives and
every file enumeration (neighbors, root traversal, tag connections,
vault statistics, all-nodes).
- Add ObsidianAPI.getIgnoreManager() accessor + regression tests.
No behavior change when path exclusions are disabled.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Dataview's `query()` resolves to a `Result` monad —
`{ successful, value: { type, values, headers }, error }` — where
`value.values` is a plain array. The tool modelled the return as a flat
`{ type, values }` object with a wrapped `DataArray`, so:
- `formatQueryResult()` switched on `result.type` (undefined → `result.value.type`),
collapsing every query to the `unknown` branch → `# Dataview: UNKNOWN`.
- it called `result.values?.array()` (undefined → `result.value.values`,
already a plain array) → `[]` → "No results found".
`dataview.status` had a separate shape mismatch: the detector returns
`{ installed, enabled, apiReady, version }` but `formatDataviewStatus`
reads `available`, so it always rendered "not available".
Fixes:
- Re-model `DataviewQueryResult` as the real monad; read payload from `.value`.
- Add `toPlainArray()` helper (accepts plain arrays and wrapped DataArrays).
- Add a `dataview.status` normalizer mapping apiReady → available.
- Correct the test mock, which encoded the same wrong flat+wrapped shape —
the reason CI stayed green while real Dataview v0.5.68 failed.
- Add formatted-output regression tests through formatResponse() for
query (LIST/TABLE) and status.
Before: graph.statistics threw 'Source path is required' when called
without sourcePath, blocking the baseline use case (vault health
snapshots, dashboards). Users had to pick an arbitrary seed and reason
about its neighborhood — not representative.
After: when sourcePath is omitted, return a new vaultStatistics shape:
totalNotes — count of .md files in the vault
totalLinks — sum of resolved link occurrences (Obsidian
semantics: A→B referenced 3× counts as 3)
orphanCount — singletons (no links in either direction)
averageDegree — 2 * totalLinks / totalNotes
largestComponentSize — biggest connected subgraph (undirected)
isolatedClusters — total connected-component count,
inclusive of singletons (so non-trivial =
isolatedClusters - orphanCount)
Per-node statistics behaviour is unchanged when sourcePath is provided.
Implementation: GraphTraversal.getVaultStatistics does one O(V+E)
pass over the resolvedLinks adjacency, plus BFS for components.
Treats the graph as undirected for component analysis (a link from A
to B means A and B are in the same component) while keeping link
counts directed (matches how metadataCache exposes them).
Tests: tests/graph-statistics-vault-wide.test.ts covers the
vault-wide path (the topology described above), per-node fallback,
empty vault, repeated occurrences, non-md filtering, and the
directed-edges-undirected-components invariant.
- vault.update content-guard hint pointed at vault.patch, which does not
exist — patch lives at edit.patch. Bad recovery hint is worse than no
hint.
- edit.window guard test claimed "before searching the file" but only
asserted on mutations; the mock's 'original' content doesn't contain
'undefined' so even a leaked guard wouldn't show a write. Add a reads
recorder and assert getFile is never called, making the test honest
about what it proves.
- Strip a stray space from the lock-key guard comment.
#210 reported that `vault.update` with no `content` writes the literal
9-byte string `"undefined"` to the target file — `String(undefined)`
coerced silently, the validator accepted the 9-byte string, and the
formatter rendered the misleading heading `✓ Updated: undefined`.
The same `String(params.x)` footgun existed at every vault-mutating
dispatch site: `edit.append` (would append "undefined"), `edit.window`
(would search for or replace with "undefined"), plus path-only cases
that produced cryptic "File not found: undefined" errors instead of a
clean dispatch-level rejection.
Fix the class, not just the instance:
- Add `requireParamStr(params, key, action, hint?)` in operations/shared.
Throws at the boundary when a required string param is missing or
wrong-typed, so a malformed MCP call cannot reach a vault sink.
- Convert the corruption-class sites (`vault.update`, `vault.create`,
`vault.delete`, `edit.append`, `edit.window`) and the noisy path-only
sites (`edit.patch`, `edit.at_line`, `edit.from_buffer`, `view.file`,
`view.window`, `view.open_in_obsidian`) to use the helper.
- `vault.update`'s `content` guard carries a hint pointing at
`vault.patch` / `edit.window` — that's the wrong-tool path that
produced the #210 corruption in the wild.
- Lift `edit.*`'s path guard above `FileLockManager.withLock` so a
missing path can't take a lock on the literal string "undefined".
- `updateFile` / `deleteFile` now return `{ success, path }` so
`formatFileWrite` renders `Updated: <path>` — belt-and-suspenders for
the second issue called out in #210.
Tests: `tests/dispatch-param-guards.test.ts` records every mutation a
mock API receives; for each malformed call we assert the route rejects
AND no mutation was recorded, proving the guard runs before any sink.
Blocking — userSuppliedCert discriminator:
- Was: certificateConfig.selfSigned === false (never set false in UI;
rows 5/7/9 of the ADR-107 table unreachable, defaulting to over-warn).
- Now: !!(certPath && keyPath) — actual cert provenance signal.
Fixed in src/mcp-server.ts and src/main.ts settings UI badge.
Non-blocking #1 — migration Notice race:
- Was: isExistingPreBindModeInstall() re-read data.json after
loadSettings() may have already persisted bindMode='loopback' as
part of the apiKey auto-save.
- Now: snapshot raw loadData() before loadSettings(), test the snapshot.
Non-blocking #2 — loopback host matcher tightened:
- Was: startsWith('127.') accepted '127.evil.com'.
- Now: strict dotted-quad regex /^127(\.\d{1,3}){3}$/ with octet range.
New tests cover hostile inputs (127.evil.com, 127.0.0.1.attacker.tld,
127.300.0.1, 1270.0.0.1).
Non-blocking #3 — Reconfigure hint:
- ADR-107 specifies the 🔴 badge includes a Reconfigure hint;
implementation now renders 'Reconfigure: switch the bind address
below to Loopback, or enable HTTPS.' beneath the verdict reason.
Non-blocking #4 — empty custom host UX:
- When bindMode='custom' && customBindHost is empty, render an
explicit Notice that the server will fall back to loopback until a
host is entered (resolves the silent 🟢 OK in that state).
Lint clean, 296/296 tests pass.
Replaces the implicit 0.0.0.0 Node-default bind with a deliberate,
classified network exposure model.
Server: MCPHttpServer.start() now resolves the listen host from
settings (bindMode + customBindHost), passes it to listen(), and
classifies the combined (protocol × bind × certSource) state. On 🔴
fires a 15s Notice + Debug.error; on 🟡 logs Debug.warn; on 🟢 silent.
The verdict-derived agent-visible warning is pushed into MCPServerPool
which conditionally injects it into the MCP initialize.instructions
field for every session created after bind, so an LLM client sees the
warning on its first turn against the server. node-mcp-server.ts (the
unused fallback) is hardcoded to loopback for consistency.
Settings: MCPPluginSettings gains bindMode ('loopback' | 'all' |
'custom') default 'loopback', customBindHost default '', and a
one-shot hasShownBindMigrationNotice flag. Existing installs with a
data.json that predates bindMode see a one-time 20s Notice on next
load pointing them at the new setting; fresh installs are silent.
UI: a new "Network binding" section between server config and the
HTTPS section, with a live colored verdict badge that re-classifies
on every change, the bind dropdown (Loopback / All interfaces /
Custom), an inline red caution under "All interfaces" whose wording
adapts to whether HTTPS is enabled, and a custom-host text field
that normalizes loopback/wildcard aliases on blur (typing 127.0.0.1
collapses the dropdown to Loopback; 0.0.0.0 collapses to All).
Tests: 7 integration assertions verifying the source wires listen()
to the resolved host, pushes verdict-derived instructions into the
pool, conditionally injects on 🔴 only, and that the pool spread-
pattern omits the field on ok/warn.
Inspired by PR #208 (fa1k3) — preserves the security instinct while
keeping the LAN/remote escape hatch the hardcoded approach removed.
Pure functions consumed by the bind-time guard, settings UI, and MCP
initialize.instructions injector: classify(NetworkState) → Verdict,
resolveListenHost(), resolveBindAxis(), normalizeBindInput(),
classifyFromSettings(), agentInstructionsForVerdict(). Single source
of truth for "how exposed is the current network configuration?".
The 9-cell ADR-107 table maps (protocol × bind × certSource) → one of
🟢 ok / 🟡 warn / 🔴 jail. Custom-input normalization collapses
loopback aliases (127.x.x.x / localhost / ::1) and wildcard aliases
(0.0.0.0 / ::) to the corresponding explicit BindMode so the same
intent expressed three ways converges to one state.
40 unit tests cover all 9 table cells, all normalization paths, the
defensive empty-custom fallback, and the agent-instructions formatter.
formatFileRead truncated the string branch to a 50-line preview; since raw
defaults false, the DEFAULT vault.read was that truncated view — an agent
reading any >50-line file could not derive a byte-matching edit.window
oldText without raw:true, the exact #133 friction ADR-203 retires. The data
layer was already byte-faithful; the presentation default undercut it.
- string branch: emit full content verbatim, no 50-line slice, no wrapping
code fence (body may contain fences — a wrapper would corrupt round-trip)
- add non-text/binary passthrough guard (closes a pre-existing latent
formatter-error path; reviewer non-blocking note)
- test now asserts the FORMATTED (raw:false) output is byte-exact, not just
not.toThrow
make check green: 0 errors, baseline 5 warnings, 244/244.
Default vault.read no longer fragments-and-flattens. New contract:
- fits READ_PAGE_CHARS (50000) → whole file, byte-exact, one load (common case)
- exceeds → verbatim page 1, single contiguous block, with line bookends
(lineStart/lineEnd/totalLines + nextPage); page=N to continue. Absolute
line numbers preserved so edit.at_line still works on large files.
- returnFullFile:true → entire file verbatim (explicit large override; param
retained & repurposed, not retired)
- query/strategy/maxFragments → semantic fragments (unchanged)
- structured envelope no longer double-encodes the body (metadata sans body)
Budget is char-based on purpose: line count is an invalid proxy for context
cost; only bookends are line-based (for at_line). Hard invariant: a default
read must never hand the agent a context-breaking raw dump.
Also fixes the latent formatFileRead crash (_Formatter error_) on full-file
shapes; formatter now renders verbatim + a Pagination section.
src/utils/file-reader.ts rewritten; vault.ts threads `page`; formatter
hardened; tool description + CHANGELOG (breaking) updated.
make check green (0 errors, baseline 5 warnings, 243/243 incl. 8 new
ADR-203 round-trip/pagination/fidelity tests).
splitContent/sortFiles don't touch RouterContext — the uniform ctx-first
param was a mechanical-extraction artifact. Removed from both defs and
their two call sites. Indentation nit deliberately NOT addressed: the
retained class-method indentation is what makes the extraction a
byte-verifiable behaviour-preserving move (per the review's normalized
diff); reflowing would destroy that property for zero functional gain.
tsc clean, lint baseline, 235/235.
router.ts was a 2,228-line monolith (~2.8x the 800-line threshold). Extract
the largest unit — executeVaultOperation + its helpers — into
src/semantic/operations/vault.ts behind a RouterContext interface.
- operations/router-context.ts: RouterContext (api/app/fragmentRetriever/
validator); SemanticRouter implements it and passes itself, so shared
state propagates with no indirection
- operations/shared.ts: Params, SearchResultItem, paramStr/Num/Bool
(router-private → shared by all handlers)
- operations/vault.ts: executeVaultOperation + splitContent/sortFiles/
copyFile/copyDirectoryRecursive; mechanical this.→ctx. (tsc is the
safety net — a missed rewrite fails compile)
- router fields api/app/fragmentRetriever/validator → public readonly
- removed a fully unreachable pre-existing dead subtree (combineSearchResults,
isDirectory, performFileBasedSearch, indexVaultFiles, getSearchWorkflowHints,
extractContext, getFileType — zero call sites anywhere; eslint doesn't flag
unused class methods, which hid them). ~290 dead lines gone.
router.ts 2228→988; vault.ts 948. Behaviour-preserving: tsc clean, lint at
the exact pre-existing baseline (0 errors, 5 unrelated warnings), 235/235
tests unchanged.
Staged per the issue's "extract vault first in isolation" guidance: #199
stays open for the remaining handlers (edit/view/graph/system/bases →
router thin dispatcher <300 lines). ADR-202.
Parallel edit.window/append/patch/at_line/from_buffer calls against the
same file (the recommended batched-tool pattern for MCP clients) ran
overlapping read-modify-write cycles: every call returned "Edit
successful" but only one edit survived, with no error surfaced — silent
data loss.
A new SemanticRouter is constructed per request, so a per-instance lock
cannot serialize concurrent requests. Add a process-wide FileLockManager
singleton (same pattern as ContentBufferManager) that serializes
operations on the *same* file path in arrival order while leaving
different paths fully concurrent. executeEditOperation runs its whole
action switch inside withLock(params.path, …).
- src/utils/file-lock.ts: promise-chain-per-path lock; chains drain so
the map stays bounded; a rejected holder doesn't break later waiters.
- router.ts: wrap executeEditOperation in the per-file lock; also move
the window-edit dynamic import into the two cases that use it (window,
from_buffer) instead of eagerly on every edit action — append/patch/
at_line no longer load it, and the path is unit-testable.
- tests/file-lock-edit-serialization.test.ts: FileLockManager unit
behaviour + a router-level #139 repro (3 concurrent appends; without
the lock the read-modify-write MockAPI loses 2 of 3 — with it all
land) + a different-files-stay-concurrent assertion.
Scope: edit.* actions (the #139 report). No router extraction (that is
#199's own PR). make check green (build + lint 0 errors + 231/231).
Stale/evicted MCP sessions were unrecoverable without a client restart
(#128): a non-initialize request bearing an evicted Mcp-Session-Id hit a
server-side synthetic `initialize` that cannot drive SDK 1.29's
web-standard transport to an initialized state (proven in #190 — non-stream
compatReq, hono RequestError→400, post-hoc accept header lost→406), "failed
open", and returned a non-spec 400 -32000 "Server not initialized" that no
client treats as session-expiry → infinite 400 loop.
Emit the Streamable HTTP spec's session-lifecycle signal and let the client
re-initialize itself:
- evicted session + non-initialize → HTTP 404 + Mcp-Session-Id (spec §3);
compliant client/bridge re-inits per §4, next request opens a fresh
session — no restart (fixes#128)
- no session id + non-initialize → HTTP 400 (spec §2)
- initialize (fresh or recreate) → unchanged, never short-circuited
Delete the synthetic compat-initialize block, requireInitializeNotice
machinery, and the createNullRes/NullResponse shim. New
sendSessionTerminated() helper centralises the spec response.
ADR-106 records the decision, the restated acceptance (404-signal +
2xx-reinit pair), and the known client caveat (spec §4 is the client's
obligation; mcp-remote reaches it via connectToRemoteServer recursion).
Regression harness tests/mcp-session-reinit.test.ts drives the request
pair through handleMCPRequest deterministically (paths return before any
SDK transport — the thing #190 showed cannot be driven synthetically).
ADR numbered 106 (not 105) to avoid collision with the #197 branch's
ADR-105; contiguous once both merge. make check green (build + lint 0
errors + 229/229 tests).
The worker-thread pool ratified/extended by ADR-104 has never executed a
single request since introduction (v0.5.8b, 22742bd): ConnectionPool's
action-level workerOps gate (tool.vault.search, …) never matched the
operation-level method strings the pool emits (tool.vault, tool.edit, …),
so processWithWorker was unreachable. Independently, the worker built to
dist/workers/workers/semantic-worker.js while WorkerManager loaded
dist/workers/semantic-worker.js — load would have failed anyway.
Verified dead against current main; no CPU-starvation field reports.
Fixing forward was rejected: it would ship an untested concurrency path
that widens the #139 parallel-edit race via a new TOCTOU window for an
unproven perf need (see #197 analysis).
- delete src/workers/semantic-worker.ts, src/utils/worker-manager.ts,
build-worker.js; drop build:worker + the build-worker.js build step
- strip worker wiring from ConnectionPool (retained as a main-thread
bounded queue — the only behaviour that ever ran) and the dead
prepareWorkerContext/workerScript from mcp-server.ts
- ADR-105 records the removal as a partial reversal of ADR-104; ADR-104
stays Accepted (SSE-route deconfliction half, shipped #196, still
governs) with a banner pointing to ADR-105
Behaviour-preserving by construction: the deleted path never ran.
make check green (build + lint 0 errors + 225/225 tests).
#192 replaced the Claude Code copy/paste command with a hand-edit-this-JSON
blob plus a warning against `claude mcp add --header`. That was a real UX
regression, and the security premise was wrong for THIS transport:
- The argv / macOS-unified-log exposure applies to **stdio** transports
(a spawned `npx mcp-remote --header ...` child), NOT native **HTTP**
transport — there is no spawned child carrying the header in argv. #143's
threat model was conflated with the mcp-remote setup it was also removing.
- Cleartext-at-rest in ~/.claude.json / .mcp.json is identical whether the
entry is added via the CLI or hand-edited, so "edit the file instead"
bought no at-rest improvement either.
(Confirmed against current Claude Code docs via claude-code-guide.)
Restores the single `claude mcp add --transport http ...` command as the
Claude Code path in the Settings UI, README, and the issue-32 template.
Removes the inaccurate warning and its now-dead `.mcp-security-warning` CSS
and the eslint-disable it required.
Kept from #192 (those parts were legit): native HTTP transport over the
deprecated mcp-remote, and the self-signed-cert / NODE_EXTRA_CA_CERTS
trust docs.
ADR-104's context has a passing line calling the `--header` form
deprecated; left as-is (accepted ADRs are immutable records of what we
believed then) — this commit is the corrective record.
Reimplemented from PR #126 (djsplice, who also reported #125), reduced to
the parts that are verified-clean. The worker-offload half of #126 is split
out to a tracking issue (inert as wired + correctness divergences — see PR
discussion / the follow-up issue).
SSE route deconfliction
In @modelcontextprotocol/sdk@1.29.0 (the pinned version), `GET /mcp` is
the standalone server->client SSE stream for server-initiated messages —
verified in webStandardStreamableHttp.js:handleGetRequest (opens
`_GET_stream`, Content-Type text/event-stream, gated on
Accept: text/event-stream + session + protocol version). A debug route
returning application/json on `GET /mcp` shadowed it, so any client that
opens the standalone stream (mcp-remote-class bridges — #128's logs show
one in use here) received JSON instead of an SSE stream and treated it as
failed. Moved debug to `GET /mcp-info`; `GET /mcp` and `POST /mcp` both
reach handleMCPRequest. Unlike #126's `app.all('/mcp')`, GET/POST are
registered individually so the existing explicit `app.delete('/mcp')`
session-close handler is preserved.
Scope note: this restores the server-initiated-notification channel (a
real latent defect on its own). It is NOT claimed to fix#128 — per the
#190 investigation that is a separate SDK-1.29 compat-init problem — and
#125's "SSE reconnection loop" is the reporter's un-reproduced diagnosis.
Two-row Levenshtein
fuzzy-match.ts rewritten from a full (m+1)x(n+1) matrix to a two-row
formulation (O(n) memory, no per-line array-of-arrays allocation) plus
length heuristics and a 0.95 early-exit. Pure CPU/memory win on the
main-thread edit.window path; behaviour-preserving. Adds the first
fuzzy-match tests (classic Levenshtein distances, symmetry,
heuristic-preservation).
build.yml: PR-status-comment step set continue-on-error so it cannot red an
otherwise-green build on fork/limited-token PRs.
Co-authored-by: djsplice <barrows.jeff@gmail.com>
Code review (PR #193) caught that the inline branch returned
`sourceFiles: paths` (caller input order) while `content` is built from
the local sourceFiles array *after* sortFiles() mutates it in place. With
sortBy set — exactly the sorted multi-file retrieval case this feature
targets — a consumer mapping content sections back to files got the wrong
order. Return the post-sort order instead.
Adds a router-level regression test (the gap the reviewer flagged: the
formatter-only tests could not have caught this).
Co-authored-by: Earl Plak <5597016+laplaque@users.noreply.github.com>
`vault combine` without a `destination` now returns the combined content
inline in the response instead of erroring. Read-only consumers can use
combine for multi-file retrieval with no side effects.
- router.ts: no-destination path returns { inline, content, ... }; the
destination-exists guard is skipped when there is no destination.
- formatters/vault.ts: FileCombineResponse gains optional destination +
inline/content; formatFileCombine renders an inline block.
- semantic-tools.ts: combine counts as a write-op (blocked in read-only
mode) only when a destination is given; param description updated.
- tests: formatter inline/destination/fallback coverage.
Reimplemented from PR #146 against current `main` (code only); supersedes
and closes out #146. Dropped the original PR's
`path: destination || '_inline_'` validator workaround — the batch.combine
validators (BatchLimitValidator, PathArrayValidator) never inspect `path`,
so the hack was unnecessary. The README/troubleshooting cert-trust docs
that #146 also carried are covered by the #143 reimplementation instead, to
avoid duplicate divergent copies.
Co-authored-by: Earl Plak <5597016+laplaque@users.noreply.github.com>
`claude mcp add --header "Authorization: Bearer <token>"` defeats
secret-at-rest protection: the CLI resolves and echoes the header value to
stdout (captured by any parent process, incl. AI agents) and on macOS the
spawned MCP child argv is written to the unified log. Editing the MCP config
file directly avoids both vectors.
- Settings UI: new shared `renderClaudeCodeConnection()` renders a
ready-to-paste JSON config + a security warning for the authenticated
path, and the safe plain CLI command only when auth is disabled. Used by
both the initial render and the live-refresh handler so they cannot drift
(the previous two copies had already diverged).
- README / SECURITY.md / troubleshooting.md / issue-32 response template:
drop `mcp-remote` + `NODE_TLS_REJECT_UNAUTHORIZED=0`, document native HTTP
transport and proper self-signed-cert trust (macOS Keychain +
NODE_EXTRA_CA_CERTS for Bun-based runtimes), add the `--header` warning.
- styles.css: `.mcp-security-warning`.
Reimplemented from PR #143 against current `main` (the settings UI had been
restructured since the PR was opened); supersedes and closes out #143.
ADR-100 left untouched — accepted ADRs are immutable records; the `--header`
deprecation is recorded in ADR-104's context instead.
Co-authored-by: Earl Plak <5597016+laplaque@users.noreply.github.com>
PR2 of 2 for #180 (ADR-201). Closes the arbitrary-code-execution vector
reachable through a synced/shared `.base` file.
- Drop `new Function('context', 'with(context){return ${expr}}')` (and its
no-implied-eval eslint-disable) from expression-evaluator.ts.
- Parse with expression-eval@5.0.1 (jsep grammar: no eval/Function/new, no
global scope; member denylist built in). Pinned exact; jsep 0.3.5 is its
audited transitive parser. Both >7d old; npm audit 0 vulns.
- Defense-in-depth AST pre-walk (assertNoForbiddenAccess): rejects
constructor/__proto__/prototype member access (Identifier AND computed
["constructor"] forms) and ThisExpression, independent of the library's
internal list — ADR-201's explicit, tested no-globals property lives in
our code, not a transitive dep.
- createEvalContext and all helpers untouched → behavioural parity.
Differential corpus (tests/bases-expression-evaluator.test.ts, from PR1):
57 EVAL_CASES still green = expression-eval == old new Function for every
realistic Bases construct; the formerly-executing escapes (2/42/99 under
new Function) now fail closed; SECURITY set never reaches a function, the
real global, or a value.
No behavioural change for legitimate `.base` expressions. Breaking only by
design: the RCE vectors, `this`, and prototype-chain access are now
refused. Clears the Obsidian "Dynamic Code Execution" finding.
209/209 tests; build + lint clean. Refs #180, ADR-201
Code review found the date-safety doc cited the wrong line (date() global,
not the real reconciliation). Correct it: (1) Obsidian metadata cache is the
primary frontmatter source, this parser is only a fallback; (2) when the
fallback runs, expression-evaluator auto-coerces date-like keys via
new Date(); (3) .base docs have no date scalars.
Surface the masked divergences instead of hiding them behind toBeFalsy:
- empty doc: yaml→null vs js-yaml→undefined — asserted concretely, proven
neutralized by parseFrontmatter's typeof/!==null guard.
- merge keys: js-yaml resolves <<:*anchor, yaml keeps literal — asserted as
a known, accepted boundary (.base/frontmatter never use merge keys).
- input anchors/aliases: confirmed both resolve structurally.
Documented all in yaml-bridge.ts. 149/149 green.
Closes#178. updateStatusBar() previously remove()'d and re-addStatusBarItem()'d
on every call. It fires multiple times during async startup (port-conflict
paths, server-start callbacks, settings); concurrent calls could each add an
element while only the last was tracked, orphaning a transient 'Mcp: error'
element that persisted until the next Obsidian reload.
Create the element exactly once and mutate it thereafter (setText + class
swap). showConnectionStatus=false now hides the single element via the
existing mcp-hidden utility instead of leaving/removing DOM. Structurally
eliminates duplicate/orphaned indicators regardless of call ordering.
Closes#174. Obsidian source-code review flagged js-yaml (unmaintained) for
replacement. Rewire bases-api.ts's 5 call sites onto yaml-bridge; drop
js-yaml + @types/js-yaml. yaml pinned to 2.8.4 (published 2026-05-02, clears
the project's 7-day supply-chain hold; 2.9.0 was 5 days old). Existing
overrides.yaml reconciled to $yaml so the direct dep governs transitive
resolution. Behaviour-equivalence proven by the differential corpus.
Introduces src/utils/yaml-bridge.ts as the single YAML parse/serialize seam
for the Bases subsystem, and a reusable .base corpus + differential tests
that run every fixture through both js-yaml (still resolvable transitively as
the behavioural oracle) and the new yaml-backed bridge, asserting
equivalence. The one intentional difference — js-yaml coerced bare/ISO dates
to Date, yaml keeps strings — is asserted explicitly and proven downstream-
safe (expression-evaluator already does new Date(value) on non-Date input).
Corpus is structured for ADR-201/#180 reuse.
Closes#163. The literal only existed in DEFAULT_SETTINGS and was passed to
the inbound https.createServer, which never sets requestCert — so Node ignored
it. certificate-manager already defaults the effective value to true via
`!== false`, so removing the default is behavior-preserving and removes the
exact token Obsidian's scorecard pattern-matches as 'disables SSL verification'.
Adds tests/security/tls-cert-verification.test.ts as a regression guard:
fails if the flagged literal returns or the secure default flips.
