Parcourir la source

fix: improve signature verification error handling #14

- Add length check before timing-safe comparison to prevent buffer length mismatch errors
- Provide more specific error message for invalid signature format
- Add comprehensive test suite (test-signature-verification.js) to verify signature verification works correctly
- All tests pass: valid signatures accepted (200), invalid/missing signatures rejected (401)

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
Claude il y a 9 mois
Parent
commit
05ec4a0045
2 fichiers modifiés avec 167 ajouts et 0 suppressions
  1. 6 0
      src/server.js
  2. 161 0
      test-signature-verification.js

+ 6 - 0
src/server.js

@@ -62,6 +62,12 @@ export class WebhookServer {
       hmac.update(rawPayload, 'utf8');
       const computedSignature = hmac.digest('hex');
 
+      // Check if signatures have same length before timing-safe comparison
+      if (signature.length !== computedSignature.length) {
+        logger.error('Signature verification failed: invalid signature format');
+        return false;
+      }
+
       // Compare signatures using timing-safe comparison
       const isValid = crypto.timingSafeEqual(
         Buffer.from(signature),

+ 161 - 0
test-signature-verification.js

@@ -0,0 +1,161 @@
+#!/usr/bin/env node
+
+/**
+ * Test script for webhook signature verification
+ * Tests both valid and invalid signatures to ensure security is working
+ */
+
+import crypto from 'crypto';
+import http from 'http';
+
+const TEST_PORT = 3099;
+const TEST_SECRET = 'test-secret-12345';
+
+// Test payload
+const payload = JSON.stringify({
+  ref: 'refs/heads/main',
+  repository: {
+    name: 'test-repo',
+    full_name: 'user/test-repo'
+  },
+  pusher: {
+    username: 'testuser'
+  }
+});
+
+// Generate valid signature
+function generateSignature(payload, secret) {
+  const hmac = crypto.createHmac('sha256', secret);
+  hmac.update(payload, 'utf8');
+  return hmac.digest('hex');
+}
+
+// Send webhook request
+async function sendWebhook(port, payload, signature, description) {
+  return new Promise((resolve, reject) => {
+    const options = {
+      hostname: '127.0.0.1',
+      port: port,
+      path: '/webhook',
+      method: 'POST',
+      headers: {
+        'Content-Type': 'application/json',
+        'Content-Length': Buffer.byteLength(payload),
+        'X-Gogs-Event': 'push',
+        'X-Gogs-Delivery': '12345'
+      }
+    };
+
+    if (signature) {
+      options.headers['X-Gogs-Signature'] = signature;
+    }
+
+    const req = http.request(options, (res) => {
+      let data = '';
+      res.on('data', chunk => data += chunk);
+      res.on('end', () => {
+        resolve({
+          statusCode: res.statusCode,
+          body: data,
+          description
+        });
+      });
+    });
+
+    req.on('error', reject);
+    req.write(payload);
+    req.end();
+  });
+}
+
+// Start test server
+async function startTestServer() {
+  const { WebhookServer } = await import('./src/server.js');
+  const server = new WebhookServer({
+    host: '127.0.0.1',
+    port: TEST_PORT,
+    secret: TEST_SECRET
+  });
+
+  return new Promise((resolve) => {
+    server.start();
+    // Give server time to start
+    setTimeout(() => resolve(server), 500);
+  });
+}
+
+// Run tests
+async function runTests() {
+  console.log('🧪 Starting signature verification tests...\n');
+
+  const server = await startTestServer();
+
+  try {
+    // Test 1: Valid signature
+    const validSignature = generateSignature(payload, TEST_SECRET);
+    const test1 = await sendWebhook(
+      TEST_PORT,
+      payload,
+      validSignature,
+      'Valid signature'
+    );
+
+    console.log(`✓ Test 1: ${test1.description}`);
+    console.log(`  Status: ${test1.statusCode} ${test1.statusCode === 200 ? '✓ PASS' : '✗ FAIL'}`);
+    console.log(`  Response: ${test1.body}\n`);
+
+    // Test 2: Invalid signature
+    const test2 = await sendWebhook(
+      TEST_PORT,
+      payload,
+      'invalid-signature-123',
+      'Invalid signature'
+    );
+
+    console.log(`✓ Test 2: ${test2.description}`);
+    console.log(`  Status: ${test2.statusCode} ${test2.statusCode === 401 ? '✓ PASS' : '✗ FAIL'}`);
+    console.log(`  Response: ${test2.body}\n`);
+
+    // Test 3: Missing signature
+    const test3 = await sendWebhook(
+      TEST_PORT,
+      payload,
+      null,
+      'Missing signature'
+    );
+
+    console.log(`✓ Test 3: ${test3.description}`);
+    console.log(`  Status: ${test3.statusCode} ${test3.statusCode === 401 ? '✓ PASS' : '✗ FAIL'}`);
+    console.log(`  Response: ${test3.body}\n`);
+
+    // Summary
+    const results = [test1, test2, test3];
+    const passed = results.filter((r, i) => {
+      if (i === 0) return r.statusCode === 200;
+      return r.statusCode === 401;
+    }).length;
+
+    console.log('═══════════════════════════════════════');
+    console.log(`Results: ${passed}/3 tests passed`);
+    console.log('═══════════════════════════════════════\n');
+
+    if (passed === 3) {
+      console.log('✅ All signature verification tests passed!');
+      console.log('   - Valid signatures are accepted (200)');
+      console.log('   - Invalid signatures are rejected (401)');
+      console.log('   - Missing signatures are rejected (401)');
+    } else {
+      console.log('❌ Some tests failed!');
+      process.exitCode = 1;
+    }
+
+  } finally {
+    await server.stop();
+    console.log('\n🛑 Test server stopped');
+  }
+}
+
+runTests().catch(err => {
+  console.error('❌ Test error:', err);
+  process.exit(1);
+});