0.1.1: answer the review — and stop shipping bytes we don't run

Two blocking Errors in the 0.1.0 review, both the same root cause: inline
`eslint-disable` comments for an `obsidianmd/*` rule are rejected outright.
The console logging they suppressed was only a Warning — the directives
were the failure. Everything else in a very long report was a Warning or a
Recommendation.

Directives gone, and the logging with them rather than suppressed. The two
dev-only `console.log`s moved onto the debug shim as `backendSpoof()` and
`lastWorkerEnv()`, which is strictly better: obsidian-drive.mjs can assert
on the values instead of a human reading them out of a console. The worker's
warn/error bridge stays — a worker is its own console target, so without it
a conversion that died inside the thread leaves nothing behind — but it is
relaxed in eslint.config.mjs, where a reader can see it, not mid-file. Also
dropped an unnecessary no-control-regex directive; the repository now has
zero inline eslint directives.

**The finding worth keeping: the review reads the bundle, not the intent.**
main.js contained `require("fs")`, `require("path")` and `require("os")`,
and the plugin was described to users as able to "read and write any file on
the system". All three lived inside the onnxruntime emscripten glue, which
was inlined *as a text string* and — since ORT fixed the renderer guard
upstream around 1.24 — never used at all. Ninety-two kilobytes of dead text
earned a filesystem-access warning on a public safety scorecard. The
decision now happens in analyzeOrtGlue() at build time and the glue is
inlined only when a patch is genuinely needed; the loud failure on an
unrecognised epilogue stays, so a future ORT bump still can't reintroduce
the bug quietly. main.js 4675571 → 4582504 bytes, and the only `require()`
left in it is `require("obsidian")`.

Encoding that string so a scanner wouldn't recognise it would also have
"worked". It is obfuscation, which the developer policies prohibit, and the
wrong instinct regardless: not shipping it is the fix.

Also: pdf.js gets `isEvalSupported: false` on both the worker and renderer
paths, so Type-4 shading functions are interpreted rather than compiled —
a converter should not compile code out of the document it was handed.
Priced against the ground-truth suite before keeping it: attention 8/8
checks on WebGPU, 376 s, identical to the recorded baseline. `authorUrl` now
points at the org profile rather than this repository, and the release
workflow attests its assets (actions/attest-build-provenance, with the
id-token and attestations permissions that needs).

Deliberately not fixed, and why: the "use window.setTimeout" and "avoid
globalThis" warnings land on code that runs inside a Web Worker, where
`window` does not exist — applying them would break the default conversion
path. The hundreds of no-unsafe-*/no-explicit-any warnings land on untyped
third-party surfaces (transformers.js env, ORT wasmPaths, pdf.js internals)
across files including the Node CLI, which is not in the plugin bundle at
all. getSettingDefinitions() is real UX value and deferred to 0.2.0.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Author: rcurrie

.github/workflows/release.yml

@@ -16,6 +16,13 @@ on:
 
 permissions:
   contents: write
+  # Artifact attestations: `id-token` signs the provenance statement with the
+  # run's OIDC identity, `attestations` writes it to the repository's store.
+  # Without them a user has to take on faith that the main.js they downloaded
+  # was built from this source — which, for a plugin that reads their documents,
+  # is worth more than the two lines it costs.
+  id-token: write
+  attestations: write
 
 jobs:
   release:
@@ -41,6 +48,17 @@ jobs:
       - run: npm run typecheck
       - run: npm run build
 
+      # Must run in the job that produced the files, against the exact bytes
+      # that get attached below — an attestation over a rebuilt artifact proves
+      # nothing about the one users download.
+      - name: Attest the release assets
+        uses: actions/attest-build-provenance@v4
+        with:
+          subject-path: |
+            plugin/dist/main.js
+            plugin/dist/manifest.json
+            plugin/dist/styles.css
+
       # Left as a draft on purpose, so the assets can be checked before anyone
       # can install them. When you publish it, leave "Set as a pre-release"
       # UNTICKED: GitHub excludes pre-releases from /releases/latest, and

PLAN.md

@@ -558,10 +558,18 @@ without the per-platform binaries. Keep it in the back pocket if a machine ever
       `plugin/` can no longer see it (and silently assumed a mobile plugin, which made Electron's
       `process` an undefined global). `plugin/check-manifest.mjs` asserts the submission rules
       directly and gates the release workflow.
