fix(security): path traversal via unsanitized nid + integrity proof serialization mismatch

python/numbersprotocol_capture/client.py

@@ -6,9 +6,10 @@
 
 import json
 import mimetypes
+import re
 from pathlib import Path
 from typing import Any
-from urllib.parse import urlencode
+from urllib.parse import quote, urlencode
 
 import httpx
 
@@ -36,6 +37,16 @@
 ASSET_SEARCH_API_URL = "https://us-central1-numbers-protocol-api.cloudfunctions.net/asset-search"
 NFT_SEARCH_API_URL = "https://eofveg1f59hrbn.m.pipedream.net"
 
+# Valid NID characters: alphanumeric only (matches IPFS CID format)
+_NID_PATTERN = re.compile(r"^[a-zA-Z0-9]+$")
+
+
+def _validate_nid_format(nid: str) -> None:
+    """Validates that a NID contains only safe alphanumeric characters."""
+    if not _NID_PATTERN.match(nid):
+        raise ValidationError("nid contains invalid characters")
+
+
 # Common MIME types by extension
 MIME_TYPES: dict[str, str] = {
     "jpg": "image/jpeg",
@@ -362,6 +373,7 @@ def update(
         """
         if not nid:
             raise ValidationError("nid is required")
+        _validate_nid_format(nid)
 
         # Build options from args if not provided
         if options is None:
@@ -388,7 +400,8 @@ def update(
 
         response = self._request(
             "PATCH",
-            f"{self._base_url}/assets/{nid}/",
+            # quote() provides defense-in-depth alongside format validation
+            f"{self._base_url}/assets/{quote(nid, safe='')}/",
             data=form_data,
             nid=nid,
         )
@@ -412,10 +425,12 @@ def get(self, nid: str) -> Asset:
         """
         if not nid:
             raise ValidationError("nid is required")
+        _validate_nid_format(nid)
 
         response = self._request(
             "GET",
-            f"{self._base_url}/assets/{nid}/",
+            # quote() provides defense-in-depth alongside format validation
+            f"{self._base_url}/assets/{quote(nid, safe='')}/",
             nid=nid,
         )
 

python/tests/test_client.py

@@ -76,3 +76,36 @@ def test_update_empty_nid_raises_error(self) -> None:
         with Capture(token="test-token") as capture:
             with pytest.raises(ValidationError, match="nid is required"):
                 capture.update("", caption="test")
+
+
+class TestNidFormatValidation:
+    """Tests for NID format validation (path traversal prevention)."""
+
+    def test_get_rejects_path_traversal_nid(self) -> None:
+        """Test that nid with path traversal characters is rejected."""
+        with Capture(token="test-token") as capture:
+            with pytest.raises(ValidationError, match="nid contains invalid characters"):
+                capture.get("../../admin/users")
+
+    def test_update_rejects_path_traversal_nid(self) -> None:
+        """Test that nid with path traversal characters is rejected in update."""
+        with Capture(token="test-token") as capture:
+            with pytest.raises(ValidationError, match="nid contains invalid characters"):
+                capture.update("../../admin/users", caption="x")
+
+    def test_get_rejects_nid_with_url_special_chars(self) -> None:
+        """Test that nid with URL-special characters is rejected."""
+        with Capture(token="test-token") as capture:
+            with pytest.raises(ValidationError, match="nid contains invalid characters"):
+                capture.get("bafybei?inject=1")
+            with pytest.raises(ValidationError, match="nid contains invalid characters"):
+                capture.get("bafybei#fragment")
+
+    def test_get_accepts_valid_alphanumeric_nid(self) -> None:
+        """Test that a valid alphanumeric NID passes format validation."""
+        valid_nid = "bafybeif3mhxhkhfwuszl2lybtai3hz3q6naqpfisd4q55mcc7opkmiv5ei"
+        with Capture(token="test-token") as capture:
+            # Should not raise ValidationError for format; will fail at network level
+            with pytest.raises(Exception) as exc_info:
+                capture.get(valid_nid)
+            assert "nid contains invalid characters" not in str(exc_info.value)

python/tests/test_crypto.py

@@ -1,7 +1,10 @@
 """Tests for crypto utilities."""
 
+import json
+
 from numbersprotocol_capture import sha256, verify_signature
 from numbersprotocol_capture.crypto import create_integrity_proof, sign_integrity_proof
+from numbersprotocol_capture.types import IntegrityProof
 
 
 class TestSha256:
@@ -100,3 +103,49 @@ def test_verify_wrong_address(self) -> None:
             signature.signature,
             wrong_address,
         )
+
+
+class TestIntegrityProofSerialization:
+    """Tests for integrity proof JSON serialization consistency."""
+
+    def test_sign_integrity_proof_uses_only_three_keys(self) -> None:
+        """Test that sign_integrity_proof serializes exactly three keys in expected order."""
+        proof = IntegrityProof(
+            proof_hash="abc123",
+            asset_mime_type="image/jpeg",
+            created_at=1700000000000,
+        )
+        # Manually reproduce the expected JSON (same as the implementation)
+        expected_json = json.dumps(
+            {
+                "proof_hash": proof.proof_hash,
+                "asset_mime_type": proof.asset_mime_type,
+                "created_at": proof.created_at,
+            },
+            separators=(",", ":"),
+        )
+        expected_sha = sha256(expected_json.encode("utf-8"))
+
+        private_key = "0x0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef"
+        signature = sign_integrity_proof(proof, private_key)
+
+        assert signature.integrity_sha == expected_sha
+
+    def test_sign_integrity_proof_ignores_extra_fields(self) -> None:
+        """Test that two identical proofs produce identical integrity_sha values."""
+        proof1 = IntegrityProof(
+            proof_hash="abc123",
+            asset_mime_type="image/jpeg",
+            created_at=1700000000000,
+        )
+        proof2 = IntegrityProof(
+            proof_hash="abc123",
+            asset_mime_type="image/jpeg",
+            created_at=1700000000000,
+        )
+        private_key = "0x0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef"
+
+        sig1 = sign_integrity_proof(proof1, private_key)
+        sig2 = sign_integrity_proof(proof2, private_key)
+
+        assert sig1.integrity_sha == sig2.integrity_sha

ts/src/client.test.ts

@@ -337,3 +337,41 @@ describe('Asset Search Validation', () => {
     ).rejects.toThrow('sampleCount must be a positive integer')
   })
 })
