After develop merged, mcp.service.ts calls decideBasicGate from mcp-auth.helpers.
The gate spec mocked the whole module returning only FailedLoginLimiter, so the
merged code crashed with 'decideBasicGate is not a function' (7/7 failing).
Spread jest.requireActual('./mcp-auth.helpers') so the real helpers are kept and
the gate exercises real logic; keep only FailedLoginLimiter stubbed so its
constructor runs without a real sweep timer.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
isExternalHttpUrl treated any http(s):// URL as external, so an absolute link
back to the app's own host (e.g. https://self/p/{uuid}, /settings/members)
emitted by the assistant stayed clickable on the anonymous share, leaking
internal UUIDs/structure and pointing at auth-gated routes. Classify a link as
external only when its host differs from window.location.host; unparseable URLs
are treated as internal (fail-closed). Tests cover own-origin absolute (flag
on -> inert), external host (kept with safe rel/target), dangerous schemes, and
no behavior change for the internal chat (flag off).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add .github/workflows/test.yml (pnpm + Node 22): on pull_request and push
to develop it installs, builds @docmost/editor-ext and runs `pnpm -r test`
across all packages (server Jest, client Vitest, editor-ext Vitest,
packages/mcp node:test). So tests now run automatically in CI, not just
on demand.
To make the run green, quarantine the 16 pre-existing stock NestJS
`should be defined` scaffold specs via jest `testPathIgnorePatterns` —
they never compiled (missing DI providers / lib0 ESM) and assert nothing
useful. Tracked for a proper fix/removal in issue #56. Verified each
pattern drops only its scaffold (46 of 62 suites still collected) and the
full `pnpm -r test` is green: server 587, client 185, editor-ext 56,
mcp 247.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The public-share widget was a separate minimal impl: plain-text answer, static
'Thinking…', no markdown, no tool-cards. Now it renders through the internal
chat's debugged presentational layer (MessageList/MessageItem/TypingIndicator/
ToolCallCard), so a share gets the same incremental streaming, animated typing
indicator, markdown, and tool-call cards. The share keeps its anonymous
transport (useChat + DefaultChatTransport '/api/shares/ai/stream',
credentials:'omit').
The shared components were already prop-driven (UIMessage[] + isStreaming) with
no transport/auth coupling; made the new props additive optionals (emptyState,
showCitations, neutralizeInternalLinks) all defaulting to current behavior, so
the internal chat is unchanged.
Security (review-caught): rendering assistant markdown on the ANONYMOUS share
made internal links (/p/{id}, /settings/...) clickable, which the old plain-text
render didn't. renderChatMarkdown gains neutralizeInternalLinks (true only on
the share): a one-shot DOMPurify afterSanitizeAttributes hook (added/removed by
reference around a single sanitize) strips href from internal/relative/non-http(s)
links (rendered inert) and keeps external http(s) links with
rel=noopener noreferrer nofollow target=_blank. Tests cover both the link
neutralization and the absence of any global-hook leak into internal renders.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add ~330 tests across server (Jest), client (Vitest), editor-ext (Vitest)
and packages/mcp (node:test) for the gitmost features added since
053a9c0d: AI chat, AI agent roles, public-share assistant, MCP per-user
auth, HTML embed, page templates/embed, realtime tree, tree
expand/collapse, and the AI-settings UI.
Test-tooling fixes (prerequisite, were silently hiding coverage):
- Repair 3 page-template specs broken by the 11-arg TransclusionService
constructor; they never compiled, so template access-control / content
-leak / unsync-strip coverage was fictitious.
- Build @docmost/editor-ext before server tests via a `pretest` hook;
the stale dist omitted the new HtmlEmbed/PageEmbed exports (TS2305).
- Let jest resolve the .tsx email templates: add `tsx` to
moduleFileExtensions and widen the ts-jest transform to (t|j)sx?.
Behaviour-preserving "extract pure core" refactors that the tests drive:
- server: resolveShareAssistantRequest + uiMessageTextLength
(public-share controller), decideBasicGate + mapAuthResultToResponse
(mcp), buildErrorAssistantRecord (ai-chat), jsonbObject export (roles).
- client: render-raw-html + shouldExecute/canEdit, decide-embed-state,
page-embed picker utils, tree-socket reducers, open/close branch maps,
isEndpointConfigured/resolveKeyField; buildTreeWithChildren now treats
a permission-trimmed orphan as a root instead of crashing.
Deferred (need a test DB or HTTP harness, documented in the specs):
repo-level Postgres integration tests and the public-share XFF E2E.
Pre-existing DI/lib0-ESM suite failures are untouched and out of scope.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Post-merge hardening from the #13 security review:
- isInitializeRequestBody now delegates to the SDK isInitializeRequest (same
predicate as packages/mcp/http.ts), so a bare {method:'initialize'} with no
id/params no longer triggers the side-effecting login() (audit-spam /
user_sessions growth) before http.ts 400s it.
- Bind the Bearer path to the instance workspace: verifyBearerAccess rejects a
token whose payload.workspaceId != the instance workspace (resolved via
workspaceRepo.findFirst, consistent with the Basic path); optional param so
it's a no-op when unset.
- Close the user-enumeration timing oracle in verifyUserCredentials: the
missing/disabled branch now runs a bcrypt compare against a module-level dummy
hash whose cost (12) matches production saltRounds, so both paths take one
equal-cost bcrypt compare; the exact CREDENTIALS_MISMATCH_MESSAGE is preserved.
- Document the trusted-proxy requirement for the spoofable per-IP brute-force
limiter in .env.example (trustProxy is on; deploy behind a trusted proxy).
- Add real-execution coverage for enforceBasicLoginGate (SSO enforced / EE-MFA
bundled vs not / user-MFA / workspace-enforced-MFA) instead of stubbing the gate.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Resolve conflicts at shared registration points by unioning both features
(footnotes + the already-merged html-embed / page-embed work):
- slash-menu/menu-items.ts, editor extensions.ts: keep both imports + configures
- collaboration.util.ts: register footnote nodes and pageEmbed
- editor-ext marked.utils.ts: register footnote + html-embed markdown extensions
- editor-ext package.json/tsconfig.json/vitest.config.ts: union of test config
(jsdom env for footnote DOM tests + combined test/spec include glob)
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The current page id was only injected as text in the system prompt, which a
proxy (CLIProxyAPI) can rewrite/truncate, so the agent could lose track of 'this
page'. Add a getCurrentPage tool the model can call to read the open page (id +
title) from the server-side request context (forUser now takes openedPage,
threaded from body.openPage — the same value used for the system prompt). The
inline system-prompt line is kept as belt-and-suspenders. Reads/writes still go
through the CASL-enforced page tools by id, so this is strictly not worse than
the existing prompt hint — just delivered over a channel the proxy can't mangle.
User-approved on the issue. Completes #43 together with the hardness-1 fix.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
AiChatWindow derived the open page via useParams(), but it's mounted in a
pathless parent layout route where :pageSlug isn't matched, so useParams()
returned {} and openPage was ALWAYS null — the agent never received current-page
context (couldn't resolve 'this page'/'the current page'). Derive pageSlug from
useMatch('/s/:spaceSlug/p/:pageSlug') against the full pathname instead, so it
resolves regardless of where the component sits in the route tree. No-match
behavior is unchanged (undefined -> query disabled -> openPage null).
Addresses Hardness #1 of #43. Hardness #2 (proxy resilience: a get_current_page
tool / hidden user-message context so identity doesn't depend on the system
prompt surviving CLIProxyAPI) remains open.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The footnote definition number ('1.') sat ~19px from its text because two
spacings stacked: the 1.5em (24px) marker min-width box (wider than the ~15px
glyph) plus a 10px flex gap. Reduce the flex gap to 0.4em (about one space) and
right-align the number within the 1.5em column so the period sits next to the
text and multi-digit numbers (10, 11, ...) stay aligned. Reads like '1. text'.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
PageTemplateController (added on this branch) guards its lookup/toggle routes
with UserThrottlerGuard, which depends on the throttler options provided by
ThrottleModule. CollaborationModule -> TransclusionModule registers that
controller, and the collab server bootstraps CollabAppModule, which did not
import ThrottleModule. The API server's AppModule does, so :3000 booted, but
the collab server (:3001) crashed at startup with
'Nest can't resolve dependencies of the UserThrottlerGuard ... THROTTLER:MODULE_OPTIONS'.
Without collab the editor can't sync, so live editing was broken on this branch.
Import ThrottleModule into CollabAppModule, mirroring AppModule, so the guard
resolves in the collab process too.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Anonymous public-share AI assistant:
- Add a workspace setting `publicShareAssistantRoleId` so an admin can pick which
agent role (identity/persona) the anonymous assistant adopts. The role's
instructions REPLACE the built-in persona while the immutable safety framework
is still always appended; the role's optional model override takes precedence
over the cheap publicShareChatModel. Resolved server-authoritatively
(workspace-scoped, soft-delete aware; disabled/missing roles fall back to the
built-in persona, so the tool scope remains the real security boundary).
- Plumb the field through the update DTO, ai-settings service, the workspace.repo
ALLOWED whitelist, resolve()/getMasked(), stream-time role resolution and the
prompt/model, plus the settings UI: a new "Assistant identity" Select listing
enabled roles (and surfacing a saved-but-disabled role explicitly).
Public-share branding / floating icon:
- Fix the AI assistant FAB overlapping the "Powered by ..." button (both were
Affixed bottom-right): stack the FAB above the bottom-right branding.
- Rename "Powered by Docmost" -> "Powered by Gitmost" and point the link at the
gitmost repo.
Tests: extend public-share-chat.spec (role persona replacement still appends the
safety framework, resolveShareRole edge cases, model-override precedence).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The html-embed feature toggle was enforced CLIENT-side in the NodeView (reads
settings.htmlEmbed from the logged-in workspace), so an anonymous public-share
viewer — who has no workspace context — always saw it as OFF and got a
placeholder instead of the executing embed. That broke the whole point (a
tracker must run for anonymous visitors).
Make it server-authoritative:
- share.service prepareContentForShare (the single path both share-content
flows use) strips htmlEmbed from served content when the workspace toggle is
OFF; both callers (updatePublicAttachments host page + lookupTransclusionForShare)
resolve the toggle once and pass it. Fail-closed: missing workspace -> OFF ->
stripped.
- NodeView executes whatever it was served in read-only/share mode
(shouldExecute = !editor.isEditable || htmlEmbedEnabled); the disabled
placeholder now only shows in the editable editor when OFF.
Net: anonymous share + toggle ON -> server serves the (admin-authored) embed ->
it executes for everyone; toggle OFF -> stripped server-side from every
share-content path (true kill switch); a non-admin embed can never be served
(save-path strip). No XSS regression in the editable editor.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- make addTreeNode receivers idempotent (invalidateOnCreatePage guard +
buildTree dedup) so the author's self-echo no longer duplicates the node
- broadcast realtime tree updates for bulk copy/duplicate and import via a
root refetch: PAGE_CREATED now carries spaceId and the WS listener falls
back to refetchRootTreeNodeEvent when no per-node snapshot is present
- remove the now-dead client-relay inbound path (isTreeEvent/handleTreeEvent)
that remained a stale-restriction-cache attack surface
- honest string|null cast for a root move's parent id
- add tests: buildTree dedup; onPageCreated per-node vs refetch branching
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The admin-only raw HTML/JS embed is a deliberate stored-XSS surface, so gate the
whole feature behind a workspace toggle that is OFF by default; it only works
when a workspace admin explicitly enables it.
- settings.htmlEmbed (boolean, default false) + workspace-update field htmlEmbed,
persisted via WorkspaceRepo.updateSetting with an audit diff. Flipping it is
admin-only (same Manage Settings CASL as other workspace toggles).
- New gate htmlEmbedAllowed(featureEnabled, role) = featureEnabled && admin/owner.
All 7 server write paths (create, duplicate, collab onStoreDocument, REST/MCP/AI
updatePageContent, single + zip import, transclusion unsync) now read the
workspace's settings.htmlEmbed and strip unless (toggle ON AND admin). OFF
(default, or a failed/empty workspace lookup) strips htmlEmbed for EVERYONE
including admins -> existing embeds are cleaned up on next save, none persist.
- Client (defense-in-depth): the /html slash item is hidden unless toggle ON +
admin; the NodeView executes nothing and shows a 'disabled in this workspace'
placeholder when OFF; an admin Switch in Workspace Settings -> General with a
description of the behavior.
- docs/html-embed-admin.md documents the toggle + admin-only + fail-closed
coedit (a non-admin save strips an admin's embed) + execution semantics.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Resolve conflicts with the independently-merged ai-agent-roles feature:
- ai-chat.module.ts: keep BOTH AiAgentRolesModule and the public-share
wiring (Share/Search modules, PublicShareChatController, services).
- ai.service.ts: take develop's getChatModel ChatModelOverride superset,
which already covers the public-share model-id-only override.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Follow-up fixes on the agent-roles feature:
- ai.service: a cross-driver override to the ollama driver (when the
workspace driver is not ollama) now fails with an explicit 503 instead
of silently reusing the workspace base URL, which belongs to a different
provider. Same-driver ollama and openai/gemini overrides are unchanged.
- migration: add a partial unique index on (workspace_id, name) WHERE
deleted_at IS NULL so role names are unique per workspace without
soft-deleted rows blocking re-creation; map Postgres 23505 to a 409
ConflictException on create/update.
- dto: validate the role id as @IsUUID instead of @IsString.
- roles list: do not expose instructions/modelConfig to non-admin members.
The list endpoint now returns a picker view (id/name/emoji/description/
enabled) to members and the full view only to admins (same gate as the
CRUD endpoints). Client IAiRole fields made optional accordingly.
Adds tests for the cross-driver-ollama throw, the 23505->409 mapping, and
the non-admin picker-view security invariant.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Integrate the already-merged step-limit work from develop. Only conflict was
ai-chat.service.spec.ts: both sides appended a describe block and edited the
import line. Resolved as a union — keep compactToolOutput + the assistantParts/
serializeSteps/rowToUiMessage suites (this branch) AND the prepareAgentStep
suite (develop), importing all symbols from ai-chat.service.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Adds explicit isIpAllowed cases for the CGNAT, ULA (fd00::/8) and IPv4-mapped
IPv6 loopback (::ffff:127.0.0.1) sample addresses from the parallel
safety-coverage branch. The mapped-loopback case is genuinely new (the existing
table only covered the mapped *private* variant); CGNAT and ULA ranges were
already covered with other samples and are kept here as explicit regression
guards for these specific addresses.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Behaviour change (split out of the test commit per review, and now covered).
Both the stream onError log line and the error text streamed to the client were
formatted by separate inline blocks that only emitted "<status>: <message>".
Route both through the shared describeProviderError() so formatting stays in one
place.
BEHAVIOUR CHANGE: describeProviderError additionally appends a single-line,
300-char-truncated snippet of the provider responseBody/text. So the log line
AND the user-facing stream error now include that snippet (e.g. the HTML error
page from a misconfigured endpoint), which previously neither did. This is
intentional — it makes a misconfigured external endpoint diagnosable — and is
safe: the API key travels in the Authorization header and is never echoed in
the response body (see the util's docstring). A `fallback` param is added so
each call site keeps its own default ('AI stream error' for the stream).
Adds ai-error.util.spec.ts covering the formatter, including the appended /
truncated body snippet, so this behaviour is no longer untested.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Behaviour change (split out of the test commit per review).
In AI SDK v6 the useChat `onFinish` callback does NOT fire when the stream
errors. A brand-new chat whose very first turn fails would therefore never run
the post-turn path: the chat list was not invalidated and the client never
adopted the server-created chat id — so the failed chat only appeared in
history after a manual refresh (the server already creates the row and stores
the error message). Running the same `onTurnFinished()` handler on `onError`
makes the failed chat show up immediately. The error itself is still surfaced
to the user via the existing `error` state.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Unit tests for the safety-critical paths: crypto secret-box (round-trip,
tamper detection, wrong key), the SSRF guard (blocked ranges + DNS-rebinding),
the ai-chat tools service, the page-embedding repo, and the
assistant-parts/serialization helpers. Those server helpers (assistantParts,
rowToUiMessage, serializeSteps) are exported ONLY for the tests — no runtime
change.
Also: keyboard a11y on the chat history header and conversation rows
(role/tabIndex/Enter+Space), and DRY refactors that move shared logic into one
place (isToolPart -> tool-parts util; buildInitialValues in the MCP form).
The behaviour-changing edits that previously rode along in this commit are
split out into the following two commits, per review.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Port two refinements from the GLM variant onto the Claude base:
- prepareAgentStep: add a comment note that AI SDK v7 renames the per-step
`system` field to `instructions` (v6 ^6.0.134 still uses `system`), so it
gets updated correctly on the next SDK bump.
- ai-chat.service.spec: add an explicit off-by-one boundary test for
prepareAgentStep, expressed via MAX_AGENT_STEPS instead of a hardcoded 18/19
so it tracks the constant if the cap changes.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Expand all kept the menu open (closeMenuOnClick={false}) while Collapse all
closed it. Make both close on click for consistent behavior, and drop the
now-pointless in-menu isExpanding loading state.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- extract collectAllIds / collectBranchIds into tree/utils and use them in
space-tree.tsx instead of inline closures
- drop the duplicate SidebarPageTreeDto, reuse the existing SidebarPageDto
for the /pages/tree endpoint
- type the getSpaceTree client call as api.post<{ items: IPage[] }>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Make the denser page-tree layout opt-in instead of hardcoded, so row
density can be toggled per deployment via the COMPACT_PAGE_TREE runtime
config flag.
- doc-tree: extract ROW_HEIGHT_STANDARD (32) / ROW_HEIGHT_COMPACT (26);
default the virtualizer row stride to STANDARD density.
- client: isCompactPageTreeEnabled() in lib/config (reads
COMPACT_PAGE_TREE, default true); used by space-tree and shared-tree
to choose the row height.
- server: EnvironmentService.isCompactPageTreeEnabled() and expose
COMPACT_PAGE_TREE through the window runtime config (static.module).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Release-cycle audit flagged WsService.invalidateSpaceRestrictionCache and
WsTreeService.notifyPageRestricted/notifyPermissionGranted as never-wired dead
code. Investigation: this community fork has NO page-permission grant/revoke/
restrict mutation site (the page-access repo mutators have zero callers — that
flow is EE / not yet built), so there is nothing to wire them into.
- Keep invalidateSpaceRestrictionCache (it's the one-line correctness primitive
the future permission-mutation path must call to avoid the 30s stale-cache
window) but document exactly that + add a test that it deletes only the
space-scoped cache key.
- Remove the untested, security-adjacent dead methods notifyPageRestricted /
notifyPermissionGranted and their now-orphaned helpers emitToUsers /
emitToSpaceExceptUsers (no remaining references; build confirms). A future
permission-change realtime feature can reintroduce them wired + tested.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Release-cycle review found two hardening gaps:
- The sync plugin deleted+rebuilt the WHOLE footnotesList on any reorder/orphan,
replacing every definition's Yjs subtree -> a collaborator typing in a
definition could lose in-flight characters on merge. Rework to targeted,
minimal mutations: attr-only setNodeMarkup for collision re-ids, delete only
genuine orphans, insert only genuinely-missing definitions (at the list end,
not shifting existing subtrees), and consolidate multiple lists only in the
abnormal paste/merge case. An unchanged (correct id, referenced) definition is
left completely untouched. Numbering is decoration-only, so physical list order
may drift after a reorder (accepted) while displayed numbers stay correct.
Invariants preserved (reviewed + tested): one SYNC_META transaction, null when
canonical (terminates), deterministic deriveFootnoteId, remote-skip -> no
re-introduced freeze or divergence.
- computeFootnoteNumbers ran per-NodeView-render (O(n^2)/keystroke in big docs).
The numbering plugin now caches the number map in its state (computed once per
docChanged); NodeViews read it O(1) via getFootnoteNumber.
Tests: no-rebuild-on-reorder asserts unchanged definition node subtrees are
identity-preserved; isRemoteTransaction skip; enableSync:false read-only; cache
correctness. Browser re-smoke: insert (no freeze), number, persist across reload,
cascade delete all pass.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Release-cycle review: POST /pages/template/lookup had only JwtAuthGuard and the
embed depth cap was client-only, so a scripted client could drive heavy
full-content fan-out (access control holds per-id, but a cost/DoS gap). And
page_template_references rows were written for any sourcePageId with no
workspace check at sync time (no leak today since lookup re-checks access, but
the graph could accumulate cross-space rows).
- Apply the standard per-user throttler (PAGE_TEMPLATE_THROTTLER, 30/min) to
/pages/template/lookup and /pages/toggle-template (mirrors ai-chat); auth +
the toggle's validateCanEdit CASL are unchanged.
- syncPageTemplateReferences / insertTemplateReferencesForPages now restrict
inserts to in-workspace source ids (filterInWorkspaceSourceIds, workspace +
not-deleted scoped, trx-aware) and still delete stale out-of-workspace rows
(self-heal). SECURITY comment: the ref table is NOT access-filtered; every
consumer must permission-filter at read time (as lookupTemplate does).
- Tests: lookup access exercises the REAL filterViewerAccessiblePageIds
(no_access / cross-workspace excluded / accessible+comment-stripped / <=50);
toggle controller CASL (cannot-edit -> Forbidden, flag not flipped); ref-sync
excludes cross-workspace and keeps in-workspace.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Release-cycle review: the per-workspace cost cap was fixed-window + per-instance
(allowed ~2x at a window boundary and K*cap behind K instances) on an anonymous
endpoint that spends the owner's provider budget. Rewrite it as a sliding-window,
CLUSTER-WIDE Redis limiter: one atomic Lua EVAL does ZREMRANGEBYSCORE (age out)
-> ZCARD -> ZADD with PEXPIRE, so concurrent instances share one budget and the
true rate over any trailing window is <= cap. Fails OPEN on a Redis error (logged)
— it's a cost backstop, not access control (the funnel gates + per-IP throttle
still apply), so a Redis blip must not take the assistant offline. Per-IP @Throttle
kept; commented that it needs an XFF-rewriting trusted proxy to be meaningful.
Extract deriveShareAccess (resolvedShareId===requestedShareId + isSharingAllowed +
!restricted, equality-only, never widening) and filterShareTranscript into pure
helpers, and add tests: limiter sliding-window + boundary-burst + fail-open;
access derivation; and red-team boundary locks (cross-share/cross-workspace swap
rejected, forged shareId can't widen tool scope, transcript injection filtered).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Release-cycle test audit: the strip boundary was tested only via a stand-in
helper re-implemented in the spec, so a deleted/misplaced guard kept CI green
(the missing create() guard was proof). Replace it with tests against real code:
- persistence.extension.onStoreDocument: real ydoc from a rich doc (columns/
table/mention/htmlEmbed) -> non-admin strip removes only htmlEmbed, every other
node preserved (data-loss guard); admin keeps; empty fragment no-throw.
- collaboration.handler.updatePageContent: real path, user?.role gate, decoded
ydoc embed-free for non-admin, kept for admin.
- transclusion unsync: member stripped, admin preserved.
- editor-ext gains a vitest setup (was zero tests) + a markdown round-trip:
the <!--html-embed:BASE64--> marker -> htmlEmbed node with decoded source, and
hasHtmlEmbedNode matches it — pinning the marked/turndown shape the import
strip relies on. tsconfig now excludes specs from the shipped dist.
- Fail-closed identity: source-pinned contracts that the gate keys on
fileTask.creatorId (zip) / request userId (single) / callerRole (create) /
authUser.role (duplicate), and missing-user -> strip (services can't load under
jest's ESM graph; helpers replay the exact predicate).
Adds the verified-safe ^src/ jest moduleNameMapper (identical fail set).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Release-cycle test audit: the /mcp auth's constant-time token guard, IP keying,
ACCESS-type pinning, and brute-force message coupling were untested. Extract
behavior-preserving pure helpers so they're testable and cover them:
- sharedTokenMatches: length-mismatch early-returns before timingSafeEqual
(which throws on unequal lengths); equal-length uses timingSafeEqual; array
header -> first element; non-string -> false.
- clientIp: req.ip > socket > first XFF hop > 'unknown' (limiter keying).
- bindAccessJwtVerifier: verifyJwt pinned to JwtType.ACCESS (rejects REFRESH).
- CREDENTIALS_MISMATCH_MESSAGE single source of truth shared by
verifyUserCredentials and isCredentialsFailure, so a reworded auth error can't
silently disable the /mcp brute-force counter.
- verifyUserCredentials no-side-effect contract asserted via a TS-AST spec
(AuthService can't load under jest): its body has no createSessionAndToken/
audit/updateLastLogin while login() has all three.
Extractions are behavior-preserving (reviewed); class delegates to the helpers,
dead code + unused imports removed.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Release-cycle test audit found the role feature's security-critical paths
untested. Adds real unit tests (against the actual functions):
- resolveRoleForRequest invariants: role comes from chat.roleId not body.roleId
(no per-turn swap), lookup scoped to workspace.id, disabled/soft-deleted role
-> null, new-chat uses body.roleId, stale chatId falls back.
- CASL admin gate: non-admin create/update/delete -> Forbidden and service not
called; admin delegates with workspace.id; list() is member-reachable.
- roleModelOverride: unknown driver dropped (never reaches getChatModel's
throwing default), valid override passes through, blanks ignored.
- getChatModel override success path (cross-driver fetch + decrypt; chatModel-
only reuse), and service update/remove cross-workspace 'not found' guards +
modelConfig tri-state.
Tiny fix: findByCreator badge left-join now also requires enabled=true, so a
disabled role (downgraded to universal by resolveRoleForRequest) no longer shows
a misleading chat-list badge.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Release-cycle review: update() re-read the role via findById (filters
deleted_at IS NULL) and passed it straight to toView(updated as AiAgentRole).
A concurrent soft-delete between the UPDATE and the re-fetch makes findById
return undefined, and toView(undefined) dereferences row.id -> opaque 500. Add
the same 'Role not found' guard remove() already uses.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