-- [ ] **Submit.** Blocked on two things that are the author's to do: the repository is still
-      **private**, and there is **no release yet** — the directory needs a tag equal to the manifest
-      version carrying `main.js`, `manifest.json`, `styles.css`. Then community.obsidian.md; the
-      `obsidian-releases` PR route is gone.
+- [x] **Submitted; 0.1.0 failed review; 0.1.1 answers it.** Two blocking Errors, both the same root
+      cause: inline `eslint-disable` comments for an `obsidianmd/*` rule are rejected outright —
+      the console logging they suppressed was only a Warning. Everything else in a very long report
+      was a Warning or Recommendation. Fixed: directives gone (dev logs moved onto the probe, where
+      the driver can assert on them instead of a human reading a console); `authorUrl` → the org
+      profile; artifact attestations on the release assets.
+      **The finding worth remembering: the review reads the bundle, not the intent.** `main.js`
+      carried `require("fs")`/`path`/`os` and the plugin was reported to users as able to "read and
+      write any file on the system" — from 92 KB of onnxruntime emscripten glue inlined *as a text
+      string* and, since ORT fixed the guard upstream ~1.24, never used at all. Now decided at build
+      time and inlined only when a patch is genuinely needed. Also `isEvalSupported: false` for
+      pdf.js, so a document's Type-4 shading functions are interpreted rather than compiled.
 - [ ] **Windows and Linux remain untested.** The code no longer assumes WebGPU (probe → candidates →
       CPU fallback, all unit-tested against fake navigators), but nothing has run on either OS.
 

engine-js/src/browser/pdf.ts