+
+describe('NID Format Validation', () => {
+  it('should reject nid with path traversal characters in get()', async () => {
+    const capture = new Capture({ token: 'test-token' })
+
+    await expect(capture.get('../../admin/users')).rejects.toThrow(
+      'nid contains invalid characters'
+    )
+  })
+
+  it('should reject nid with path traversal characters in update()', async () => {
+    const capture = new Capture({ token: 'test-token' })
+
+    await expect(capture.update('../../admin/users', { caption: 'x' })).rejects.toThrow(
+      'nid contains invalid characters'
+    )
+  })
+
+  it('should reject nid with URL-special characters', async () => {
+    const capture = new Capture({ token: 'test-token' })
+
+    await expect(capture.get('bafybei?inject=1')).rejects.toThrow(
+      'nid contains invalid characters'
+    )
+    await expect(capture.get('bafybei#fragment')).rejects.toThrow(
+      'nid contains invalid characters'
+    )
+  })
+
+  it('should accept valid alphanumeric nid (format validation only, not network)', async () => {
+    const capture = new Capture({ token: 'test-token' })
+
+    // This should not throw a ValidationError for format — it will fail at network level
+    await expect(capture.get(TEST_NID)).rejects.not.toThrow(
+      'nid contains invalid characters'
+    )
+  })
+})

ts/src/client.ts

@@ -22,6 +22,20 @@ import {
 import { sha256, createIntegrityProof, signIntegrityProof } from './crypto.js'
 
 const DEFAULT_BASE_URL = 'https://api.numbersprotocol.io/api/v3'
+
+/** Valid NID characters: alphanumeric only (matches IPFS CID format) */
+const NID_PATTERN = /^[a-zA-Z0-9]+$/
+
+/**
+ * Validates that a NID contains only safe alphanumeric characters.
+ * @internal
+ */
+function validateNidFormat(nid: string): void {
+  if (!NID_PATTERN.test(nid)) {
+    throw new ValidationError('nid contains invalid characters')
+  }
+}
+
 const HISTORY_API_URL =
   'https://e23hi68y55.execute-api.us-east-1.amazonaws.com/default/get-commits-storage-backend-jade-near'
 const MERGE_TREE_API_URL =
@@ -286,6 +300,7 @@ export class Capture {
     if (!nid) {
       throw new ValidationError('nid is required')
     }
+    validateNidFormat(nid)
 
     if (options.headline && options.headline.length > 25) {
       throw new ValidationError('headline must be 25 characters or less')
@@ -308,7 +323,8 @@ export class Capture {
 
     const response = await this.request<AssetApiResponse>(
       'PATCH',
-      `${this.baseUrl}/assets/${nid}/`,
+      // encodeURIComponent provides defense-in-depth alongside format validation
+      `${this.baseUrl}/assets/${encodeURIComponent(nid)}/`,
       formData,
       nid
     )
@@ -333,10 +349,12 @@ export class Capture {
     if (!nid) {
       throw new ValidationError('nid is required')
     }
+    validateNidFormat(nid)
 
     const response = await this.request<AssetApiResponse>(
       'GET',
-      `${this.baseUrl}/assets/${nid}/`,
+      // encodeURIComponent provides defense-in-depth alongside format validation
+      `${this.baseUrl}/assets/${encodeURIComponent(nid)}/`,
       undefined,
       nid
     )

ts/src/crypto.ts

@@ -41,7 +41,11 @@ export async function signIntegrityProof(
   const wallet = new Wallet(privateKey)
 
   // Compute integrity hash of the signed metadata JSON
-  const proofJson = JSON.stringify(proof)
+  const proofJson = JSON.stringify({
+    proof_hash: proof.proof_hash,
+    asset_mime_type: proof.asset_mime_type,
+    created_at: proof.created_at,
+  })
   const proofBytes = new TextEncoder().encode(proofJson)
   const integritySha = await sha256(proofBytes)