fix: keep tracked-change revision ids in signed 32-bit range

.changeset/revision-id-int32-bounds.md

@@ -0,0 +1,6 @@
+---
+"@stll/folio-core": patch
+"@stll/docx-core": patch
+---
+
+Keep tracked-change revision ids within the range OOXML consumers accept. Suggestion-mode edits seeded their `w:ins`/`w:del` id counter from the clock, producing 13-digit `w:id` values that made exported documents fail to open. Ids now continue from the document's own highest revision id. Port of eigenpal/docx-editor#1093.

packages/core/src/docx/paragraphParser.test.ts

@@ -223,6 +223,39 @@ describe("parseParagraph tracked-change hardening", () => {
       id: 5,
     });
   });
+
+  test("folds an out-of-range id on an inline w:ins into the int32 range (eigenpal #1093)", () => {
+    const OUT_OF_RANGE = "2147483654"; // MAX_REVISION_ID + 7
+    const paragraph = parseParagraphXml(`
+      <w:p xmlns:w="http://schemas.openxmlformats.org/wordprocessingml/2006/main">
+        <w:ins w:id="${OUT_OF_RANGE}" w:author="A">
+          <w:r><w:t>added</w:t></w:r>
+        </w:ins>
+      </w:p>
+    `);
+
+    const insertion = paragraph.content.find((c) => c.type === "insertion");
+    expect(insertion?.type).toBe("insertion");
+    if (!insertion || insertion.type !== "insertion") {
+      return;
+    }
+    expect(insertion.info.id).toBeLessThanOrEqual(2_147_483_647);
+    expect(insertion.info.id).toBeGreaterThanOrEqual(0);
+  });
+
+  test("folds an out-of-range id on a paragraph-mark w:ins (pPrMark)", () => {
+    const OUT_OF_RANGE = "2147483654";
+    const paragraph = parseParagraphXml(`
+      <w:p xmlns:w="http://schemas.openxmlformats.org/wordprocessingml/2006/main">
+        <w:pPr><w:rPr><w:ins w:id="${OUT_OF_RANGE}" w:author="A"/></w:rPr></w:pPr>
+        <w:r><w:t>text</w:t></w:r>
+      </w:p>
+    `);
+
+    expect(paragraph.pPrMark).toBeDefined();
+    expect(paragraph.pPrMark?.info.id).toBeLessThanOrEqual(2_147_483_647);
+    expect(paragraph.pPrMark?.info.id).toBeGreaterThanOrEqual(0);
+  });
 });
 
 describe("parseParagraph rendered page break markers", () => {

packages/core/src/docx/paragraphParser.ts

@@ -35,6 +35,7 @@ import type {
   TrackedChangeInfo,
   MathEquation,
 } from "../types/document";
+import { normalizeRevisionId } from "@stll/docx-core/model";
 import { isValidHexId } from "../utils/hexId";
 import {
   parseBookmarkStart as parseBookmarkStartFromModule,
@@ -954,7 +955,9 @@ function parseTrackedChangeInfo(node: XmlElement): TrackedChangeInfo {
   const initials = (getAttribute(node, "w", "initials") ?? "").trim();
 
   const info: TrackedChangeInfo = {
-    id: Number.isInteger(parsedId) && parsedId >= 0 ? parsedId : 0,
+    // `w:id` is attacker-controlled and unbounded in the schema; fold it into
+    // the range consumers accept at the parse boundary (eigenpal #1093).
+    id: normalizeRevisionId(parsedId),
     author: author.length > 0 ? author : "Unknown",
   };
   if (date.length > 0) {

packages/core/src/docx/runParser.ts

@@ -41,6 +41,7 @@ import type {
   MediaFile,
   ShapeContent,
 } from "../types/document";
+import { normalizeRevisionId } from "@stll/docx-core/model";
 import { parseGroupDrawing } from "./groupDrawingParser";
 import { parseImage } from "./imageParser";
 import {
@@ -656,7 +657,9 @@ function parsePropertyChangeInfo(changeElement: XmlElement): RunPropertyChange["
   const rsid = (getAttribute(changeElement, "w", "rsid") ?? "").trim();
 
   const info: RunPropertyChange["info"] = {
-    id: Number.isInteger(parsedId) && parsedId >= 0 ? parsedId : 0,
+    // `w:id` is attacker-controlled and unbounded in the schema; fold at the
+    // parse boundary (eigenpal #1093).
+    id: normalizeRevisionId(parsedId),
     author: author.length > 0 ? author : "Unknown",
   };
   if (date.length > 0) {

packages/core/src/docx/sectionParser.ts

@@ -31,6 +31,7 @@ import type {
   BorderSpec,
   ColorValue,
 } from "../types/document";
+import { normalizeRevisionId } from "@stll/docx-core/model";
 import { parseHeaderReference, parseFooterReference } from "./headerFooterRefParser";
 import { parseFootnoteProperties, parseEndnoteProperties } from "./notePropertiesParser";
 import { BorderStyleSchema, ThemeColorSlotSchema, narrowEnum } from "./parserEnums";
@@ -142,7 +143,9 @@ function parsePropertyChangeInfo(node: XmlElement): SectionPropertyChange["info"
   const rsid = (getAttribute(node, "w", "rsid") ?? "").trim();
 
   const info: SectionPropertyChange["info"] = {
-    id: Number.isInteger(parsedId) && parsedId >= 0 ? parsedId : 0,
+    // `w:id` is attacker-controlled and unbounded in the schema; fold at the
+    // parse boundary (eigenpal #1093).
+    id: normalizeRevisionId(parsedId),
     author: author.length > 0 ? author : "Unknown",
   };
   if (date.length > 0) {

packages/core/src/docx/serializer/paragraphSerializer.test.ts

@@ -176,6 +176,44 @@ describe("serializeParagraph tracked-change hardening", () => {
     expect(xml).toContain('<w:ins w:id="0" w:author="Unknown">');
     expect(xml).not.toContain("w:date=");
   });
+
+  test("folds an out-of-range revision id into the signed 32-bit range (eigenpal #1093)", () => {
+    // A `Date.now()`-derived id (~1.8e12) is a well-formed positive integer, so
+    // the invalid/negative guard used to pass it straight through to `w:id`.
+    const paragraph: Paragraph = {
+      type: "paragraph",
+      content: [
+        {
+          type: "insertion",
+          info: { id: 1_784_212_345_678, author: "Reviewer" },
+          content: [{ type: "run", content: [{ type: "text", text: "Added" }] }],
+        },
+      ],
+    };
+
+    const xml = serializeParagraph(paragraph);
+    const id = Number(/<w:ins w:id="(\d+)"/u.exec(xml)?.[1]);
+    expect(id).toBeLessThanOrEqual(2_147_483_647);
+    expect(id).toBeGreaterThanOrEqual(0);
+  });
+
+  test("keeps distinct out-of-range ids distinct so revisions do not merge", () => {
+    const idFor = (id: number): string => {
+      const paragraph: Paragraph = {
+        type: "paragraph",
+        content: [
+          {
+            type: "insertion",
+            info: { id, author: "Reviewer" },
+            content: [{ type: "run", content: [{ type: "text", text: "Added" }] }],
+          },
+        ],
+      };
+      return /<w:ins w:id="(\d+)"/u.exec(serializeParagraph(paragraph))?.[1] ?? "";
+    };
+
+    expect(idFor(1_784_212_345_678)).not.toBe(idFor(1_784_212_345_679));
+  });
 });
 
 describe("serializeParagraph native frame geometry", () => {

packages/core/src/docx/serializer/paragraphSerializer.ts

@@ -35,6 +35,7 @@ import type {
   TextFormatting,
   TrackedChangeInfo,
 } from "../../types/document";
+import { normalizeRevisionId } from "@stll/docx-core/model";
 import { isValidHexColor } from "../../utils/colorResolver";
 import { numPrEqual } from "../numberingParser";
 import { reconcileRawSdtPr } from "../sdtPropertiesPatch";
@@ -375,7 +376,8 @@ function serializeTrackedChangeAttrs(info: TrackedChangeInfo): string {
   // NOTE: `w:initials` is intentionally NOT emitted — ECMA-376 CT_TrackChange
   // defines only w:id/w:author/w:date. Initials are carried in-model for UI
   // attribution only (w:comment is the sole standards-clean initials target).
-  const parts = [`w:id="${info.id}"`, `w:author="${escapeXml(info.author)}"`];
+  // Bound `w:id` here so an overflowing id cannot reach the XML (eigenpal #1093).
+  const parts = [`w:id="${normalizeRevisionId(info.id)}"`, `w:author="${escapeXml(info.author)}"`];
   if (info.date !== undefined) {
     parts.push(`w:date="${escapeXml(info.date)}"`);
   }
@@ -547,7 +549,7 @@ function extractRPrInner(rPrXml: string): string {
 }
 
 function serializeParagraphPropertyChange(change: ParagraphPropertyChange): string {
-  const normalizedId = Number.isInteger(change.info.id) && change.info.id >= 0 ? change.info.id : 0;
+  const normalizedId = normalizeRevisionId(change.info.id);
   const authorCandidate = typeof change.info.author === "string" ? change.info.author.trim() : "";
   const normalizedAuthor = authorCandidate.length > 0 ? authorCandidate : "Unknown";
   const normalizedDate = typeof change.info.date === "string" ? change.info.date.trim() : undefined;
@@ -950,7 +952,7 @@ function serializeTrackedChange(
   change: Insertion | Deletion | MoveFrom | MoveTo,
 ): string {
   const info = change.info;
-  const normalizedId = Number.isInteger(info.id) && info.id >= 0 ? info.id : 0;
+  const normalizedId = normalizeRevisionId(info.id);
   const authorCandidate = typeof info.author === "string" ? info.author.trim() : "";
   const normalizedAuthor = authorCandidate.length > 0 ? authorCandidate : "Unknown";
   const normalizedDate = typeof info.date === "string" ? info.date.trim() : undefined;

packages/core/src/docx/serializer/runSerializer.ts

@@ -37,6 +37,7 @@ import type {
   BlockContent,
   RunPropertyChange,
 } from "../../types/document";
+import { normalizeRevisionId } from "@stll/docx-core/model";
 import { HIGHLIGHT_COLOR_VALUES } from "../../types/documentEnumValues";
 import { isValidHexColor } from "../../utils/colorResolver";
 // oxlint-disable-next-line import/no-cycle -- OOXML model is mutually recursive: shape textboxes hold paragraphs, paragraphs hold runs
@@ -444,7 +445,7 @@ function extractRPrInner(rPrXml: string): string {
 }
 
 function serializeRunPropertyChange(change: RunPropertyChange): string {
-  const normalizedId = Number.isInteger(change.info.id) && change.info.id >= 0 ? change.info.id : 0;
+  const normalizedId = normalizeRevisionId(change.info.id);
   const authorCandidate = typeof change.info.author === "string" ? change.info.author.trim() : "";
   const normalizedAuthor = authorCandidate.length > 0 ? authorCandidate : "Unknown";
   const normalizedDate = typeof change.info.date === "string" ? change.info.date.trim() : undefined;

packages/core/src/docx/serializer/sectionPropertiesSerializer.ts

@@ -6,6 +6,7 @@ import type {
   SectionPropertyChange,
   SectionProperties,
 } from "../../types/document";
+import { normalizeRevisionId } from "@stll/docx-core/model";
 import { getUnserializedSectionPropertyChildNames } from "../sectionParser";
 import { serializeBorder } from "./borderSerializer";
 import { escapeXml, intAttr } from "./xmlUtils";
@@ -303,7 +304,7 @@ function serializeOnOffElement(value: boolean | undefined, name: string): string
 }
 
 function serializeSectionPropertyChange(change: SectionPropertyChange): string {
-  const normalizedId = Number.isInteger(change.info.id) && change.info.id >= 0 ? change.info.id : 0;
+  const normalizedId = normalizeRevisionId(change.info.id);
   const authorCandidate = typeof change.info.author === "string" ? change.info.author.trim() : "";
   const normalizedAuthor = authorCandidate.length > 0 ? authorCandidate : "Unknown";
   const normalizedDate = typeof change.info.date === "string" ? change.info.date.trim() : undefined;

packages/core/src/docx/serializer/tableSerializer.ts

@@ -36,6 +36,7 @@ import type {
   ShadingProperties,
   Paragraph,
 } from "../../types/document";
+import { normalizeRevisionId } from "@stll/docx-core/model";
 import { isValidHexColor } from "../../utils/colorResolver";
 import { serializeBorder } from "./borderSerializer";
 import { escapeXml, intAttr } from "./xmlUtils";
@@ -47,7 +48,7 @@ function normalizeTrackedChangeInfo(info: { id: number; author: string; date?: s
   author: string;
   date?: string;
 } {
-  const normalizedId = Number.isInteger(info.id) && info.id >= 0 ? info.id : 0;
+  const normalizedId = normalizeRevisionId(info.id);
   const authorCandidate = typeof info.author === "string" ? info.author.trim() : "";
   const normalizedAuthor = authorCandidate.length > 0 ? authorCandidate : "Unknown";
   const normalizedDate = typeof info.date === "string" ? info.date.trim() : undefined;

packages/core/src/docx/tableParser.ts

@@ -49,6 +49,7 @@ import type {
   BookmarkEnd,
   BookmarkStart,
 } from "../types/document";
+import { normalizeRevisionId } from "@stll/docx-core/model";
 import { parseBookmarkEnd, parseBookmarkStart } from "./bookmarkParser";
 import {
   appendBookmarkMarkerToLastParagraphInBlocks,
@@ -133,7 +134,9 @@ function parseTrackedChangeInfo(node: XmlElement): TableStructuralChangeInfo["in
   const initials = (getAttribute(node, "w", "initials") ?? "").trim();
 
   const info: TableStructuralChangeInfo["info"] = {
-    id: Number.isInteger(parsedId) && parsedId >= 0 ? parsedId : 0,
+    // `w:id` is attacker-controlled and unbounded in the schema; fold at the
+    // parse boundary (eigenpal #1093).
+    id: normalizeRevisionId(parsedId),
     author: author.length > 0 ? author : "Unknown",
   };
   if (date.length > 0) {

packages/core/src/prosemirror/commentIdAllocator.ts

@@ -13,7 +13,11 @@
  */
 
 import type { EditorView } from "prosemirror-view";
+
+import { MAX_REVISION_ID } from "@stll/docx-core/model";
+
 import type { Comment } from "../types/content";
+import { seedRevisionIdsAbove } from "./plugins/revisionIds";
 
 /** Sentinel ID for a comment that hasn't been persisted yet (anchored to selection). */
 export const PENDING_COMMENT_ID = -1;
@@ -37,9 +41,19 @@ export type CommentIdAllocator = {
 export function createCommentIdAllocator(): CommentIdAllocator {
   let nextId = 1;
   return {
-    next: () => nextId++,
+    next: () => {
+      if (nextId > MAX_REVISION_ID) {
+        nextId = 1;
+      }
+      return nextId++;
+    },
     seedAbove(maxId: number) {
-      if (maxId >= nextId) nextId = maxId + 1;
+      if (!Number.isInteger(maxId) || maxId < 0 || maxId >= MAX_REVISION_ID) {
+        return;
+      }
+      if (maxId >= nextId) {
+        nextId = maxId + 1;
+      }
     },
   };
 }
@@ -58,14 +72,28 @@ export function seedCommentAllocator(
   view: EditorView | null,
 ): void {
   let max = 0;
-  for (const comment of comments ?? []) max = Math.max(max, comment.id);
+  for (const comment of comments ?? []) {
+    if (Number.isInteger(comment.id) && comment.id > max && comment.id <= MAX_REVISION_ID) {
+      max = comment.id;
+    }
+  }
   if (view) {
     view.state.doc.descendants((node) => {
       for (const mark of node.marks) {
-        if (mark.attrs["revisionId"] != null)
-          max = Math.max(max, mark.attrs["revisionId"] as number);
+        const revisionId = mark.attrs["revisionId"];
+        if (
+          typeof revisionId === "number" &&
+          Number.isInteger(revisionId) &&
+          revisionId > max &&
+          revisionId <= MAX_REVISION_ID
+        ) {
+          max = revisionId;
+        }
       }
     });
   }
   allocator.seedAbove(max);
+  // Keep the tracked-revision counter in the same shared OOXML id space so
+  // suggestion-mode mints cannot collide with comment ids (eigenpal #1093).
+  seedRevisionIdsAbove(max);
 }