diff --git a/python/numbersprotocol_capture/client.py b/python/numbersprotocol_capture/client.py index 529391a..c0c3b27 100644 --- a/python/numbersprotocol_capture/client.py +++ b/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, ) diff --git a/python/tests/test_client.py b/python/tests/test_client.py index 8a081c0..654d9d7 100644 --- a/python/tests/test_client.py +++ b/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) diff --git a/python/tests/test_crypto.py b/python/tests/test_crypto.py index 61b55bc..e190ff2 100644 --- a/python/tests/test_crypto.py +++ b/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 diff --git a/ts/src/client.test.ts b/ts/src/client.test.ts index e1e50fb..e2e350d 100644 --- a/ts/src/client.test.ts +++ b/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' + ) + }) +}) diff --git a/ts/src/client.ts b/ts/src/client.ts index c86440e..fc5ceba 100644 --- a/ts/src/client.ts +++ b/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( '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( 'GET', - `${this.baseUrl}/assets/${nid}/`, + // encodeURIComponent provides defense-in-depth alongside format validation + `${this.baseUrl}/assets/${encodeURIComponent(nid)}/`, undefined, nid ) diff --git a/ts/src/crypto.ts b/ts/src/crypto.ts index de214ab..b82762f 100644 --- a/ts/src/crypto.ts +++ b/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)