Merge pull request #204 from modelcontextprotocol/fix/eocd-comment-length

fix: update ZIP EOCD comment_length when signing

Author: tobinsouth

src/node/sign.ts

@@ -89,8 +89,20 @@ export function signMcpbFile(
   // Create signature block with PKCS#7 data
   const signatureBlock = createSignatureBlock(pkcs7Signature);
 
+  // Update ZIP EOCD comment_length to include signature block
+  // This ensures strict ZIP parsers accept the signed file
+  const updatedContent = Buffer.from(mcpbContent);
+  const eocdOffset = findEocdOffset(updatedContent);
+  if (eocdOffset !== -1) {
+    const currentCommentLength = updatedContent.readUInt16LE(eocdOffset + 20);
+    updatedContent.writeUInt16LE(
+      currentCommentLength + signatureBlock.length,
+      eocdOffset + 20,
+    );
+  }
+
   // Append signature block to MCPB file
-  const signedContent = Buffer.concat([mcpbContent, signatureBlock]);
+  const signedContent = Buffer.concat([updatedContent, signatureBlock]);
   writeFileSync(mcpbPath, signedContent);
 }
 
@@ -218,6 +230,20 @@ export async function verifyMcpbFile(
   }
 }
 
+/**
+ * Finds the offset of the ZIP End of Central Directory record
+ * by scanning backwards for the EOCD magic bytes (0x06054b50)
+ */
+function findEocdOffset(buffer: Buffer): number {
+  // EOCD is at least 22 bytes, scan backwards from the end
+  for (let i = buffer.length - 22; i >= 0; i--) {
+    if (buffer.readUInt32LE(i) === 0x06054b50) {
+      return i;
+    }
+  }
+  return -1;
+}
+
 /**
  * Creates a signature block buffer with PKCS#7 signature
  */

test/sign.e2e.test.ts

@@ -468,4 +468,50 @@ describe("MCPB Signing E2E Tests", () => {
   it("should remove signatures", async () => {
     await testSignatureRemoval();
   });
+
+  it("should update EOCD comment_length after signing", async () => {
+    const testFile = path.join(TEST_DIR, "test-eocd.mcpb");
+    fs.copyFileSync(TEST_MCPB, testFile);
+
+    // Read original EOCD comment_length
+    const originalContent = fs.readFileSync(testFile);
+    let eocdOffset = -1;
+    for (let i = originalContent.length - 22; i >= 0; i--) {
+      if (originalContent.readUInt32LE(i) === 0x06054b50) {
+        eocdOffset = i;
+        break;
+      }
+    }
+    expect(eocdOffset).toBeGreaterThanOrEqual(0);
+    const originalCommentLength = originalContent.readUInt16LE(eocdOffset + 20);
+    expect(originalCommentLength).toBe(0); // Fresh ZIP has no comment
+
+    // Sign the file
+    signMcpbFile(testFile, SELF_SIGNED_CERT, SELF_SIGNED_KEY);
+
+    // Read signed file and verify EOCD comment_length was updated
+    const signedContent = fs.readFileSync(testFile);
+    let signedEocdOffset = -1;
+    for (let i = signedContent.length - 22; i >= 0; i--) {
+      if (signedContent.readUInt32LE(i) === 0x06054b50) {
+        signedEocdOffset = i;
+        break;
+      }
+    }
+    expect(signedEocdOffset).toBeGreaterThanOrEqual(0);
+    const signedCommentLength = signedContent.readUInt16LE(
+      signedEocdOffset + 20,
+    );
+
+    // Comment length should equal everything after the EOCD record's original end
+    const eocdMinSize = 22; // minimum EOCD size (no comment)
+    const dataAfterEocd =
+      signedContent.length -
+      (signedEocdOffset + eocdMinSize + originalCommentLength);
+    expect(signedCommentLength).toBe(dataAfterEocd);
+    expect(signedCommentLength).toBeGreaterThan(0);
+
+    // Clean up
+    fs.unlinkSync(testFile);
+  });
 });