fix(core): reject a foreign prefix in parseWaitpointId

parseWaitpointId no longer strips an arbitrary <prefix>_ before classifying a body, so a run_ or batch_ id can never be misread as a waitpoint id. deriveWaitpointIdFromAnchor keeps its own prefix-agnostic stripping, since its input is always a known run/batch anchor.
2026-08-21 18:46:55 +01:00
parent 4c1f6e513f
commit d801da8c1a
2 changed files with 44 additions and 10 deletions
@@ -446,7 +446,10 @@ describe("waitpoint ids: run-ops format with version char w", () => {

  it("classifies both the prefixed and the bare form identically", () => {
    const body = generateWaitpointId("MANUAL");
-    expect(parseWaitpointId(body)).toEqual(parseWaitpointId(`waitpoint_${body}`));
+    const bare = parseWaitpointId(body);
+    const prefixed = parseWaitpointId(`waitpoint_${body}`);
+    expect(bare).toEqual(prefixed);
+    expect(bare).toEqual({ format: "b32hexW", type: "MANUAL", timestamp: expect.any(Date) });
  });

  it("recovers the mint timestamp from the core", () => {
@@ -494,6 +497,26 @@ describe("waitpoint ids: run-ops format with version char w", () => {
    expect(parseWaitpointId(generateRunOpsIdV2("7")).format).toBe("legacy");
    expect(parseRunId(`run_${generateWaitpointId("RUN")}`).format).toBe("legacy");
  });
+
+  it("rejects a well-formed waitpoint body wearing a foreign prefix", () => {
+    const body = `${"0".repeat(24)}rw`; // valid core + RUN type char + version w
+    expect(parseWaitpointId(`run_${body}`).format).toBe("legacy");
+    expect(parseWaitpointId(`batch_${body}`).format).toBe("legacy");
+    expect(parseWaitpointId(`waitpoint_${body}`)).toEqual({
+      format: "b32hexW",
+      type: "RUN",
+      timestamp: expect.any(Date),
+    });
+    expect(parseWaitpointId(body).format).toBe("b32hexW");
+  });
+
+  it("handles a bare body that happens to contain an underscore sanely (never throws, never misclassifies)", () => {
+    const body = generateWaitpointId("BATCH");
+    const withUnderscore = `_${body.slice(1)}`;
+    expect(() => parseWaitpointId(withUnderscore)).not.toThrow();
+    // "_" is outside the base32hex alphabet, so this can never be a real waitpoint id.
+    expect(parseWaitpointId(withUnderscore).format).toBe("legacy");
+  });
 });

 describe("deriveWaitpointIdFromAnchor", () => {
@@ -291,7 +291,7 @@ export function deriveWaitpointIdFromAnchor(
  anchorId: string,
  type: WaitpointIdType
 ): string | undefined {
-  const body = stripIdPrefix(anchorId);
+  const body = stripAnchorPrefix(anchorId);
  if (!parseRunOpsIdBody(body) && !parseRunOpsIdV2Body(body)) {
    return undefined;
  }
@@ -300,12 +300,14 @@ export function deriveWaitpointIdFromAnchor(
 }

 /**
- * Classify a waitpoint id. Accepts the prefixed form (`waitpoint_<body>`) and the bare
- * internal form, because both circulate: IdUtil.generate() returns each, and the store's
- * own call sites carry the internal one. Total: never throws.
+ * Classify a waitpoint id. Accepts the prefixed (`waitpoint_<body>`) and bare forms, but
+ * NOT another entity's prefix (`run_`, `batch_`, ...) — this is the discriminator a
+ * later ticket uses to route a possibly customer-supplied id, so a foreign prefix must
+ * classify legacy rather than have its body reinterpreted as a waitpoint id. Total:
+ * never throws.
 */
 export function parseWaitpointId(id: string): ParsedWaitpointId {
-  const body = stripIdPrefix(id);
+  const body = stripWaitpointIdPrefix(id);
  if (body.length !== RUN_OPS_ID_LENGTH) return LEGACY_WAITPOINT_ID;
  if (body[RUN_OPS_ID_VERSION_INDEX] !== WAITPOINT_ID_VERSION) return LEGACY_WAITPOINT_ID;

@@ -318,14 +320,23 @@ export function parseWaitpointId(id: string): ParsedWaitpointId {
  return { format: "b32hexW", type, timestamp };
 }

-// Strip a single leading `<prefix>_` if present, so the friendly and internal forms
-// classify identically. Only the FIRST underscore separates the prefix, mirroring
-// fromFriendlyId's two-part contract.
-function stripIdPrefix(id: string): string {
+// Strip any `<prefix>_` if present. Prefix-agnostic is correct ONLY here: the caller
+// already knows anchorId names a run or batch anchor, so there is no foreign prefix to
+// guard against. Do not reuse for parseWaitpointId — see stripWaitpointIdPrefix.
+function stripAnchorPrefix(id: string): string {
  const underscore = id.indexOf("_");
  return underscore === -1 ? id : id.slice(underscore + 1);
 }

+const WAITPOINT_ID_PREFIX = "waitpoint_";
+
+// Strip the `waitpoint_` prefix if present; any other prefix, or a bare body, is left
+// as-is. Unlike stripAnchorPrefix, this must never strip a foreign prefix down to a body
+// that then happens to pass the run-ops shape check.
+function stripWaitpointIdPrefix(id: string): string {
+  return id.startsWith(WAITPOINT_ID_PREFIX) ? id.slice(WAITPOINT_ID_PREFIX.length) : id;
+}
+
 export function generateInternalId(): string {
  return cuid();
 }