Skip to content

feat(shared): add a browser-safe @finos/calm-shared/browser entry point - #3022

Merged
rocketstack-matt merged 25 commits into
finos:mainfrom
rocketstack-matt:feat/2880-shared-browser-entry
Aug 26, 2026
Merged

feat(shared): add a browser-safe @finos/calm-shared/browser entry point#3022
rocketstack-matt merged 25 commits into
finos:mainfrom
rocketstack-matt:feat/2880-shared-browser-entry

Conversation

@rocketstack-matt

Copy link
Copy Markdown
Member

Description

Adds a browser-safe entry point, @finos/calm-shared/browser, exposing the pure CALM engine — validate (JSON Schema + Spectral rules), generate, diff/timeline, SchemaDirectory, the document-loader seam and a manifest of which CLI commands are available in browsers — with no Node-only module evaluated in its import graph. The existing root entry keeps identical behaviour for the CLI and calm-server.

This is the prerequisite for the in-browser learning lab (#2879, Phase B): until now the lab's calm validate was a hand-wired Ajv re-implementation because importing any real command implementation dragged in fs, winston, playwright and friends at load time.

Closes #2880.

What changed

Seams (behaviour-preserving for Node callers)

  • Logger — winston construction moves to logger.node.ts; the root barrel registers it at load via registerNodeLoggerFactory. initLogger keeps its signature and Node output is byte-identical; the browser graph only ever sees loglevel.
  • Document loadersdocument-loader.ts is now interface-only; the fs-backed buildDocumentLoader lives in node-document-loader.ts (same name re-exported from the root). New InMemoryDocumentLoader (caller-supplied documentId → object map) and buildBrowserDocumentLoader (in-memory → CalmHub → direct URL). DirectUrlDocumentLoader's net.isIP becomes a browser-safe ipLiteralVersion with the same contract (incl. IPv4-mapped tails and IPv6 zone ids), so the SSRF decision is unchanged.
  • Validation output — formatters move to a small registry (format-output.ts); json/pretty are built in, junit is registered only from the root entry. pretty-output.ts no longer imports path or reads process unguarded.
  • Pure coresvalidate-core.ts (validate), generate-core.ts (generate) and diff-core.ts (diffDocuments, diffTimeline); validate.ts (which keeps exitBasedOffOfValidationOutcome), runGenerate/runDiff/runTimelineDiff are thin Node wrappers with unchanged signatures and messages.
  • Resolver modules now import type CalmReferenceResolver, severing the static fs/axios link from the validate path; hub/resource-types.ts is a leaf so document-id-utils no longer pulls the axios Hub client into browser bundles.

Package surface

  • exports map: "."dist/index.js, "./browser"dist/browser.js (types alongside). This seals deep imports; the CLI's eleven @finos/calm-shared/src/... imports (and the spec files that mocked those paths) now go through the root barrel, which additionally exports RESOURCE_TYPES.
  • Removed the junk "module": "esnext" field and the unused @stoplight/spectral-cli dependency (imported by zero files). The lockfile delta is that subtree's removal plus esbuild as a shared devDependency. It also carries a cli 1.55.0 → 1.56.0 version-sync line — pre-existing lockfile drift that npm install --package-lock-only corrected, unrelated to this change.
  • cli/tsconfig.json had module/moduleResolution outside compilerOptions (silently ignored, so the CLI resolved with node10 rules); they now sit inside it, which is what lets @finos/calm-shared/browser type-resolve.

Capability manifestBROWSER_COMMAND_SUPPORT / browserSupportFor(command) list every top-level CLI command (and the hub subgroups) as supported or unsupported with a reason, so browser consumers can report honestly why e.g. calm docify isn't available. cli/src/browser-manifest.spec.ts builds the Commander program and asserts the manifest and the registered commands match, so they can't drift.

Guardshared/scripts/check-browser-entry.mjs runs in npm test --workspace shared: bundles src/browser.ts with esbuild for the browser, fails on any Node builtin request outside a four-entry allowlist (Spectral's dependency chain requests fs/path/buffer but never touches fs/path at runtime — verified by a spike), then executes a probe that runs real validate(), generate() and diffDocuments() calls with those builtins stubbed to throw on touch and process/Buffer defined away; the guard also fails if an allowlist entry stops matching, so the list can't silently rot. browser-surface.spec.ts mirrors the probe inside vitest (schema + Spectral through injected loaders, no fs).

Docsshared/AGENTS.md and shared/README.md describe the two entry points, the browser-safe-by-default rule, the guard/allowlist, and the consumer bundler fallbacks (resolve.fallback: { fs: false, path: false, buffer: false }, browser main field first).

Deliberate departures / notes for reviewers

  • formatOutput no longer logs Formatting output as … — that line depended on validate() having run first and was never reliable.
  • exitBasedOffOfValidationOutcome (process.exit) stays exported from the root entry (in validate.ts, outside the browser graph); its own TODO says it belongs in the CLI — follow-up below.
  • Two (cli)-scoped commits are on this branch (refactor(cli) import migration, fix(cli) tsconfig), so merging will trigger a CLI patch release.

Follow-ups (to be filed as issues)

Type of Change

  • 🐛 Bug fix (non-breaking change which fixes an issue)
  • ✨ New feature (non-breaking change which adds functionality)
  • 💥 Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • 📚 Documentation update
  • 🎨 Code style/formatting changes
  • ♻️ Refactoring (no functional changes)
  • ⚡ Performance improvements
  • ✅ Test additions or updates
  • 🔧 Chore (maintenance, dependencies, CI, etc.)

Affected Components

  • CLI (cli/)
  • Schema (calm/)
  • CALM AI (calm-ai/)
  • CALM Hub (calm-hub/)
  • CALM Hub UI (calm-hub-ui/)
  • CALM Server (calm-server/)
  • CALM Widgets (calm-widgets/)
  • Documentation (docs/)
  • Shared (shared/)
  • VS Code Extension (calm-plugins/vscode/)
  • Dependencies
  • CI/CD

Commit Message Format ✅

Testing

  • I have tested my changes locally
  • I have added/updated unit tests
  • All existing tests pass

Full workspace sweep on a fresh npm ci: npm run build, npm run lint, npm test green across all workspaces (0 test failures; shared 100 files / 1142 tests incl. the guard, cli 29 / 649, calm-server 51). npm run build:cli && node cli/dist/index.js --help prints the command list. npx tsc -p cli/tsconfig.json --noEmit error count drops from 72 to 60 (all removed errors were exports-resolution failures from the misplaced tsconfig settings; no new ones).

Checklist

  • My commits follow the conventional commit format
  • I have updated documentation if necessary
  • I have added tests for my changes (if applicable)
  • My changes follow the project's coding standards

…exports subpaths type-resolve

module and moduleResolution were set at the top level of cli/tsconfig.json,
outside compilerOptions, so TypeScript silently ignored them and fell back
to the base config's Node resolution, which is not exports-aware. This broke
type resolution for package.json "exports" subpaths such as
@finos/calm-shared/browser and @finos/calm-models/types.
… pin SSRF ip-literal cases

The guard's runtime probe ran under Node, so process/Buffer globals were
still visible even though builtin module requests were stubbed; a stray
process.cwd() or Buffer.from() anywhere in the browser graph would pass
the guard and only throw in a real browser. Define process and Buffer as
undefined for the probe bundle so it sees browser semantics.

Also pin several SSRF-relevant host shapes (IPv4-mapped IPv6, zone IDs,
leading zeros, trailing colons, embedded IPv4 tails) in ipLiteralVersion's
test table, each confirmed against net.isIP.

Also tighten the guard's shared/src importer check to require a path
separator after the prefix, and document why the probe bundle's builtin
requests are intentionally not re-checked.
…mports and specs

Move ResourceType, RESOURCE_TYPES and isValidResourceType out of
calm-hub-client.ts (which imports axios) into a new resource-types.ts, and
have document-id-utils.ts import from there instead. calm-hub-client.ts
re-exports the moved names so its public surface and the root barrel are
unchanged. This keeps the axios-based hub client out of the browser
bundle's graph, since document-id-utils.ts is imported by browser.ts.

Add the missing .js suffixes to generate-core.ts's local imports for
consistency with its other imports.

Move the unmocked 'generate core' describe block out of generate.spec.ts
into a new generate-core.spec.ts that imports generate-core directly and
never mocks anything, so it needs no vi.doUnmock/resetModules dance that
only worked because it ran last in the file.
…ld assumption

Comment above the registerNodeLoggerFactory/registerOutputFormatter calls
in index.ts explaining they are load-bearing side effects, so
"sideEffects": false must never be added to shared/package.json.

Note in InMemoryDocumentLoader's doc comment that it returns documents by
reference rather than cloning them, unlike FileSystemDocumentLoader, which
re-parses from disk on every load.

Note in the README that the browser entry guard's allowlist assumes
bundlers resolve with the browser main field first, and that a node/SSR
bundle target will see more builtins than the allowlist covers.
…ral check

ipLiteralVersion() rejected IPv6 zone identifiers (e.g. fe80::1%eth0),
diverging from net.isIP's contract, which classifies them as version 6.
Split off a %zone suffix before classification: the zone must be
non-empty and match Node's accepted zone charset, and the part before %
must classify as IPv6 on its own (an IPv4 address with a zone id, e.g.
1.2.3.4%eth0, is never an IP literal, matching net.isIP).

Pinned test rows now match net.isIP's actual output for all zone-id
shapes checked (fe80::1%eth0 -> 6, fe80::1% -> 0, 1.2.3.4%eth0 -> 0,
::1%25 -> 6, fe80::1%eth0%x -> 0) instead of the implementation's
previous (incorrect) behaviour.
…ays out of the browser graph

Mirrors the generate-core/generate and diff-core/diff seam: validate.ts kept
exitBasedOffOfValidationOutcome (three process.exit calls) inside the module
graph reachable from browser.ts. Moving validate() and its helpers into
validate-core.ts lets browser.ts import the pure core directly, dropping
exitBasedOffOfValidationOutcome out of the browser bundle entirely.
…harden the allowlist check

- browser-probe.ts now exercises generate() and diffDocuments() through the
  browser entry, not just validate(), so process.exit-free core logic outside
  validate is also guarded.
- check-browser-entry.mjs normalises importer path separators before matching
  against ALLOWED (so the guard behaves the same on Windows) and reports any
  ALLOWED entry that matched nothing, so the allowlist can't silently rot.
- browser-capabilities.spec.ts asserts the unsupported-command list is
  non-empty before looping over it, and its doc comment states the
  drift-test's exact granularity.
…eral check

The IPV4_TAIL regex is unanchored at the start, so it could partial-match into
the middle of a hextet (e.g. matching "1.2.3.4" inside "a1.2.3.4") whenever any
colon appeared earlier in the string, wrongly classifying strings like
"db8::a1.2.3.4" as valid IPv6. A mapped tail is now only accepted when it is
immediately preceded by ':' (or starts the string), matching net.isIP.
…rift-test granularity

hub-commands.spec.ts mocked @finos/calm-shared via three separately-named
aliases (documentIdUtils/semver/canonical) that all pointed at the same
importActual barrel; collapsed to a single actual spread, with a comment
noting the vi.fn(...) overrides must come after it.

browser-manifest.spec.ts now documents exactly what granularity the drift
check operates at: top-level commands plus the hub subgroups, with workspace
subcommands covered by the single workspace entry.
Copilot AI lite review requested due to automatic review settings August 25, 2026 15:58

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a browser-safe @finos/calm-shared/browser entry point while isolating Node-specific functionality from pure CALM engines.

Changes:

  • Adds browser-safe validation, generation, diffing, loaders, and capability metadata.
  • Separates Node filesystem, logging, and output integrations.
  • Adds package exports, browser guards, tests, documentation, and CLI updates.

Reviewed changes

Copilot reviewed 60 out of 61 changed files in this pull request and generated 2 comments.

Show a summary per file
File Review result
shared/vitest.config.ts No final finding.
shared/src/util/ip-literal.ts No final finding.
shared/src/util/ip-literal.spec.ts No final finding.
shared/src/schema-directory.spec.ts No final finding.
shared/src/resolver/schema-directory-reference-resolver.ts No final finding.
shared/src/resolver/caching-tracking-resolver.ts No final finding.
shared/src/logger.ts No final finding.
shared/src/logger.spec.ts No final finding.
shared/src/logger.node.ts No final finding.
shared/src/index.ts No final finding.
shared/src/hub/resource-types.ts No final finding.
shared/src/hub/document-id-utils.ts No final finding.
shared/src/hub/calm-hub-client.ts No final finding.
shared/src/document-loader/node-document-loader.ts No final finding.
shared/src/document-loader/node-document-loader.spec.ts No final finding.
shared/src/document-loader/in-memory-document-loader.ts No final finding.
shared/src/document-loader/in-memory-document-loader.spec.ts No final finding.
shared/src/document-loader/document-loader.ts No final finding.
shared/src/document-loader/document-loader.spec.ts No final finding.
shared/src/document-loader/direct-url-document-loader.ts No final finding.
shared/src/document-loader/browser-document-loader.ts Critical (1 vote): Browser Axios redirects can bypass origin and private-address checks; redirects must be rejected/revalidated or remote loaders disabled.
shared/src/document-loader/browser-document-loader.spec.ts No final finding.
shared/src/commands/validate/validate.ts No final finding.
shared/src/commands/validate/validate-core.ts No final finding.
shared/src/commands/validate/output-formats/pretty-output.ts No final finding.
shared/src/commands/validate/output-formats/junit-output.ts No final finding.
shared/src/commands/validate/format-output.ts No final finding.
shared/src/commands/validate/format-output.spec.ts No final finding.
shared/src/commands/generate/generate.ts No final finding.
shared/src/commands/generate/generate-core.ts No final finding.
shared/src/commands/generate/generate-core.spec.ts No final finding.
shared/src/commands/diff/diff.ts No final finding.
shared/src/commands/diff/diff.spec.ts No final finding.
shared/src/commands/diff/diff-core.ts No final finding.
shared/src/browser.ts No final finding.
shared/src/browser-surface.spec.ts No final finding.
shared/src/browser-capabilities.ts Moderate (2 votes): timeline is advertised as supported although the registered command uses Node-only runTimelineGenerate; expose a pure core or mark it unsupported.
shared/src/browser-capabilities.spec.ts No final finding.
shared/scripts/check-browser-entry.mjs No final finding.
shared/scripts/browser-probe.ts No final finding.
shared/README.md No final finding.
shared/package.json No final finding.
shared/AGENTS.md No final finding.
package-lock.json No final finding.
cli/tsconfig.json No final finding.
cli/src/command-helpers/workspace/rm.ts No final finding.
cli/src/command-helpers/workspace/ref-rewrite.ts No final finding.
cli/src/command-helpers/workspace/ref-rewrite.spec.ts No final finding.
cli/src/command-helpers/workspace/push.ts No final finding.
cli/src/command-helpers/workspace/push.spec.ts No final finding.
cli/src/command-helpers/workspace/document-id-prompt.ts No final finding.
cli/src/command-helpers/workspace/config.ts No final finding.
cli/src/command-helpers/workspace/commands.ts No final finding.
cli/src/command-helpers/workspace/commands.spec.ts No final finding.
cli/src/command-helpers/workspace/bump.ts No final finding.
cli/src/command-helpers/workspace/bump.spec.ts No final finding.
cli/src/command-helpers/hub-commands.spec.ts No final finding.
cli/src/command-helpers/ai-tools.ts No final finding.
cli/src/cli.spec.ts No final finding.
cli/src/cli.e2e.spec.ts No final finding.
cli/src/browser-manifest.spec.ts No final finding.
Suppressed comments (2)

shared/src/commands/generate/generate-core.ts:16

  • generate() initializes the schema directory here, but instantiate() still calls schemaDirectory.loadSchemas() at shared/src/commands/generate/components/instantiate.ts:152. Every generation therefore reloads and reparses all filesystem schemas (and repeats any custom loader initialization); keep initialization in one layer or make it explicitly idempotent.
    shared/src/index.ts:65
  • Removing this export changes the package-root API: expectDirectoryMatch and expectFilesMatch were previously re-exported from @finos/calm-shared, while the CLI's new relative test import does not preserve existing consumers. Restore the re-export; it is Node-only and does not affect the ./browser graph.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread shared/src/browser-capabilities.ts Outdated
Comment thread shared/src/document-loader/browser-document-loader.ts
…ability manifest

The CLI's timeline command synthesises a timeline from versioned architecture
files on the local filesystem, but the browser entry only exports
diffTimeline (the diff --timeline core). Correct the manifest so browser
consumers report timeline as unsupported instead of claiming it works.
axios's maxRedirects only applies in Node; browsers follow redirects
transparently, so a redirect from an allowed origin could be answered by a
different one, bypassing DirectUrlDocumentLoader's host allowlist and
CalmHubDocumentLoader's configured origin. Revalidate the final response
origin (XHR responseURL, fetch Response.url) against the origin requested,
and reject the response on a mismatch.
@jpgough-ms

Copy link
Copy Markdown
Member

Code review (medium effort)

Five findings, all in the new DirectUrlDocumentLoader. No correctness bugs beyond #4, no efficiency/simplification issues — the rest of the diff is clean.

  1. SAFE_PATH_PATTERN diverges from CalmHubDocumentLoader's copy (shared/src/document-loader/direct-url-document-loader.ts:29)

    • Comment claims it "mirrors" the sibling regex; it doesn't.
    • calmhub-document-loader.ts:10: /^[a-zA-Z0-9/_\-.]+(\.json)?$/
    • direct-url-document-loader.ts:29: /^[a-zA-Z0-9/_.-]+$/
    • A future tightening of one allowlist won't propagate to the other.
  2. Directory-traversal guard duplicated, not shared (direct-url-document-loader.ts:146)

    • documentId.includes('/..') plus its comment is copy-pasted from CalmHubDocumentLoader.
    • response-origin.ts's assertResponseOrigin is the existing precedent for factoring shared cross-loader security checks out.
    • Risk: a fix to the traversal check lands in one loader and not the other.
  3. SSRF guard has no DNS resolution before connect (direct-url-document-loader.ts:31)

    • isPrivateHost only rejects IP-literal/localhost hostnames.
    • An allowlisted hostname (calm init-config --allowed-remote-hosts <host>) that later DNS-rebinds to 127.0.0.1 or a metadata IP passes the check and axios connects to it at request time.
    • Pre-existing logic, config-gated — narrow but real.
  4. Sealed exports map drops test-helper exports (shared/src/index.ts:42)

    • export * from './test/file-comparison.js' removed; shared/package.json's new exports map exposes only . and ./browser.
    • expectFilesMatch/expectDirectoryMatch are no longer reachable via @finos/calm-shared.
    • Confirmed intentional: cli/src/cli.e2e.spec.ts switched to a relative import in this same PR to work around it. Any other workspace package importing these helpers from the package root will fail to resolve — no deep-path fallback since exports is sealed.
  5. Reintroduces previously-disabled verbose axios logging (direct-url-document-loader.ts:77)

    • addAxiosDebug() duplicates a method CalmHubDocumentLoader disabled via // TODO this is far, far too verbose for -v (dead code there, call site commented out).
    • Here it's called unconditionally whenever debug is true — reintroduces the same verbosity problem in a second copy.

@rocketstack-matt

Copy link
Copy Markdown
Member Author

@jpgough-ms thanks — verdicts per item:

1, 2, 3, 5 — all pre-exist at this PR's merge base (direct-url-document-loader.ts:28/72/76/145 and calmhub-document-loader.ts:9/102 on 92ed20c6); this PR only swapped net.isIP for the browser-safe check and added assertResponseOrigin. Rather than grow an approved 25-commit PR with unrelated hardening, I've tracked them as #3024 (shared path/traversal guards, DNS-resolution SSRF check pinned to the checked address, dropping addAxiosDebug, plus the duplicate loadSchemas() call Copilot flagged). Happy to pick that up next.

4 — intentional: expectFilesMatch/expectDirectoryMatch are fs-based test helpers that only shared's own specs and cli.e2e.spec.ts use (the latter now imports them relatively), and shared is private: true, so there is no external consumer. Keeping test utilities out of the package's public surface was part of the point of sealing exports; if you'd rather keep the Node-only re-export for continuity, it's a one-liner and doesn't touch the browser graph — say the word.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cli Affects `cli` code config shared

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Browser entry point for @finos/calm-shared: run the real CALM engine in browsers

3 participants