@@ -141,7 +141,7 @@ export interface LoadPdfBrowserOptions {
 }
 
 /**
- * Extra `getDocument` parameters needed when there is no document.
+ * `getDocument` parameters for this host.
  *
  * Font handling matters as much as the canvas: with `disableFontFace` left at
  * its browser default, pdf.js registers `FontFace`s against `document.fonts`.
@@ -150,8 +150,22 @@ export interface LoadPdfBrowserOptions {
  * the one the fixture suite validated.
  */
 function docParams(opts: LoadPdfBrowserOptions): Record<string, unknown> {
-  if (typeof document !== "undefined") return {};
+  /**
+   * Never compile PDF-supplied code.
+   *
+   * pdf.js turns Type 4 (PostScript calculator) shading functions into
+   * JavaScript with `new Function(src, …)` when it can, and falls back to an
+   * interpreter when it can't. The speed only matters for gradient-heavy
+   * artwork, and we rasterize once per page; a document conversion tool
+   * compiling expressions out of the document it was handed is a bad trade at
+   * any speed, and it is what Obsidian's review flags as dynamic code
+   * execution. Applies on *both* paths — the renderer fallback runs the same
+   * pdf.js as the worker.
+   */
+  const shared = { isEvalSupported: false };
+  if (typeof document !== "undefined") return shared;
   return {
+    ...shared,
     CanvasFactory: OffscreenCanvasFactory,
     FilterFactory: NoopFilterFactory,
     disableFontFace: true,

manifest.json

@@ -1,10 +1,10 @@
 {
   "id": "reflow",
   "name": "Reflow",
-  "version": "0.1.0",
+  "version": "0.1.1",
   "minAppVersion": "1.5.0",
   "description": "Convert a PDF into clean, readable Markdown entirely on your device. Figures, tables, and math survive. No API key, no upload, no page limit.",
   "author": "Ampdat",
-  "authorUrl": "https://github.com/ampdat/reflow",
+  "authorUrl": "https://github.com/ampdat",
   "isDesktopOnly": true
 }

plugin/README.md

@@ -114,10 +114,44 @@ Obsidian's installer downloads **only** `main.js`, `manifest.json` and
    would quietly convert on the main thread forever. `worker.ts` is bundled to
    `build/worker.js` (scratch, gitignored) and inlined from there.
 2. **pdf.js's parsing worker is inlined too** ([`assets.ts`](assets.ts)), for the
-   remote-code reason above. `main.js` is ~4.7 MB as a result: the main bundle,
+   remote-code reason above. `main.js` is ~4.6 MB as a result: the main bundle,
    the worker bundle, and two copies of the pdf.js worker (the renderer path and
    the conversion worker each need one).
 
+### What the review reads is the bundle, not your intent
+
+Two findings in the 0.1.0 review came from bytes in `main.js` that the plugin
+never executes, and both are worth remembering because the instinct they invite
+is the wrong one.
+
+**"Direct Filesystem Access — can read and write any file on the system."**
+`main.js` contained `require("fs")`, `require("path")` and `require("os")`. All
+of them lived inside the onnxruntime emscripten glue, which was inlined *as a
+text string* and — on a current dependency tree — never used at all, because
+the guard it patches has been fixed upstream since ORT ~1.24. Ninety-two
+kilobytes of dead text was enough to have the plugin described to users as
+something that reads their filesystem. The fix is in
+[`esbuild.config.mjs`](esbuild.config.mjs): decide at build time whether a patch
+is needed and inline the glue only then. Encoding the string so a scanner
+wouldn't recognise it would have "worked" and is exactly what the developer
+policies prohibit — obfuscation is a policy violation, not a workaround.
+
+**Dynamic code execution.** pdf.js compiles Type 4 (PostScript calculator)
+shading functions with `new Function(src, …)`. `docParams()` in
+[`browser/pdf.ts`](../engine-js/src/browser/pdf.ts) now passes
+`isEvalSupported: false` on both the worker and renderer paths, so pdf.js uses
+its interpreter and never compiles code out of the document it was handed. Its
+own `new Function("")` capability probe still appears in the bundle, so a static
+scanner may still flag the pattern; the substance is that nothing in a converted
+PDF becomes executable code.
+
+**Inline `eslint-disable` comments for `obsidianmd/*` rules are rejected
+outright** — that, not the logging they suppressed, is what failed 0.1.0. The
+console calls themselves are only a warning. Where a rule genuinely has to be
+relaxed (the worker's warn/error bridge, which is a worker's only route to the
+console), it is relaxed in [`eslint.config.mjs`](eslint.config.mjs) where it is
+visible, never mid-file.
+
 ### The manifest lives at the repository root
 
 [`../manifest.json`](../manifest.json), not next to this source, because the

plugin/check-manifest.mjs

@@ -40,7 +40,6 @@ const d = manifest.description ?? "";
 check(d.length <= 250, `description must be 250 characters or fewer (is ${d.length})`);
 check(d.endsWith("."), "description must end with a period");
 check(!/^(this is a |this plugin|a plugin)/i.test(d), "description must not start with 'This is a plugin'");
-// eslint-disable-next-line no-control-regex -- deliberately matching non-ASCII
 check(!/[\u{1F300}-\u{1FAFF}\u{2600}-\u{27BF}]/u.test(d), "description must not contain emoji");
 
 check("fundingUrl" in manifest === false || !!manifest.fundingUrl, "remove fundingUrl if unused");

plugin/esbuild.config.mjs

@@ -34,54 +34,79 @@ const dev = watch || process.argv.includes("--dev");
 //
 // Instead, inline a *patched* copy as text under `virtual:ort-glue`; ort-env.ts
 // serves it from a blob URL via `wasmPaths.mjs`. See plugin/ort-env.ts.
+//
+// The decision is made **here, at build time**, and the text is inlined only if
+// a patch is actually needed. It used to be inlined unconditionally and the
+// same question asked again at runtime, which cost more than the 92 KB: the
+// glue's own Node branch (`var fs = require("fs")`, `require("path")`,
+// `require("os")`) sat in main.js as dead string data, and Obsidian's review
+// reads the bundle, not the guard around it. The plugin was reported as
+// "Direct Filesystem Access ... can read and write any file on the system" on
+// the strength of text it never executes. Encoding the string to hide it would
+// be obfuscation, which the developer policies forbid outright — and would be
+// the wrong instinct anyway. Not shipping it is the honest fix.
 const ORT_GLUE = "node_modules/@huggingface/transformers/dist/ort-wasm-simd-threaded.jsep.mjs";
 const GLUE_EPILOGUE =
   "if (isNode) isPthread = (await import('worker_threads')).workerData === 'em-pthread';";
 
+/**
+ * Does the installed glue still need our patch, and if so, what does the
+ * patched copy look like?
+ *
+ * Runs once per build rather than per bundle, so both the main and the worker
+ * context agree — and so the answer can be `define`d into both.
+ */
+function analyzeOrtGlue() {
+  const source = readFileSync(ORT_GLUE, "utf8");
+
+  // onnxruntime-web ~1.24-1.26 added the missing guard upstream, so on a
+  // current transformers there is nothing to patch. Detect that rather than
+  // assuming a version: `process?.type != "renderer"` in the epilogue.
+  if (/isPthread\s*=\s*\(await import\(["']worker_threads["']\)\)/.test(source)) {
+    const epilogue = source.slice(source.lastIndexOf("export default"));
+    if (/process\??\.type\s*!=/.test(epilogue)) {
+      console.log("  [ort-glue] upstream glue guards on process.type — nothing inlined");
+      return { patched: false, source: "" };
+    }
+  }
+
+  if (!source.includes(GLUE_EPILOGUE)) {
+    // Neither the known-broken epilogue nor the upstream fix: fail loudly
+    // rather than shipping a glue we only think we understand.
+    throw new Error(
+      `[ort-glue] unrecognized node-detection epilogue in ${ORT_GLUE}. ` +
+        `onnxruntime-web changed; re-check the patch in plugin/ort-env.ts.`,
+    );
+  }
+  const patched = source.replace(GLUE_EPILOGUE, "// [reflow] node pthread bootstrap removed");
+  // One `worker_threads` reference legitimately survives: the one inside the
+  // module body, which *is* guarded by `"renderer" != process.type` and so is
+  // unreachable here. Anything else means the patch missed something.
+  const remaining = patched.split("worker_threads").length - 1;
+  if (remaining !== 1 || !patched.includes('"renderer"!=process.type')) {
+    throw new Error(
+      `[ort-glue] expected exactly one renderer-guarded worker_threads reference ` +
+        `after patching, found ${remaining} (guard present: ` +
+        `${patched.includes('"renderer"!=process.type')})`,
+    );
+  }
+  console.log(`  [ort-glue] patched ${(patched.length / 1024).toFixed(1)} KB of emscripten glue`);
+  return { patched: true, source: patched };
+}
+
+const ortGlue = analyzeOrtGlue();
+
 const inlinePatchedOrtGlue = {
   name: "inline-patched-ort-glue",
   setup(build) {
     build.onResolve({ filter: /^virtual:ort-glue$/ }, () => ({
       path: "virtual:ort-glue",
       namespace: "ort-glue",
     }));
-    build.onLoad({ filter: /.*/, namespace: "ort-glue" }, () => {
-      const source = readFileSync(ORT_GLUE, "utf8");
-
-      // onnxruntime-web ~1.24-1.26 added the missing guard upstream, so on a
-      // current transformers there is nothing to patch. Detect that rather than
-      // assuming a version: `process?.type != "renderer"` in the epilogue.
-      if (/isPthread\s*=\s*\(await import\(["']worker_threads["']\)\)/.test(source)) {
-        const epilogue = source.slice(source.lastIndexOf("export default"));
-        if (/process\??\.type\s*!=/.test(epilogue)) {
-          console.log("  [ort-glue] upstream glue already guards on process.type — no patch needed");
-          return { contents: source, loader: "text" };
-        }
-      }
-
-      if (!source.includes(GLUE_EPILOGUE)) {
-        // Neither the known-broken epilogue nor the upstream fix: fail loudly
-        // rather than shipping a glue we only think we understand.
-        throw new Error(
-          `[ort-glue] unrecognized node-detection epilogue in ${ORT_GLUE}. ` +
-            `onnxruntime-web changed; re-check the patch in plugin/ort-env.ts.`,
-        );
-      }
-      const patched = source.replace(GLUE_EPILOGUE, "// [reflow] node pthread bootstrap removed");
-      // One `worker_threads` reference legitimately survives: the one inside the
-      // module body, which *is* guarded by `"renderer" != process.type` and so is
-      // unreachable here. Anything else means the patch missed something.
-      const remaining = patched.split("worker_threads").length - 1;
-      if (remaining !== 1 || !patched.includes('"renderer"!=process.type')) {
-        throw new Error(
-          `[ort-glue] expected exactly one renderer-guarded worker_threads reference ` +
-            `after patching, found ${remaining} (guard present: ` +
-            `${patched.includes('"renderer"!=process.type')})`,
-        );
-      }
-      console.log(`  [ort-glue] patched ${(patched.length / 1024).toFixed(1)} KB of emscripten glue`);
-      return { contents: patched, loader: "text" };
-    });
+    build.onLoad({ filter: /.*/, namespace: "ort-glue" }, () => ({
+      contents: ortGlue.source,
+      loader: "text",
+    }));
   },
 };
 
@@ -149,7 +174,13 @@ const shared = {
   // with --node-integration-in-worker, so `process.release.name === "node"` is
   // true there too and transformers.js would pick its cpu-only node backend.
   banner: { js: forceWebBackend },
-  define: { __REFLOW_DEV__: String(dev) },
+  define: {
+    __REFLOW_DEV__: String(dev),
+    // Whether `virtual:ort-glue` carries a patched copy at all. Asked at build
+    // time so ort-env.ts doesn't have to re-derive it by regexing text that,
+    // on a healthy dependency tree, isn't in the bundle.
+    __REFLOW_ORT_GLUE_PATCHED__: String(ortGlue.patched),
+  },
   sourcemap: watch ? "inline" : false,
   minify: !watch,
   logLevel: "info",

plugin/eslint.config.mjs

@@ -20,8 +20,9 @@ export default tseslint.config(
     languageOptions: {
       parserOptions: { project: "./tsconfig.json", tsconfigRootDir: import.meta.dirname },
       globals: {
-        // Build-time constant, `define`d by esbuild (see virtual.d.ts).
+        // Build-time constants, `define`d by esbuild (see virtual.d.ts).
         __REFLOW_DEV__: "readonly",
+        __REFLOW_ORT_GLUE_PATCHED__: "readonly",
         // `manifest.json` lives at the repository root, because that is where
         // the community directory reads it from. This config's base path is
         // plugin/, so the obsidianmd rules can no longer find it and fall back
@@ -62,6 +63,15 @@ export default tseslint.config(
       "obsidianmd/rule-custom-message": "off",
     },
   },
+  {
+    // The bridge that carries worker-side warnings and errors to the renderer's
+    // console (see the comment at the call site). Configured here rather than
+    // with an inline `eslint-disable`, which Obsidian's review rejects outright
+    // for obsidianmd/* rules — and rightly: a rule turned off in a config file
+    // is visible, one turned off mid-file is not.
+    files: ["worker-host.ts"],
+    rules: { "obsidianmd/rule-custom-message": "off" },
+  },
   {
     // probe.ts is development-only: release builds resolve it to probe-stub.ts
     // (see esbuild.config.mjs), so nothing here reaches an installed plugin.

plugin/main.ts

@@ -342,6 +342,8 @@ export default class ReflowPlugin extends Plugin {
   private workerUrl: string | null = null;
   /** How the last conversion actually ran, for the probe and the log. */
   lastRunMode: "worker" | "renderer" | null = null;
+  /** Environment the conversion worker reported on boot; read by the probe. */
+  lastWorkerEnv: Record<string, unknown> | null = null;
   /** Compute-backend probe, run once per session (see `probeOnce`). */
   private deviceProbe: Promise<DeviceProbe> | null = null;
   /** Its resolved value, for the synchronous message path. */
@@ -363,19 +365,21 @@ export default class ReflowPlugin extends Plugin {
    */
   private installDevProbe(): void {
     const w = window as unknown as { __reflow_spoofResult?: string };
-    // eslint-disable-next-line obsidianmd/rule-custom-message -- dev build only
-    console.log(
-      `[reflow] backend spoof: ${w.__reflow_spoofResult} | ` +
-        `navigator.gpu: ${typeof navigator !== "undefined" && "gpu" in navigator} | ` +
-        `ort glue: ${ortConfig.strategy} (${ortConfig.glueBytes} B)`,
-    );
 
     installProbe({
       // The injected platform libs, so an ad-hoc probe can exercise pdf.js or
       // transformers.js directly (bisecting a stall to one of them) without a
       // plugin rebuild — same rationale as exposing `ort`.
       pdfjs,
       transformers,
+      /**
+       * Whether the esbuild banner's `process.release.name` rename took, read
+       * back off the global it wrote. This used to be logged at load; it is
+       * pulled rather than pushed now, which costs nothing (the plugin is
+       * loaded long before anyone asks) and keeps a diagnostic that only
+       * developers want out of every user's console.
+       */
+      backendSpoof: () => w.__reflow_spoofResult ?? null,
       readBinary: async (path: string) => {
         const f = this.app.vault.getAbstractFileByPath(path);
         if (!(f instanceof TFile)) throw new Error(`not a file in the vault: ${path}`);
@@ -394,6 +398,15 @@ export default class ReflowPlugin extends Plugin {
         return this.settings.useWorker;
       },
       lastRunMode: () => this.lastRunMode,
+      /**
+       * What the conversion worker reported about its environment on boot —
+       * WebGPU, OffscreenCanvas, whether the backend spoof took there too.
+       *
+       * Also formerly a `console.log`. Holding it is strictly more useful than
+       * printing it: `tools/obsidian-drive.mjs` can assert on the values
+       * instead of a human reading them out of a console.
+       */
+      lastWorkerEnv: () => this.lastWorkerEnv,
       ortConfig,
       benchPage: async (path: string, opts?: Record<string, unknown>) => {
         const f = this.app.vault.getAbstractFileByPath(path);
@@ -679,8 +692,7 @@ export default class ReflowPlugin extends Plugin {
           signal,
           ...hooks,
           onReady: (env) => {
-            // eslint-disable-next-line obsidianmd/rule-custom-message -- dev build only
-            if (__REFLOW_DEV__) console.log("[reflow] worker ready:", env);
+            this.lastWorkerEnv = env;
           },
         });
       }