diff --git a/.pmat/baseline.json b/.pmat/baseline.json index b9fc95b9..99578aab 100644 --- a/.pmat/baseline.json +++ b/.pmat/baseline.json @@ -1,6 +1,6 @@ { - "version": "3.27.0", - "created_at": "2026-07-29T06:10:44.211143673Z", + "version": "3.28.2", + "created_at": "2026-07-30T09:05:03.427036698Z", "git_context": null, "files": { "./benches/cli_bench.rs": { @@ -3819,38 +3819,38 @@ }, "./examples/crypto_mcdc_undo_webhook.rs": { "content_hash": [ - 55, - 36, - 15, - 46, - 67, - 142, + 143, + 133, + 131, + 225, 254, - 242, - 176, - 253, - 232, + 225, + 90, + 136, + 68, + 12, + 22, + 153, + 59, + 163, + 83, + 121, + 178, + 1, + 64, + 187, + 113, + 205, 41, - 95, - 87, + 132, + 210, + 152, + 136, + 15, 49, - 115, - 189, - 89, - 70, - 115, - 202, - 11, - 29, - 243, - 92, - 158, - 3, - 56, - 245, - 216, - 148, - 48 + 94, + 167, + 64 ], "score": { "structural_complexity": 25.0, @@ -3859,8 +3859,8 @@ "coupling_score": 15.0, "doc_coverage": 10.0, "consistency_score": 10.0, - "entropy_score": 6.0, - "total": 96.36363, + "entropy_score": 5.5, + "total": 95.90909, "grade": "AMinus", "confidence": 1.0, "language": "Rust", @@ -3868,11 +3868,11 @@ "penalties_applied": [ { "source_metric": "Duplication", - "amount": 4.0, + "amount": 4.5, "applied_to": [ "Duplication" ], - "issue": "Found 8 duplicate code patterns" + "issue": "Found 9 duplicate code patterns" } ], "critical_defects_count": 0, @@ -6864,38 +6864,38 @@ }, "./examples/migrate_webhook.rs": { "content_hash": [ - 24, - 103, - 106, - 238, - 253, - 203, - 243, - 115, - 59, - 58, - 232, - 220, - 150, - 8, - 248, - 201, - 149, - 182, - 43, - 100, - 8, - 212, - 15, - 75, - 228, - 55, - 155, - 82, - 130, + 202, + 153, + 64, + 35, + 156, + 162, + 112, + 134, + 21, + 196, + 217, + 2, + 221, + 214, + 25, + 70, + 120, + 1, + 112, + 181, 138, + 78, + 84, + 162, + 124, + 127, 2, - 101 + 97, + 255, + 122, + 64, + 97 ], "score": { "structural_complexity": 25.0, @@ -14188,38 +14188,38 @@ }, "./examples/webhook_source.rs": { "content_hash": [ - 199, - 66, - 90, + 54, + 73, 80, - 75, - 25, - 158, - 154, - 102, 74, - 223, - 207, - 34, - 86, - 65, - 253, - 51, - 224, - 11, - 115, - 81, - 111, - 78, - 97, - 197, - 134, - 4, - 36, - 227, 154, - 190, - 190 + 215, + 23, + 125, + 219, + 202, + 55, + 126, + 246, + 132, + 245, + 68, + 253, + 77, + 94, + 6, + 13, + 9, + 88, + 110, + 8, + 245, + 188, + 192, + 40, + 67, + 77, + 153 ], "score": { "structural_complexity": 25.0, @@ -17262,48 +17262,48 @@ }, "./src/cli/commands/subcmd_args.rs": { "content_hash": [ - 110, - 76, - 177, - 30, + 115, + 176, + 220, + 149, + 44, + 173, + 123, + 83, + 131, + 149, + 153, 67, - 85, - 94, - 122, - 63, - 168, - 119, - 8, - 136, - 223, - 211, - 159, - 207, - 55, - 177, - 88, - 21, - 51, - 196, - 81, - 130, - 63, - 57, - 158, - 101, + 164, + 106, + 144, + 23, + 213, + 226, + 225, + 54, + 128, + 105, + 249, + 156, + 0, + 27, 253, - 238, - 99 + 220, + 47, + 78, + 250, + 48 ], "score": { "structural_complexity": 25.0, "semantic_complexity": 20.0, - "duplication_ratio": 10.418251, + "duplication_ratio": 10.596491, "coupling_score": 15.0, "doc_coverage": 10.0, "consistency_score": 10.0, "entropy_score": 5.0, - "total": 95.41825, + "total": 95.59649, "grade": "AMinus", "confidence": 1.0, "language": "Rust", @@ -17319,11 +17319,11 @@ }, { "source_metric": "Duplication", - "amount": 9.581749, + "amount": 9.403509, "applied_to": [ "Duplication" ], - "issue": "Code duplication: 47.9%" + "issue": "Code duplication: 47.0%" } ], "critical_defects_count": 0, @@ -26626,49 +26626,49 @@ }, "./src/cli/lock_core.rs": { "content_hash": [ - 232, - 233, - 96, - 197, - 179, 61, - 246, - 153, - 219, - 190, + 30, + 5, 18, - 196, - 34, - 8, - 40, - 114, + 133, + 226, + 247, + 157, + 222, + 255, + 47, + 30, + 82, + 216, + 45, + 217, + 247, + 170, + 75, + 204, + 20, 246, - 129, + 114, + 162, + 26, + 221, + 59, + 62, 231, - 203, - 147, - 116, - 117, - 2, - 13, - 208, - 28, - 2, - 179, - 27, - 194, - 86 + 53, + 97, + 80 ], "score": { - "structural_complexity": 0.0, + "structural_complexity": 2.5, "semantic_complexity": 19.0, - "duplication_ratio": 17.732426, + "duplication_ratio": 20.0, "coupling_score": 15.0, "doc_coverage": 10.0, "consistency_score": 10.0, "entropy_score": 5.0, - "total": 76.73242, - "grade": "B", + "total": 81.5, + "grade": "BPlus", "confidence": 1.0, "language": "Rust", "file_path": "./src/cli/lock_core.rs", @@ -26679,15 +26679,7 @@ "applied_to": [ "Duplication" ], - "issue": "Found 49 duplicate code patterns" - }, - { - "source_metric": "Duplication", - "amount": 2.2675736, - "applied_to": [ - "Duplication" - ], - "issue": "Code duplication: 11.3%" + "issue": "Found 39 duplicate code patterns" }, { "source_metric": "StructuralComplexity", @@ -26695,15 +26687,15 @@ "applied_to": [ "StructuralComplexity" ], - "issue": "High cognitive complexity: 102" + "issue": "High cognitive complexity: 93" }, { "source_metric": "StructuralComplexity", - "amount": 15.0, + "amount": 12.5, "applied_to": [ "StructuralComplexity" ], - "issue": "High cyclomatic complexity: 61" + "issue": "High cyclomatic complexity: 55" }, { "source_metric": "SemanticComplexity", @@ -26996,6 +26988,93 @@ }, "git_context": null }, + "./src/cli/lock_output.rs": { + "content_hash": [ + 68, + 186, + 198, + 19, + 133, + 145, + 171, + 216, + 167, + 57, + 121, + 111, + 18, + 230, + 236, + 35, + 238, + 2, + 48, + 60, + 29, + 77, + 43, + 240, + 77, + 155, + 24, + 108, + 156, + 164, + 89, + 36 + ], + "score": { + "structural_complexity": 25.0, + "semantic_complexity": 19.5, + "duplication_ratio": 16.55914, + "coupling_score": 15.0, + "doc_coverage": 10.0, + "consistency_score": 10.0, + "entropy_score": 5.0, + "total": 91.87195, + "grade": "AMinus", + "confidence": 1.0, + "language": "Rust", + "file_path": "./src/cli/lock_output.rs", + "penalties_applied": [ + { + "source_metric": "Duplication", + "amount": 5.0, + "applied_to": [ + "Duplication" + ], + "issue": "Found 16 duplicate code patterns" + }, + { + "source_metric": "Duplication", + "amount": 3.4408603, + "applied_to": [ + "Duplication" + ], + "issue": "Code duplication: 17.2%" + }, + { + "source_metric": "SemanticComplexity", + "amount": 0.5, + "applied_to": [ + "SemanticComplexity" + ], + "issue": "Too many parameters: 6" + } + ], + "critical_defects_count": 0, + "has_critical_defects": false, + "has_contract_coverage": false + }, + "components": { + "complexity_breakdown": {}, + "duplication_sources": [], + "coupling_dependencies": [], + "doc_missing_items": [], + "consistency_violations": [] + }, + "git_context": null + }, "./src/cli/lock_repair.rs": { "content_hash": [ 34, @@ -27756,38 +27835,38 @@ }, "./src/cli/mod.rs": { "content_hash": [ - 172, - 227, - 234, - 66, - 128, - 216, - 44, - 63, - 133, - 196, - 35, - 3, - 116, - 214, + 25, + 57, + 86, + 151, + 211, + 164, + 157, + 15, 11, - 182, - 146, - 200, - 245, - 139, - 137, - 162, - 134, - 243, - 58, - 169, - 239, - 132, - 159, - 26, - 180, - 230 + 77, + 207, + 177, + 135, + 127, + 214, + 222, + 184, + 156, + 131, + 65, + 89, + 95, + 135, + 62, + 119, + 70, + 206, + 14, + 56, + 103, + 183, + 155 ], "score": { "structural_complexity": 25.0, @@ -29138,48 +29217,48 @@ }, "./src/cli/prove.rs": { "content_hash": [ - 163, - 7, - 240, - 154, + 99, 50, - 207, + 153, + 196, + 134, + 52, + 29, + 140, + 90, + 238, 46, - 231, - 214, - 182, - 47, - 14, - 236, - 215, - 198, - 86, - 141, - 203, - 50, - 5, - 81, - 68, - 101, - 7, - 139, - 138, + 91, + 148, + 110, + 19, + 26, + 11, 58, + 34, + 22, + 215, + 214, + 74, + 245, + 199, + 212, + 12, + 19, 224, - 154, - 247, - 84, - 164 + 203, + 0, + 127 ], "score": { "structural_complexity": 8.0, "semantic_complexity": 20.0, - "duplication_ratio": 17.0922, + "duplication_ratio": 17.152779, "coupling_score": 15.0, "doc_coverage": 10.0, "consistency_score": 10.0, "entropy_score": 5.0, - "total": 85.0922, + "total": 85.15278, "grade": "AMinus", "confidence": 1.0, "language": "Rust", @@ -29195,11 +29274,11 @@ }, { "source_metric": "Duplication", - "amount": 2.9078012, + "amount": 2.847222, "applied_to": [ "Duplication" ], - "issue": "Code duplication: 14.5%" + "issue": "Code duplication: 14.2%" }, { "source_metric": "StructuralComplexity", @@ -29897,48 +29976,48 @@ }, "./src/cli/rules.rs": { "content_hash": [ - 107, - 241, - 27, - 59, - 161, - 124, - 11, - 166, - 139, - 215, - 50, - 53, - 120, - 186, - 71, - 107, + 170, + 47, + 210, + 143, + 246, + 239, + 80, + 12, + 112, + 225, + 142, + 99, + 48, + 98, + 223, + 145, + 7, + 162, + 135, + 72, + 142, + 18, + 123, + 106, + 224, + 192, + 43, + 216, 209, - 4, - 78, - 60, - 214, - 228, - 65, - 35, - 156, - 148, - 208, - 196, - 177, - 177, - 156, - 174 + 193, + 90, + 96 ], "score": { "structural_complexity": 25.0, "semantic_complexity": 20.0, - "duplication_ratio": 17.058823, + "duplication_ratio": 17.167381, "coupling_score": 15.0, "doc_coverage": 10.0, "consistency_score": 10.0, "entropy_score": 5.0, - "total": 92.78075, + "total": 92.87944, "grade": "AMinus", "confidence": 1.0, "language": "Rust", @@ -29950,15 +30029,94 @@ "applied_to": [ "Duplication" ], - "issue": "Found 29 duplicate code patterns" + "issue": "Found 35 duplicate code patterns" }, { "source_metric": "Duplication", - "amount": 2.9411767, + "amount": 2.832618, "applied_to": [ "Duplication" ], - "issue": "Code duplication: 14.7%" + "issue": "Code duplication: 14.2%" + } + ], + "critical_defects_count": 0, + "has_critical_defects": false, + "has_contract_coverage": false + }, + "components": { + "complexity_breakdown": {}, + "duplication_sources": [], + "coupling_dependencies": [], + "doc_missing_items": [], + "consistency_violations": [] + }, + "git_context": null + }, + "./src/cli/rules_serve.rs": { + "content_hash": [ + 243, + 65, + 249, + 234, + 135, + 38, + 223, + 68, + 125, + 5, + 61, + 72, + 57, + 242, + 21, + 241, + 231, + 97, + 67, + 212, + 31, + 34, + 196, + 214, + 196, + 159, + 90, + 36, + 174, + 150, + 202, + 15 + ], + "score": { + "structural_complexity": 25.0, + "semantic_complexity": 19.0, + "duplication_ratio": 20.0, + "coupling_score": 15.0, + "doc_coverage": 10.0, + "consistency_score": 10.0, + "entropy_score": 5.0, + "total": 94.545456, + "grade": "AMinus", + "confidence": 1.0, + "language": "Rust", + "file_path": "./src/cli/rules_serve.rs", + "penalties_applied": [ + { + "source_metric": "Duplication", + "amount": 5.0, + "applied_to": [ + "Duplication" + ], + "issue": "Found 10 duplicate code patterns" + }, + { + "source_metric": "SemanticComplexity", + "amount": 1.0, + "applied_to": [ + "SemanticComplexity" + ], + "issue": "Too many parameters: 7" } ], "critical_defects_count": 0, @@ -42567,48 +42725,48 @@ }, "./src/core/mod.rs": { "content_hash": [ - 138, - 190, - 246, - 61, - 32, - 29, - 72, - 146, - 146, - 251, - 100, - 162, - 45, - 158, + 27, + 22, + 161, + 204, + 137, + 99, + 74, + 174, 207, - 48, - 16, - 134, - 85, 116, - 198, - 151, - 89, - 41, - 58, - 35, - 66, - 112, - 157, + 238, + 108, + 32, + 242, + 192, + 77, + 236, + 46, 169, - 20, - 112 + 143, + 56, + 167, + 119, + 81, + 227, + 199, + 149, + 103, + 132, + 181, + 0, + 157 ], "score": { "structural_complexity": 25.0, "semantic_complexity": 20.0, - "duplication_ratio": 17.272728, + "duplication_ratio": 16.875, "coupling_score": 15.0, "doc_coverage": 10.0, "consistency_score": 10.0, "entropy_score": 9.0, - "total": 96.61157, + "total": 96.25, "grade": "AMinus", "confidence": 1.0, "language": "Rust", @@ -42624,11 +42782,11 @@ }, { "source_metric": "Duplication", - "amount": 2.7272727, + "amount": 3.125, "applied_to": [ "Duplication" ], - "issue": "Code duplication: 13.6%" + "issue": "Code duplication: 15.6%" } ], "critical_defects_count": 0, @@ -55560,54 +55718,228 @@ }, "git_context": null }, + "./src/core/webhook_http.rs": { + "content_hash": [ + 28, + 160, + 117, + 124, + 101, + 112, + 118, + 231, + 195, + 205, + 80, + 96, + 221, + 113, + 124, + 98, + 241, + 180, + 96, + 60, + 129, + 70, + 73, + 245, + 112, + 185, + 74, + 134, + 46, + 84, + 155, + 119 + ], + "score": { + "structural_complexity": 12.299999, + "semantic_complexity": 20.0, + "duplication_ratio": 17.558685, + "coupling_score": 15.0, + "doc_coverage": 10.0, + "consistency_score": 10.0, + "entropy_score": 5.0, + "total": 89.85869, + "grade": "AMinus", + "confidence": 1.0, + "language": "Rust", + "file_path": "./src/core/webhook_http.rs", + "penalties_applied": [ + { + "source_metric": "Duplication", + "amount": 5.0, + "applied_to": [ + "Duplication" + ], + "issue": "Found 16 duplicate code patterns" + }, + { + "source_metric": "Duplication", + "amount": 2.4413147, + "applied_to": [ + "Duplication" + ], + "issue": "Code duplication: 12.2%" + }, + { + "source_metric": "StructuralComplexity", + "amount": 5.7000003, + "applied_to": [ + "StructuralComplexity" + ], + "issue": "High cognitive complexity: 34" + }, + { + "source_metric": "StructuralComplexity", + "amount": 7.0, + "applied_to": [ + "StructuralComplexity" + ], + "issue": "High cyclomatic complexity: 44" + } + ], + "critical_defects_count": 0, + "has_critical_defects": false, + "has_contract_coverage": false + }, + "components": { + "complexity_breakdown": {}, + "duplication_sources": [], + "coupling_dependencies": [], + "doc_missing_items": [], + "consistency_violations": [] + }, + "git_context": null + }, "./src/core/webhook_server.rs": { "content_hash": [ - 163, - 200, - 57, - 79, - 67, - 236, - 141, + 237, + 45, + 155, + 208, + 11, + 125, + 190, + 199, + 38, + 170, 6, - 181, - 67, + 17, + 163, + 133, + 157, + 239, + 201, 167, 146, - 60, - 94, - 63, - 201, - 19, - 132, - 141, - 242, - 230, - 105, + 181, + 235, + 133, + 28, + 4, 125, - 126, + 6, + 158, + 158, + 1, + 114, + 163, + 147 + ], + "score": { + "structural_complexity": 22.3, + "semantic_complexity": 20.0, + "duplication_ratio": 20.0, + "coupling_score": 15.0, + "doc_coverage": 10.0, + "consistency_score": 10.0, + "entropy_score": 5.5, + "total": 93.454544, + "grade": "AMinus", + "confidence": 1.0, + "language": "Rust", + "file_path": "./src/core/webhook_server.rs", + "penalties_applied": [ + { + "source_metric": "Duplication", + "amount": 4.5, + "applied_to": [ + "Duplication" + ], + "issue": "Found 9 duplicate code patterns" + }, + { + "source_metric": "StructuralComplexity", + "amount": 2.7, + "applied_to": [ + "StructuralComplexity" + ], + "issue": "High cognitive complexity: 24" + } + ], + "critical_defects_count": 0, + "has_critical_defects": false, + "has_contract_coverage": false + }, + "components": { + "complexity_breakdown": {}, + "duplication_sources": [], + "coupling_dependencies": [], + "doc_missing_items": [], + "consistency_violations": [] + }, + "git_context": null + }, + "./src/core/webhook_sig.rs": { + "content_hash": [ + 133, + 183, + 226, + 81, + 109, + 10, + 100, + 85, + 10, + 125, + 66, 216, - 43, - 181, - 203, - 193, - 213, - 94, - 5 + 202, + 135, + 72, + 93, + 20, + 19, + 233, + 8, + 138, + 81, + 173, + 192, + 245, + 49, + 229, + 218, + 58, + 45, + 192, + 171 ], "score": { "structural_complexity": 24.7, "semantic_complexity": 20.0, - "duplication_ratio": 17.353846, + "duplication_ratio": 17.894737, "coupling_score": 15.0, "doc_coverage": 10.0, "consistency_score": 10.0, "entropy_score": 5.0, - "total": 92.77623, + "total": 93.267944, "grade": "AMinus", "confidence": 1.0, "language": "Rust", - "file_path": "./src/core/webhook_server.rs", + "file_path": "./src/core/webhook_sig.rs", "penalties_applied": [ { "source_metric": "Duplication", @@ -55615,15 +55947,15 @@ "applied_to": [ "Duplication" ], - "issue": "Found 30 duplicate code patterns" + "issue": "Found 10 duplicate code patterns" }, { "source_metric": "Duplication", - "amount": 2.646154, + "amount": 2.1052632, "applied_to": [ "Duplication" ], - "issue": "Code duplication: 13.2%" + "issue": "Code duplication: 10.5%" }, { "source_metric": "StructuralComplexity", @@ -55649,48 +55981,48 @@ }, "./src/core/webhook_source.rs": { "content_hash": [ - 181, - 175, - 196, - 216, - 197, - 223, - 181, - 143, - 210, - 67, + 178, + 60, + 72, + 255, + 0, + 34, + 88, + 14, 47, - 75, - 42, - 56, - 176, - 94, - 32, - 171, - 74, - 244, - 132, - 86, - 78, + 30, + 183, + 121, + 116, + 102, + 203, + 182, + 151, + 231, + 37, + 106, + 219, + 151, + 159, + 54, 173, - 91, - 72, - 9, - 152, - 100, - 112, - 119, - 114 + 43, + 234, + 254, + 230, + 56, + 7, + 101 ], "score": { - "structural_complexity": 24.1, + "structural_complexity": 17.0, "semantic_complexity": 20.0, "duplication_ratio": 20.0, "coupling_score": 15.0, "doc_coverage": 10.0, "consistency_score": 10.0, "entropy_score": 5.0, - "total": 94.63636, + "total": 97.0, "grade": "AMinus", "confidence": 1.0, "language": "Rust", @@ -55702,15 +56034,23 @@ "applied_to": [ "Duplication" ], - "issue": "Found 12 duplicate code patterns" + "issue": "Found 10 duplicate code patterns" }, { "source_metric": "StructuralComplexity", - "amount": 0.90000004, + "amount": 4.5, "applied_to": [ "StructuralComplexity" ], - "issue": "High cognitive complexity: 18" + "issue": "High cognitive complexity: 30" + }, + { + "source_metric": "StructuralComplexity", + "amount": 3.5, + "applied_to": [ + "StructuralComplexity" + ], + "issue": "High cyclomatic complexity: 37" } ], "critical_defects_count": 0, @@ -62836,48 +63176,48 @@ }, "./tests/falsification_event_sources_b.rs": { "content_hash": [ - 55, - 135, - 22, - 249, - 243, - 243, - 235, - 83, - 248, - 225, - 232, - 28, - 163, - 91, - 54, + 206, + 251, + 189, + 118, + 136, 82, - 225, - 48, - 226, - 164, - 179, - 180, - 179, - 181, - 15, - 204, + 92, + 210, + 190, + 155, + 67, + 112, + 134, + 38, + 128, + 102, + 178, 166, - 81, - 84, - 176, - 64, - 205 + 241, + 146, + 222, + 77, + 87, + 16, + 66, + 174, + 65, + 23, + 227, + 120, + 248, + 190 ], "score": { "structural_complexity": 25.0, "semantic_complexity": 20.0, - "duplication_ratio": 17.777779, + "duplication_ratio": 17.860466, "coupling_score": 15.0, "doc_coverage": 10.0, "consistency_score": 10.0, "entropy_score": 5.0, - "total": 93.43435, + "total": 93.509514, "grade": "AMinus", "confidence": 1.0, "language": "Rust", @@ -62893,11 +63233,11 @@ }, { "source_metric": "Duplication", - "amount": 2.2222223, + "amount": 2.139535, "applied_to": [ "Duplication" ], - "issue": "Code duplication: 11.1%" + "issue": "Code duplication: 10.7%" } ], "critical_defects_count": 0, @@ -64171,48 +64511,48 @@ }, "./tests/falsification_migrate_webhook.rs": { "content_hash": [ - 17, - 137, - 122, - 211, - 162, - 92, - 8, - 186, - 31, - 86, - 185, - 110, - 216, - 214, + 192, 167, - 24, - 226, - 20, - 106, - 215, - 169, - 148, - 127, - 69, - 32, - 11, + 179, + 102, 58, - 24, - 76, - 210, - 152, - 172 + 186, + 49, + 103, + 94, + 80, + 83, + 106, + 60, + 231, + 238, + 33, + 102, + 230, + 57, + 201, + 156, + 230, + 125, + 109, + 206, + 30, + 218, + 6, + 140, + 90, + 101, + 193 ], "score": { "structural_complexity": 25.0, "semantic_complexity": 20.0, - "duplication_ratio": 14.986523, + "duplication_ratio": 15.092348, "coupling_score": 15.0, "doc_coverage": 10.0, "consistency_score": 10.0, "entropy_score": 5.0, - "total": 99.98653, + "total": 90.99304, "grade": "AMinus", "confidence": 1.0, "language": "Rust", @@ -64228,11 +64568,11 @@ }, { "source_metric": "Duplication", - "amount": 5.0134773, + "amount": 4.907652, "applied_to": [ "Duplication" ], - "issue": "Code duplication: 25.1%" + "issue": "Code duplication: 24.5%" } ], "critical_defects_count": 0, @@ -70749,48 +71089,48 @@ }, "./tests/falsification_undo_webhook.rs": { "content_hash": [ - 255, - 44, - 54, - 36, - 254, - 188, - 194, - 95, - 231, - 63, - 234, - 244, 46, - 211, - 194, - 37, - 166, - 188, - 194, - 249, + 16, + 119, + 149, + 104, 55, + 40, + 233, + 45, + 209, + 173, + 112, + 90, + 124, + 17, + 51, + 158, + 210, + 85, + 105, 6, - 244, - 73, - 136, - 70, - 108, - 186, - 41, - 186, - 129, - 199 + 149, + 163, + 128, + 97, + 32, + 239, + 223, + 246, + 173, + 157, + 66 ], "score": { "structural_complexity": 25.0, "semantic_complexity": 20.0, - "duplication_ratio": 17.64045, + "duplication_ratio": 17.656675, "coupling_score": 15.0, "doc_coverage": 10.0, "consistency_score": 10.0, "entropy_score": 5.0, - "total": 93.309494, + "total": 93.32425, "grade": "AMinus", "confidence": 1.0, "language": "Rust", @@ -70802,15 +71142,15 @@ "applied_to": [ "Duplication" ], - "issue": "Found 30 duplicate code patterns" + "issue": "Found 31 duplicate code patterns" }, { "source_metric": "Duplication", - "amount": 2.3595505, + "amount": 2.3433242, "applied_to": [ "Duplication" ], - "issue": "Code duplication: 11.8%" + "issue": "Code duplication: 11.7%" } ], "critical_defects_count": 0, @@ -71965,12 +72305,12 @@ } }, "summary": { - "total_files": 900, - "avg_score": 93.31748, + "total_files": 904, + "avg_score": 93.3116, "grade_distribution": { - "AMinus": 824, - "BPlus": 21, - "B": 42, + "AMinus": 828, + "BPlus": 22, + "B": 41, "BMinus": 10, "CPlus": 2, "F": 1 @@ -71979,7 +72319,7 @@ "JavaScript": 8, "Python": 3, "Ruby": 1, - "Rust": 888 + "Rust": 892 } } } \ No newline at end of file diff --git a/Cargo.lock b/Cargo.lock index 99da111e..89c9b922 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1134,6 +1134,7 @@ dependencies = [ "dhat", "flate2", "glob", + "hmac", "indexmap", "openssl", "pforge-config", diff --git a/Cargo.toml b/Cargo.toml index 0e67c3a0..d8192b95 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -66,6 +66,10 @@ bzip2 = "0.6" zstd = "0.13" tar = "0.4" sha2 = "0.10" +# HMAC-SHA256 for webhook request authentication (FJ-3104). One pure-Rust crate: +# sha2 is already a direct dep above, and hmac/digest/subtle were already in +# Cargo.lock transitively, so this adds no new transitive dependencies. +hmac = "0.12" flate2 = "1" rusqlite = { version = "0.32", features = ["bundled"] } dhat = { version = "0.3.3", optional = true } diff --git a/contracts/webhook-receiver-v1.yaml b/contracts/webhook-receiver-v1.yaml new file mode 100644 index 00000000..89cbe284 --- /dev/null +++ b/contracts/webhook-receiver-v1.yaml @@ -0,0 +1,325 @@ +--- +metadata: + version: 1.0.0 + created: '2026-07-30' + author: PAIML Engineering + description: | + Guarantees for forjar's webhook receiver — the surface that turns an inbound + HTTP request into an InfraEvent driving the rules engine. + + The receiver had never been reachable: `run_webhook_server` had ZERO non-test + callers and no `Commands` variant dispatched it, so it could not be started, + dogfooded, or reached by any sender. Every defect below survived ~35 passing + tests because those tests called the functions directly, and unreachable code + cannot be verified by using it. + + Four defect shapes, all confirmed by measurement rather than reading: + + * `compute_hmac_hex` was documented and named HMAC-SHA256 — its doc comment + described the ipad/opad construction — and computed a keyed BLAKE3 hash. + Against RFC 4231 TC2 it returned `30f3b0f1…` where HMAC-SHA256 is + `5bdcc146…`, so no standard sender could ever authenticate. It survived + because every signature test computed its expected value BY CALLING THE + FUNCTION UNDER TEST: `f(x) == f(x)` holds for any function. + * There was no HTTP framing. One `read()`, and `Content-Length` was parsed + into the header map and never read back. A correctly-framed delivery whose + body landed in a second TCP segment returned 400 with the event silently + dropped; with a secret set it degraded to SignatureInvalid, because the MAC + covered the prefix. + * Two independent fail-OPEN defaults: `secret` defaulted to None, and an + EMPTY `allowed_paths` meant allow-every-path. The configuration an operator + would write to lock the endpoint down was the least restrictive available. + * Connections were handled inline on the accept loop, so one client that + connected and sent nothing delayed a legitimate signed delivery by 5383ms + — entirely upstream of signature checking, so the secret was no defence. + kind: pattern + references: + - PMAT-201 — forjar webhook receiver + - RFC 4231 — HMAC-SHA256 test vectors + - RFC 9110 §9.5 — 405 MUST carry Allow + - Stripe webhook signatures — t=/v1= scheme and tolerance window + - GitHub X-Hub-Signature-256 — interop over the bare body + - execution-safety-v1.yaml — the action-execution boundary this must not cross +invariants: +- id: INV-MAC-IS-HMAC-SHA256 + statement: >- + Request authentication is HMAC-SHA256, matching RFC 4231 vectors exactly. The name, + the documentation and the computation agree. A digest produced by any conforming implementation + (openssl, GitHub, Stripe) verifies, and one produced by a different primitive does + not. +- id: INV-EXPECTED-VALUES-COME-FROM-OUTSIDE + statement: >- + No signature test derives its expected value from the function under test. Every expected + digest is an RFC 4231 vector or a literal generated by `openssl dgst -sha256 -hmac`. + This is the invariant whose absence let a substituted primitive ship: a self-referential + assertion cannot detect one. +- id: INV-MAC-COVERS-THE-WIRE-BYTES + statement: >- + The MAC is computed over the exact octets received, before any UTF-8 interpretation. + `WebhookRequest.body` is `Vec` and the signature functions take `&[u8]`, so a + lossy decode cannot sit between the wire and the verification. A body containing invalid + UTF-8, signed over its true bytes, authenticates; the U+FFFD-substituted form does + not. +- id: INV-SIGNATURE-BINDS-TIMESTAMP-METHOD-PATH + statement: >- + The signed payload is `t=\n\n\n`. A digest minted for one + allowed path does not verify at another, and one minted for POST does not verify for + another method. A body-only MAC replays forever and re-routes freely between entries + in `allowed_paths`, while `_path` reads like an authorization input. +- id: INV-FRESHNESS-IS-BIDIRECTIONAL + statement: >- + A request is rejected when |now - t| exceeds the tolerance window (default 300s). + Future-dated timestamps are rejected too: otherwise a sender — or an attacker choosing + t — could extend a captured request's validity. +- id: INV-DELIVERY-IS-AT-MOST-ONCE + statement: >- + A repeated signature produces at most one InfraEvent. Because t is inside the signed + payload the v1 digest is unique per send, so it serves as the delivery id with no + extra header for a sender to get wrong. A duplicate is answered 200, never 4xx — a + retrying sender did nothing wrong, and a 4xx would make it retry harder. CooldownTracker + is not a substitute: it rate-limits a rulebook, it does not make a delivery exactly-once. +- id: INV-FRAMING-IS-CONTENT-LENGTH-DRIVEN + statement: >- + The receiver reads until CRLFCRLF, then reads exactly Content-Length octets, across + as many segments as that takes. A body split at any boundary — up to one byte per + segment — is reassembled identically. Duplicate or non-numeric Content-Length is rejected + 400; Transfer-Encoding is rejected 501 rather than mis-framed. +- id: INV-SIZE-CAP-IS-ENFORCED-BEFORE-BUFFERING + statement: >- + A Content-Length exceeding max_body_bytes is answered 413 BEFORE the body is read, + so the cap bounds memory rather than describing it. Any configured maximum is honoured, + including above 64 KiB — the previous read buffer was clamped to min(max, 65536)+4096, + which made a larger cap silently inert and BodyTooLarge unreachable from the server. +- id: INV-FAIL-CLOSED-AT-STARTUP + statement: >- + The server refuses to bind when the configuration would expose an unauthenticated + or plaintext endpoint: no secret without an explicit `allow_unauthenticated`, an empty + `allowed_paths`, or a non-loopback bind without `tls_terminated_upstream`. Validation + runs before the listener, so a rejected configuration leaves no socket open. Refusal + rather than a warning, because the failure mode is arbitrary event injection into + a script-capable rules engine. +- id: INV-RESPONSES-ARE-VALID-JSON-AND-NON-REFLECTIVE + statement: >- + Every response body is JSON built by serde_json, carrying a fixed reason code from + this crate. No request-derived text is interpolated. The previous builder emitted + `{"status":"PathNotAllowed { path: "/evil" }"}` — invalid JSON, under Content-Type: + application/json, echoing attacker input. The Debug form is logged server-side only. +- id: INV-STATUS-CODES-ARE-HONEST + statement: >- + Outcomes map to distinct statuses: 405 (+ Allow: POST) for a non-POST, 413 for an + oversize body, 404 for an unknown path, 401 for missing/invalid/stale signatures, + 501 for chunked, 431 for an oversize head, 408 on timeout, 503 when the queue is full + or the event pipeline is closed. Every failure used to collapse to 403 while the 401/405/413 + arms of the status table were dead code. +- id: INV-PRE-AUTH-WORK-IS-BOUNDED + statement: >- + An unauthenticated client cannot deny service. Connections are handled by a bounded + worker pool with a bounded queue and a whole-connection deadline; when the queue is + full the receiver answers 503 and closes rather than queueing without limit. Idle + sockets do not delay a legitimate delivery. +- id: INV-ACKNOWLEDGEMENT-IMPLIES-ACCEPTANCE + statement: >- + A 200 is returned only when the event was handed to the pipeline. A closed receiver + yields 503, not 200. The previous code discarded the SendError and returned 200 anyway, + telling a sender its delivery was accepted when nothing would ever process it. +- id: INV-ONE-CLOCK + statement: >- + Produced events timestamp via `tripwire::eventlog::now_iso8601`, the same function + the trigger path uses. The receiver's private `now_iso8601` returned `format!("{}Z", + secs)` — epoch seconds with a Z suffix, not ISO 8601 — so two mutually unparseable + formats landed in one audit-log field. +- id: INV-ACTIONS-ARE-NOT-EXECUTED-HERE + statement: >- + `forjar rules serve` evaluates events and REPORTS matching actions; it does not run + them. This is a hard sequencing gate, not a limitation: `rulebook_template::expand_action` + substitutes attacker-controlled payload keys into `RulebookAction.script` via String::replace + with no shell quoting. Wiring an executor to a network listener would convert an inbound + request into command execution. The injection is currently unreachable (`expand_action` + has no callers) and must stay so until the quoting is fixed. +falsification_tests: +- id: FT-RFC4231 + rule: INV-MAC-IS-HMAC-SHA256 + prediction: >- + Every published RFC 4231 digest is reproduced exactly by compute_hmac_hex. + test: src/core/tests_webhook_sig.rs::rfc4231_tc1/tc2/tc3/tc6 + if_fails: >- + The primitive is not HMAC-SHA256. Compare the returned digest against `openssl dgst + -sha256 -hmac`; a 64-char mismatch means a substituted hash function, not a formatting + bug. + method: >- + Assert the published RFC 4231 digests. The old keyed-BLAKE3 returns 30f3b0f1… for + TC2 and fails immediately. +- id: FT-INDEPENDENT-ORACLE + rule: INV-EXPECTED-VALUES-COME-FROM-OUTSIDE + prediction: >- + A digest produced by openssl verifies against forjar, and a live openssl-signed delivery + to the running binary returns 200. + test: >- + src/core/tests_webhook_sig.rs::verify_accepts_an_openssl_generated_signature + if_fails: >- + forjar and the rest of the world disagree about the algorithm or the canonical payload. + Diff the exact bytes being signed before suspecting the MAC. + method: >- + Verify a digest literal generated by `openssl dgst -sha256 -hmac`, plus an end-to-end + delivery signed by openssl against the running binary. +- id: FT-NON-UTF8-BODY + rule: INV-MAC-COVERS-THE-WIRE-BYTES + prediction: >- + A body containing a raw 0xFF, signed over its exact octets, verifies; the U+FFFD-substituted + form does not. + test: >- + src/core/tests_webhook_sig.rs::verify_covers_non_utf8_bytes; tests_webhook_server.rs::non_utf8_body_signed_over_wire_bytes_is_accepted + if_fails: >- + A lossy UTF-8 decode has re-entered the path between the socket and verification. + Check that WebhookRequest.body is still Vec end to end. + method: >- + Sign a body containing a raw 0xFF over its exact bytes; assert it verifies and that + the U+FFFD-substituted form does NOT. +- id: FT-PATH-TRANSFER + rule: INV-SIGNATURE-BINDS-TIMESTAMP-METHOD-PATH + prediction: >- + A signature minted for one allowed path is rejected with 401 when replayed at another. + test: >- + src/core/tests_webhook_server.rs::signature_for_one_path_is_rejected_at_another + if_fails: >- + The path left the signed payload, so the MAC is body-only again and any allowed path + accepts any other's signature. + method: >- + With two allowed paths, replay a digest minted for the first at the second and assert + 401. +- id: FT-STALE-AND-FUTURE + rule: INV-FRESHNESS-IS-BIDIRECTIONAL + prediction: >- + |now - t| beyond the tolerance is rejected on BOTH sides of the window. + test: >- + src/core/tests_webhook_sig.rs::timestamp_in_the_future_is_rejected; tests_webhook_server.rs::stale_timestamp_is_rejected + if_fails: >- + Either the freshness check was dropped or it became one-sided; a future-dated t would + then extend a captured request's validity indefinitely. + method: Assert both edges of the window, and a live 1-hour-old delivery → 401. +- id: FT-DUPLICATE + rule: INV-DELIVERY-IS-AT-MOST-ONCE + prediction: >- + Byte-identical signed requests sent twice both return 200 and produce exactly ONE + event. + test: >- + src/core/tests_webhook_server.rs::duplicate_delivery_yields_exactly_one_event + if_fails: >- + The replay guard is not consulted, or the delivery id is no longer unique per send. + Confirm t is still inside the signed payload. + method: >- + Send byte-identical signed requests twice; assert both 200 and that the channel yields + exactly one event. +- id: FT-SPLIT-DELIVERY + rule: INV-FRAMING-IS-CONTENT-LENGTH-DRIVEN + prediction: >- + A delivery whose body arrives in a later segment — down to one byte per read — is + reassembled and accepted. + test: >- + src/core/tests_webhook_http.rs::body_split_across_segments_is_reassembled, body_split_one_byte_at_a_time; + tests_webhook_server.rs::signed_delivery_split_across_segments_is_accepted + if_fails: >- + Framing regressed to a single read(). Content-Length is being ignored again and events + are silently dropped off-loopback. + method: >- + Deliver head and body in separate writes separated by a pause, and byte-at-a-time + through a chunked reader. The old single-read path returns 400 and drops the event. +- id: FT-SIZE-CAP + rule: INV-SIZE-CAP-IS-ENFORCED-BEFORE-BUFFERING + prediction: >- + An oversize Content-Length is answered 413 with NO body sent, and a configured cap + above 64 KiB is honoured. + test: >- + src/core/tests_webhook_http.rs::oversize_content_length_is_rejected_before_reading_the_body, + max_body_above_64k_is_honoured + if_fails: >- + The size decision moved after buffering, so the cap describes rather than bounds memory; + or a read-buffer clamp reappeared. + method: >- + Declare an oversize Content-Length and send NO body: a 413 proves the decision preceded + buffering. Separately, accept a 200KB body under a 1MiB cap. +- id: FT-FAIL-CLOSED + rule: INV-FAIL-CLOSED-AT-STARTUP + prediction: >- + Each unsafe configuration is refused before binding, and the port remains bindable + afterwards. + test: >- + src/core/tests_webhook_server.rs::refuses_to_start_without_a_secret, refuses_to_start_with_an_empty_allow_list, + refuses_a_non_loopback_bind_without_upstream_tls, bad_config_does_not_bind + if_fails: >- + A fail-open default returned. An empty allowed_paths or a missing secret is again + accepted, exposing arbitrary event injection. + method: >- + Assert each rejection, and that the port remains bindable afterwards — proving nothing + was left listening. +- id: FT-JSON-RESPONSES + rule: INV-RESPONSES-ARE-VALID-JSON-AND-NON-REFLECTIVE + prediction: >- + Every status's body parses as JSON, hostile codes are escaped, and a 404 does not + echo the requested path. + test: >- + src/core/tests_webhook_http.rs::every_response_body_is_valid_json, response_escapes_hostile_codes; + tests_webhook_server.rs::unknown_path_is_404 + if_fails: >- + Response construction went back to string interpolation; the body is malformed JSON + and reflects attacker input. + method: >- + Parse the body of every status with serde_json; feed a code containing quotes and + braces; assert a 404 does not echo the requested path. +- id: FT-STATUS-MAPPING + rule: INV-STATUS-CODES-ARE-HONEST + prediction: 'Each outcome renders its own status line, and 405 carries Allow: POST.' + test: >- + src/core/tests_webhook_http.rs::previously_unreachable_statuses_have_reasons, method_not_allowed_carries_allow_header; + tests_webhook_server.rs::get_is_405_with_allow_header, unknown_path_is_404 + if_fails: >- + Outcomes are collapsing to one status again, making a rejection reason indistinguishable + to a sender. + method: 'Assert each status line and that 405 carries Allow: POST.' +- id: FT-IDLE-SOCKETS + rule: INV-PRE-AUTH-WORK-IS-BOUNDED + prediction: >- + With idle sockets held open, a legitimate signed delivery still completes in well + under a second. + test: >- + src/core/tests_webhook_server.rs::idle_connections_do_not_delay_a_real_delivery + if_fails: >- + Connection handling moved back onto the accept loop, so any unauthenticated client + can stall every delivery. + method: >- + Hold idle sockets open, then time a legitimate signed delivery and assert it completes + well under a second. The old inline handler measured 5383ms. +- id: FT-SECRET-REDACTED + rule: INV-FAIL-CLOSED-AT-STARTUP + prediction: Formatting WebhookConfig never emits the secret's value. + test: src/core/tests_webhook_server.rs::debug_redacts_the_secret + if_fails: >- + A derive(Debug) was restored and the shared secret will reach logs and panic messages. + method: Format the config and assert the secret value does not appear. +known_gaps: +- id: GAP-GITHUB-BINDING + statement: >- + A GitHub-signed request (X-Hub-Signature-256) is verified over the BARE body, so it + carries neither timestamp nor path binding. GitHub cannot be told to sign a custom + canonical form. Acceptable because GitHub delivers to one configured URL and the replay + guard still makes each delivery single-use, but such a request has weaker guarantees + than a t=/v1= one. +- id: GAP-NO-PERSISTED-REPLAY-SET + statement: >- + The replay guard is in-memory, so a restart forgets deliveries seen inside the current + tolerance window. Bounded by the freshness check to a ≤300s sliver, which is why persistence + was judged not worth a schema. +- id: GAP-NESTED-PAYLOAD-NOT-FLATTENED + statement: >- + `parse_json_payload` flattens only the top level; a nested object is stringified. + So a rulebook cannot match on `repository.full_name`. A rulebook-matching feature + rather than a receiver defect; deferred. +- id: GAP-NO-EVENT-NAME-FROM-HEADERS + statement: >- + Request headers do not reach event identity, so a GitHub `ping` cannot be distinguished + from a real delivery and every accepted request becomes a WebhookReceived event. Deferred. +- id: GAP-ACTION-EXECUTION-BLOCKED-ON-QUOTING + statement: >- + No action executor exists here by design (INV-ACTIONS-ARE-NOT-EXECUTED-HERE). Wiring + one requires fixing the unquoted template substitution in rulebook_template::expand_action + first. Tracked separately. diff --git a/examples/crypto_mcdc_undo_webhook.rs b/examples/crypto_mcdc_undo_webhook.rs index fec76029..2a45c7c8 100644 --- a/examples/crypto_mcdc_undo_webhook.rs +++ b/examples/crypto_mcdc_undo_webhook.rs @@ -104,16 +104,26 @@ fn main() { method: "POST".into(), path: "/webhook".into(), headers: HashMap::new(), - body: r#"{"action":"deploy","env":"prod"}"#.into(), + body: r#"{"action":"deploy","env":"prod"}"#.as_bytes().to_vec(), source_ip: Some("10.0.0.5".into()), }; println!(" Valid POST: {:?}", validate_request(&config, &req)); - let event = request_to_event(&req).unwrap(); + let event = request_to_event(&req, None, None).unwrap(); println!(" Event type: {}", event.event_type); println!(" Payload action: {}", event.payload["action"]); - println!(" HMAC: {}...", &compute_hmac_hex("secret", "data")[..16]); - println!(" ACK: {}", ack_response(200, "ok").lines().next().unwrap()); + println!( + " HMAC: {}...", + &forjar::core::webhook_sig::compute_hmac_hex("secret".as_bytes(), "data".as_bytes())[..16] + ); + println!( + " ACK: {}", + String::from_utf8(forjar::core::webhook_http::response(200, "ok")) + .unwrap() + .lines() + .next() + .unwrap() + ); println!("\n{}", "=".repeat(50)); println!("All crypto/mcdc/undo/webhook criteria survived."); diff --git a/examples/migrate_webhook.rs b/examples/migrate_webhook.rs index 1d9f8942..838dbb63 100644 --- a/examples/migrate_webhook.rs +++ b/examples/migrate_webhook.rs @@ -12,8 +12,7 @@ use forjar::core::migrate::{docker_to_pepita, migrate_config}; use forjar::core::parser::parse_config; use forjar::core::types::*; use forjar::core::webhook_source::{ - compute_hmac_hex, parse_json_payload, request_to_event, validate_request, WebhookConfig, - WebhookRequest, + parse_json_payload, request_to_event, validate_request, WebhookConfig, WebhookRequest, }; use std::collections::HashMap; @@ -80,18 +79,31 @@ resources: secret: Some("deploy-secret".into()), max_body_bytes: 1024, allowed_paths: vec!["/hooks/deploy".into()], + ..WebhookConfig::default() }; // Valid request with HMAC let body = r#"{"action":"deploy","env":"production"}"#; - let sig = compute_hmac_hex("deploy-secret", body); + // The signature binds timestamp, method and path — not the body alone — so a + // digest minted for /hooks/deploy cannot be replayed at another allowed path. + let t_now = forjar::core::webhook_sig::unix_now(); + let signed = forjar::core::webhook_sig::canonical_payload( + t_now, + "POST", + "/hooks/deploy", + body.as_bytes(), + ); + let digest = forjar::core::webhook_sig::compute_hmac_hex(b"deploy-secret", &signed); let mut headers = HashMap::new(); - headers.insert("x-forjar-signature".into(), sig); + headers.insert( + "x-forjar-signature".into(), + format!("t={t_now},v1={digest}"), + ); let req = WebhookRequest { method: "POST".into(), path: "/hooks/deploy".into(), headers, - body: body.into(), + body: body.as_bytes().to_vec(), source_ip: Some("10.0.0.1".into()), }; let vr = validate_request(&config, &req); @@ -103,7 +115,7 @@ resources: method: "GET".into(), path: "/hooks/deploy".into(), headers: HashMap::new(), - body: "".into(), + body: Vec::new(), source_ip: None, }; let vr = validate_request(&config, &bad_req); @@ -112,11 +124,11 @@ resources: // ── FJ-3104: Payload parsing ── println!("\n[FJ-3104] Webhook Payload Parsing:"); - let payload = parse_json_payload(body).unwrap(); + let payload = parse_json_payload(body.as_bytes()).unwrap(); println!(" action={}, env={}", payload["action"], payload["env"]); assert_eq!(payload["action"], "deploy"); - let event = request_to_event(&req).unwrap(); + let event = request_to_event(&req, None, None).unwrap(); println!(" Event type: {:?}", event.event_type); println!( " Payload keys: {:?}", diff --git a/examples/webhook_source.rs b/examples/webhook_source.rs index 27bb5aef..1aecf7dd 100644 --- a/examples/webhook_source.rs +++ b/examples/webhook_source.rs @@ -10,8 +10,7 @@ use forjar::core::webhook_server; use forjar::core::webhook_source::{ - ack_response, compute_hmac_hex, parse_json_payload, request_to_event, validate_request, - WebhookConfig, WebhookRequest, + parse_json_payload, request_to_event, validate_request, WebhookConfig, WebhookRequest, }; use std::collections::HashMap; use std::sync::atomic::{AtomicBool, Ordering}; @@ -43,7 +42,7 @@ fn main() { method: "POST".into(), path: "/webhook".into(), headers: HashMap::new(), - body: r#"{"action":"deploy","env":"production"}"#.into(), + body: r#"{"action":"deploy","env":"production"}"#.as_bytes().to_vec(), source_ip: Some("10.0.0.1".into()), }; let result = validate_request(&config, &good_req); @@ -53,7 +52,7 @@ fn main() { method: "GET".into(), path: "/webhook".into(), headers: HashMap::new(), - body: String::new(), + body: Vec::new(), source_ip: None, }; let result = validate_request(&config, &get_req); @@ -63,7 +62,7 @@ fn main() { method: "POST".into(), path: "/admin/hack".into(), headers: HashMap::new(), - body: "{}".into(), + body: "{}".as_bytes().to_vec(), source_ip: None, }; let result = validate_request(&config, &bad_path); @@ -73,7 +72,13 @@ fn main() { println!("\n3. HMAC Signature Verification:"); let secret = "my-webhook-secret"; let body = r#"{"event":"deploy"}"#; - let sig = compute_hmac_hex(secret, body); + let t_now = forjar::core::webhook_sig::unix_now(); + let signed = + forjar::core::webhook_sig::canonical_payload(t_now, "POST", "/webhook", body.as_bytes()); + let sig = format!( + "t={t_now},v1={}", + forjar::core::webhook_sig::compute_hmac_hex(secret.as_bytes(), &signed) + ); println!(" Secret: {secret}"); println!(" Signature: {}...", &sig[..16]); @@ -86,7 +91,7 @@ fn main() { method: "POST".into(), path: "/webhook".into(), headers: HashMap::new(), - body: body.into(), + body: body.as_bytes().to_vec(), source_ip: None, }; signed_req.headers.insert("x-forjar-signature".into(), sig); @@ -106,7 +111,7 @@ fn main() { r#"{"count":42,"tags":["web","prod"]}"#, ]; for body in &payloads { - match parse_json_payload(body) { + match parse_json_payload(body.as_bytes()) { Ok(kv) => { let pairs: Vec<_> = kv.iter().map(|(k, v)| format!("{k}={v}")).collect(); println!(" {} → {}", body, pairs.join(", ")); @@ -117,7 +122,7 @@ fn main() { // 5. Convert to InfraEvent println!("\n5. Request → InfraEvent:"); - let event = request_to_event(&good_req).unwrap(); + let event = request_to_event(&good_req, None, None).unwrap(); println!(" Type: {:?}", event.event_type); println!(" Payload:"); for (k, v) in &event.payload { @@ -126,9 +131,10 @@ fn main() { // 6. HTTP response formatting println!("\n6. Response Formatting:"); - let resp = ack_response(200, "accepted"); + let resp = String::from_utf8(forjar::core::webhook_http::response(200, "accepted")).unwrap(); println!(" 200: {}", resp.lines().next().unwrap()); - let resp = ack_response(401, "unauthorized"); + let resp = + String::from_utf8(forjar::core::webhook_http::response(401, "unauthorized")).unwrap(); println!(" 401: {}", resp.lines().next().unwrap()); // 7. Webhook server: start, receive, and stop diff --git a/src/cli/commands/subcmd_args.rs b/src/cli/commands/subcmd_args.rs index 538a7183..93e72f58 100644 --- a/src/cli/commands/subcmd_args.rs +++ b/src/cli/commands/subcmd_args.rs @@ -217,6 +217,48 @@ pub enum RulesCmd { #[arg(long)] json: bool, }, + /// Serve webhooks and evaluate them against a rulebook + /// + /// The webhook receiver had no entry point at all: `run_webhook_server` had + /// zero non-test callers, so it could not be started, dogfooded, or reached by + /// any sender. Unreachable code cannot be verified by using it, which is why + /// its defects survived ~35 passing tests. + Serve { + /// Path to rulebook YAML file + #[arg(short, long, default_value = "forjar.yaml")] + file: PathBuf, + /// Address to bind. A non-loopback bind requires --tls-terminated-upstream. + #[arg(long, default_value = "127.0.0.1")] + bind: String, + /// Port to listen on + #[arg(long, default_value_t = 8484)] + port: u16, + /// Read the HMAC-SHA256 shared secret from this file + /// + /// A file rather than a flag: a secret on the command line is visible in + /// `ps` output and lands in shell history. + #[arg(long)] + secret_file: Option, + /// Request paths to accept (repeatable). Empty denies everything. + #[arg(long = "path", default_values_t = [String::from("/webhook")])] + paths: Vec, + /// Accept unsigned requests. Every accepted request can fire rulebook + /// actions, so this must be stated explicitly. + #[arg(long)] + allow_unauthenticated: bool, + /// Assert TLS is terminated upstream, permitting a non-loopback bind + #[arg(long)] + tls_terminated_upstream: bool, + /// Signature freshness window in seconds + #[arg(long, default_value_t = 300)] + tolerance_secs: u64, + /// Validate the configuration and exit without binding + #[arg(long)] + check: bool, + /// JSON output + #[arg(long)] + json: bool, + }, } /// FJ-3403: Plugin management subcommands. diff --git a/src/cli/mod.rs b/src/cli/mod.rs index 26834e0d..91382fae 100644 --- a/src/cli/mod.rs +++ b/src/cli/mod.rs @@ -165,6 +165,7 @@ mod remote_state; mod repro_proof; mod reseal; mod rules; +mod rules_serve; mod run_task; mod runtime_invariants; mod saga_coordinator; diff --git a/src/cli/rules.rs b/src/cli/rules.rs index 796f1e3b..cf2a383c 100644 --- a/src/cli/rules.rs +++ b/src/cli/rules.rs @@ -14,6 +14,36 @@ pub fn dispatch_rules(cmd: RulesCmd) -> Result<(), String> { match cmd { RulesCmd::Validate { file, json } => cmd_rules_validate(&file, json), RulesCmd::Coverage { file, json } => cmd_rules_coverage(&file, json), + RulesCmd::Serve { + file, + bind, + port, + secret_file, + paths, + allow_unauthenticated, + tls_terminated_upstream, + tolerance_secs, + check, + json, + } => { + let config = super::rules_serve::build_config( + bind, + port, + secret_file.as_deref(), + paths, + allow_unauthenticated, + tls_terminated_upstream, + tolerance_secs, + )?; + // Rulebook first: a bad rulebook must not cost a bound port. + let rulebook = super::rules_serve::load_rulebook(&file)?; + if check { + config.validate_startup()?; + super::rules_serve::print_check(&config, rulebook.rulebooks.len(), json); + return Ok(()); + } + super::rules_serve::serve(config, rulebook, json) + } } } diff --git a/src/cli/rules_serve.rs b/src/cli/rules_serve.rs new file mode 100644 index 00000000..c4b2ae00 --- /dev/null +++ b/src/cli/rules_serve.rs @@ -0,0 +1,207 @@ +//! FJ-3109: `forjar rules serve` — the webhook receiver's entry point. +//! +//! # Why this exists +//! +//! `run_webhook_server` had ZERO non-test callers. No `Commands` variant reached +//! it, so the receiver could not be started by anyone, which meant it could not be +//! dogfooded and no sender could ever exercise it. That is how it accumulated a +//! wrong MAC algorithm, no HTTP framing, two fail-open defaults and an +//! unauthenticated-DoS path while carrying ~35 passing tests: every one of them +//! called the functions directly. +//! +//! # ⚠️ Actions are NOT executed here +//! +//! Accepted events are evaluated against the rulebook and the matching actions are +//! REPORTED. They are deliberately not run. `rulebook_template::expand_action` +//! substitutes attacker-controlled payload keys into `RulebookAction.script` with +//! `String::replace` and no shell quoting, so wiring an executor to a network +//! listener would turn an inbound request into command execution. +//! +//! That injection is currently unreachable — `expand_action` has no callers and +//! `cli::trigger` only prints `action.action_type()` — and it must stay that way +//! until the quoting is fixed. Receiver authentication, freshness and idempotency +//! landing FIRST is the sequencing gate, not a preference. + +use crate::core::rules_runtime; +use crate::core::types::{CooldownTracker, InfraEvent, RulebookConfig}; +use crate::core::webhook_server::run_webhook_server; +use crate::core::webhook_source::WebhookConfig; +use std::path::Path; +use std::sync::atomic::AtomicBool; +use std::sync::Arc; + +/// Assemble a [`WebhookConfig`] from CLI arguments. +#[allow(clippy::too_many_arguments)] +pub fn build_config( + bind: String, + port: u16, + secret_file: Option<&Path>, + paths: Vec, + allow_unauthenticated: bool, + tls_terminated_upstream: bool, + tolerance_secs: u64, +) -> Result { + let secret = match secret_file { + None => None, + Some(p) => { + let raw = std::fs::read_to_string(p) + .map_err(|e| format!("read secret file {}: {e}", p.display()))?; + // Trim: an editor's trailing newline would otherwise become part of + // the key, and every signature would fail with no clue why. + let trimmed = raw.trim().to_string(); + if trimmed.is_empty() { + return Err(format!("secret file {} is empty", p.display())); + } + Some(trimmed) + } + }; + + Ok(WebhookConfig { + bind, + port, + secret, + allowed_paths: paths, + allow_unauthenticated, + tls_terminated_upstream, + signature_tolerance_secs: tolerance_secs, + machine: Some(hostname_or_local()), + ..WebhookConfig::default() + }) +} + +/// Best-effort local machine name for event attribution. +fn hostname_or_local() -> String { + std::env::var("HOSTNAME") + .ok() + .filter(|h| !h.is_empty()) + .unwrap_or_else(|| "local".to_string()) +} + +/// Load and validate the rulebook before binding anything. +pub fn load_rulebook(file: &Path) -> Result { + let content = + std::fs::read_to_string(file).map_err(|e| format!("read {}: {e}", file.display()))?; + let issues = crate::core::rules_engine::validate_rulebook_yaml(&content)?; + let errors: Vec<_> = issues + .iter() + .filter(|i| i.severity == crate::core::rules_engine::IssueSeverity::Error) + .collect(); + if !errors.is_empty() { + let msgs: Vec = errors + .iter() + .map(|i| format!(" {}: {}", i.rulebook, i.message)) + .collect(); + return Err(format!("rulebook validation failed:\n{}", msgs.join("\n"))); + } + serde_yaml_ng::from_str(&content).map_err(|e| format!("parse rulebook: {e}")) +} + +/// Print the configuration that WOULD be served, without binding. +pub fn print_check(config: &WebhookConfig, rulebooks: usize, json: bool) { + let authenticated = config.secret.is_some(); + if json { + let out = serde_json::json!({ + "check": true, + "bind": config.bind, + "port": config.port, + "authenticated": authenticated, + "allowed_paths": config.allowed_paths, + "signature_tolerance_secs": config.signature_tolerance_secs, + "rulebooks": rulebooks, + "actions_executed": false, + }); + println!("{}", serde_json::to_string_pretty(&out).unwrap_or_default()); + } else { + println!("Webhook receiver configuration OK"); + println!(" Bind: {}:{}", config.bind, config.port); + println!( + " Auth: {}", + if authenticated { + "HMAC-SHA256 (X-Forjar-Signature: t=…,v1=…; X-Hub-Signature-256 accepted)" + } else { + "NONE (--allow-unauthenticated)" + } + ); + println!(" Paths: {}", config.allowed_paths.join(", ")); + println!(" Tolerance: {}s", config.signature_tolerance_secs); + println!(" Rulebooks: {rulebooks}"); + println!(" Actions: reported, NOT executed (see module docs)"); + } +} + +/// Run the receiver, evaluating each accepted event against the rulebook. +pub fn serve(config: WebhookConfig, rulebook: RulebookConfig, json: bool) -> Result<(), String> { + // Fail before binding, so a bad config never leaves a socket open. + config.validate_startup()?; + + let (tx, rx) = std::sync::mpsc::channel::(); + let shutdown = Arc::new(AtomicBool::new(false)); + let server_config = config.clone(); + let server_shutdown = Arc::clone(&shutdown); + + let handle = + std::thread::spawn(move || run_webhook_server(&server_config, tx, server_shutdown)); + + eprintln!( + "forjar: webhook receiver listening on {}:{} ({} path(s), {})", + config.bind, + config.port, + config.allowed_paths.len(), + if config.secret.is_some() { + "signed" + } else { + "UNAUTHENTICATED" + } + ); + + let mut tracker = CooldownTracker::default(); + // Ends when the server thread drops the sender. + for event in rx { + let results = rules_runtime::evaluate_event(&event, &rulebook, &mut tracker); + let fired: Vec<_> = results + .iter() + .filter(|r| !r.cooldown_blocked && !r.disabled && !r.actions.is_empty()) + .collect(); + report(&event, &fired, json); + } + + shutdown.store(true, std::sync::atomic::Ordering::Relaxed); + match handle.join() { + Ok(result) => result, + Err(_) => Err("webhook server thread panicked".to_string()), + } +} + +/// Report what an event matched. Actions are named, never run. +fn report(event: &InfraEvent, fired: &[&rules_runtime::EvalResult], json: bool) { + if json { + let out = serde_json::json!({ + "event": format!("{:?}", event.event_type), + "timestamp": event.timestamp, + "machine": event.machine, + "event_id": event.payload.get("_event_id"), + "path": event.payload.get("_path"), + "fired": fired.iter().map(|r| serde_json::json!({ + "rulebook": r.rulebook, + "actions": r.actions.len(), + })).collect::>(), + "actions_executed": false, + }); + println!("{}", serde_json::to_string(&out).unwrap_or_default()); + } else { + println!( + "event {:?} at {} path={} → {} rulebook(s) matched", + event.event_type, + event.timestamp, + event.payload.get("_path").map_or("-", String::as_str), + fired.len() + ); + for r in fired { + println!( + " {} → {} action(s) (not executed)", + r.rulebook, + r.actions.len() + ); + } + } +} diff --git a/src/core/mod.rs b/src/core/mod.rs index e4e7602a..ebf71d37 100644 --- a/src/core/mod.rs +++ b/src/core/mod.rs @@ -46,9 +46,17 @@ pub mod state_encryption; pub mod store; pub mod strutil; pub mod task; +#[cfg(test)] +mod tests_webhook_http; +#[cfg(test)] +mod tests_webhook_server; +#[cfg(test)] +mod tests_webhook_sig; pub mod types; pub mod watch_daemon; +pub mod webhook_http; pub mod webhook_server; +pub mod webhook_sig; pub mod webhook_source; pub mod do330; diff --git a/src/core/tests_webhook_http.rs b/src/core/tests_webhook_http.rs new file mode 100644 index 00000000..1e27ee99 --- /dev/null +++ b/src/core/tests_webhook_http.rs @@ -0,0 +1,383 @@ +//! Tests for [`crate::core::webhook_http`]. +//! +//! The framing tests use a reader that hands out the request in ARBITRARY chunks, +//! because that is exactly what the old code got wrong: it did one `read()` and +//! treated whatever arrived as the whole message. Its own tests wrote a ~19-byte +//! body in a single `write_all` on loopback, which always lands in one segment, so +//! they could never fail. + +use super::webhook_http::*; +use std::io::Read; +use std::time::Duration; + +/// A reader that yields pre-set chunks, then EOF. +/// +/// Models a body split across TCP segments without needing a real socket. A +/// `read()` never spans two chunks, so each chunk is a segment boundary the +/// implementation has to cope with. +/// +/// Tracks an offset WITHIN the current chunk: the first version advanced to the +/// next chunk after every read, so a chunk larger than the caller's buffer +/// silently lost its tail. That made two tests fail against correct product code +/// — a harness defect, and a reminder that a fixture can be the thing that's +/// wrong. +struct ChunkReader { + chunks: Vec>, + idx: usize, + off: usize, +} + +impl ChunkReader { + fn new(chunks: &[&[u8]]) -> Self { + Self { + chunks: chunks.iter().map(|c| c.to_vec()).collect(), + idx: 0, + off: 0, + } + } +} + +impl Read for ChunkReader { + fn read(&mut self, buf: &mut [u8]) -> std::io::Result { + if self.idx >= self.chunks.len() { + return Ok(0); + } + let chunk = &self.chunks[self.idx]; + let remaining = &chunk[self.off..]; + let n = remaining.len().min(buf.len()); + buf[..n].copy_from_slice(&remaining[..n]); + self.off += n; + if self.off >= chunk.len() { + self.idx += 1; + self.off = 0; + } + Ok(n) + } +} + +fn deadline() -> Duration { + Duration::from_secs(5) +} + +fn head(body_len: usize) -> String { + format!( + "POST /webhook HTTP/1.1\r\nHost: x\r\nContent-Type: application/json\r\nContent-Length: {body_len}\r\n\r\n" + ) +} + +// ── Framing ────────────────────────────────────────────────────────────────── + +#[test] +fn body_in_one_segment_is_read() { + let body = br#"{"action":"deploy"}"#; + let raw = format!("{}{}", head(body.len()), String::from_utf8_lossy(body)); + let mut r = ChunkReader::new(&[raw.as_bytes()]); + match read_request(&mut r, 4096, deadline()) { + ReadOutcome::Complete { + method, + path, + body: b, + .. + } => { + assert_eq!(method, "POST"); + assert_eq!(path, "/webhook"); + assert_eq!(b, body); + } + other => panic!("expected Complete, got {other:?}"), + } +} + +/// THE regression. Head and body in separate segments — measured to return 400 +/// with the event silently dropped, because Content-Length was never consulted. +#[test] +fn body_split_across_segments_is_reassembled() { + let body = br#"{"action":"deploy"}"#; + let h = head(body.len()); + let mut r = ChunkReader::new(&[h.as_bytes(), body]); + match read_request(&mut r, 4096, deadline()) { + ReadOutcome::Complete { body: b, .. } => assert_eq!(b, body), + other => panic!("split delivery must reassemble, got {other:?}"), + } +} + +/// Byte-at-a-time is the pathological case a single read() can never survive. +#[test] +fn body_split_one_byte_at_a_time() { + let body = br#"{"a":1}"#; + let h = head(body.len()); + let mut chunks: Vec<&[u8]> = vec![h.as_bytes()]; + let singles: Vec<[u8; 1]> = body.iter().map(|b| [*b]).collect(); + for s in &singles { + chunks.push(s); + } + let mut r = ChunkReader::new(&chunks); + match read_request(&mut r, 4096, deadline()) { + ReadOutcome::Complete { body: b, .. } => assert_eq!(b, body), + other => panic!("expected Complete, got {other:?}"), + } +} + +/// Even the header block can arrive split. +#[test] +fn head_split_across_segments() { + let body = b"{}"; + let h = head(body.len()); + let (a, b) = h.split_at(12); + let mut r = ChunkReader::new(&[a.as_bytes(), b.as_bytes(), body]); + assert!(matches!( + read_request(&mut r, 4096, deadline()), + ReadOutcome::Complete { .. } + )); +} + +/// A body longer than Content-Length must not bleed into the request. +#[test] +fn body_longer_than_content_length_is_truncated() { + let h = head(2); + let raw = format!("{h}{{}}EXTRA"); + let mut r = ChunkReader::new(&[raw.as_bytes()]); + match read_request(&mut r, 4096, deadline()) { + ReadOutcome::Complete { body, .. } => assert_eq!(body, b"{}"), + other => panic!("expected Complete, got {other:?}"), + } +} + +#[test] +fn missing_content_length_means_empty_body() { + let raw = "POST /webhook HTTP/1.1\r\nHost: x\r\n\r\n"; + let mut r = ChunkReader::new(&[raw.as_bytes()]); + match read_request(&mut r, 4096, deadline()) { + ReadOutcome::Complete { body, .. } => assert!(body.is_empty()), + other => panic!("expected Complete, got {other:?}"), + } +} + +// ── Size limits ────────────────────────────────────────────────────────────── + +/// 413 decided from the HEADER, before the body is buffered — otherwise the cap +/// does not actually bound memory. The old code clamped its read buffer to +/// `min(max_body_bytes, 65536) + 4096`, so a configured max above 64 KiB was +/// silently inert AND `BodyTooLarge` was unreachable from the server. +#[test] +fn oversize_content_length_is_rejected_before_reading_the_body() { + let h = head(10_000); + // Note: no body chunk at all. If the implementation tried to read it first + // this would time out or under-read instead of answering 413. + let mut r = ChunkReader::new(&[h.as_bytes()]); + assert_eq!( + read_request(&mut r, 1024, deadline()), + ReadOutcome::Rejected { + status: 413, + code: "body_too_large" + } + ); +} + +/// A max above 64 KiB must be honoured, not clamped. +#[test] +fn max_body_above_64k_is_honoured() { + let body = vec![b'x'; 200_000]; + let h = head(body.len()); + let mut r = ChunkReader::new(&[h.as_bytes(), &body]); + match read_request(&mut r, 1024 * 1024, deadline()) { + ReadOutcome::Complete { body: b, .. } => assert_eq!(b.len(), 200_000), + other => panic!("1MiB max must accept a 200KB body, got {other:?}"), + } +} + +#[test] +fn oversize_head_is_rejected_with_431() { + let mut big = String::from("POST /webhook HTTP/1.1\r\n"); + for i in 0..2000 { + big.push_str(&format!("X-Pad-{i}: aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\r\n")); + } + let mut r = ChunkReader::new(&[big.as_bytes()]); + assert_eq!( + read_request(&mut r, 4096, deadline()), + ReadOutcome::Rejected { + status: 431, + code: "head_too_large" + } + ); +} + +// ── Malformed requests ─────────────────────────────────────────────────────── + +/// A duplicate Content-Length is a request-smuggling primitive when a proxy and +/// the origin pick different ones. The old header map silently kept the last. +#[test] +fn duplicate_content_length_is_rejected() { + let raw = "POST /webhook HTTP/1.1\r\nContent-Length: 2\r\nContent-Length: 3\r\n\r\n{}"; + let mut r = ChunkReader::new(&[raw.as_bytes()]); + assert_eq!( + read_request(&mut r, 4096, deadline()), + ReadOutcome::Rejected { + status: 400, + code: "duplicate_content_length" + } + ); +} + +#[test] +fn non_numeric_content_length_is_rejected() { + let raw = "POST /webhook HTTP/1.1\r\nContent-Length: banana\r\n\r\n"; + let mut r = ChunkReader::new(&[raw.as_bytes()]); + assert_eq!( + read_request(&mut r, 4096, deadline()), + ReadOutcome::Rejected { + status: 400, + code: "invalid_content_length" + } + ); +} + +/// Chunked bodies are refused outright rather than mis-framed. +#[test] +fn transfer_encoding_is_rejected_with_501() { + let raw = "POST /webhook HTTP/1.1\r\nTransfer-Encoding: chunked\r\n\r\n0\r\n\r\n"; + let mut r = ChunkReader::new(&[raw.as_bytes()]); + assert_eq!( + read_request(&mut r, 4096, deadline()), + ReadOutcome::Rejected { + status: 501, + code: "transfer_encoding_unsupported" + } + ); +} + +#[test] +fn empty_connection_owes_no_response() { + let mut r = ChunkReader::new(&[]); + assert_eq!(read_request(&mut r, 4096, deadline()), ReadOutcome::Empty); +} + +#[test] +fn malformed_request_line_is_rejected() { + let raw = "BADLINE\r\n\r\n"; + let mut r = ChunkReader::new(&[raw.as_bytes()]); + assert_eq!( + read_request(&mut r, 4096, deadline()), + ReadOutcome::Rejected { + status: 400, + code: "malformed_request_line" + } + ); +} + +/// Truncated body (client closed early) is a clean 400, not a hang. +#[test] +fn truncated_body_is_rejected() { + let h = head(100); + let mut r = ChunkReader::new(&[h.as_bytes(), b"short"]); + assert_eq!( + read_request(&mut r, 4096, deadline()), + ReadOutcome::Rejected { + status: 400, + code: "incomplete_body" + } + ); +} + +/// A bare LF head terminator is not valid HTTP framing and is not accepted. +#[test] +fn bare_lf_is_not_a_head_terminator() { + let raw = "POST /webhook HTTP/1.1\nContent-Length: 2\n\n{}"; + let mut r = ChunkReader::new(&[raw.as_bytes()]); + // No CRLFCRLF, so the head never terminates and the read ends at EOF. + assert!(matches!( + read_request(&mut r, 4096, deadline()), + ReadOutcome::Rejected { .. } + )); +} + +/// Headers are lowercased so lookup is unambiguous, and the query string is +/// preserved on the path for signature purposes. +#[test] +fn headers_are_lowercased_and_query_preserved() { + let raw = "POST /webhook?x=1 HTTP/1.1\r\nX-Forjar-Signature: t=1,v1=ab\r\n\r\n"; + let mut r = ChunkReader::new(&[raw.as_bytes()]); + match read_request(&mut r, 4096, deadline()) { + ReadOutcome::Complete { path, headers, .. } => { + assert_eq!(path, "/webhook?x=1"); + assert!(headers + .iter() + .any(|(k, v)| k == "x-forjar-signature" && v == "t=1,v1=ab")); + } + other => panic!("expected Complete, got {other:?}"), + } +} + +// ── Responses ──────────────────────────────────────────────────────────────── + +/// EVERY status must produce parseable JSON. The old builder emitted +/// `{"status":"PathNotAllowed { path: "/evil" }"}` — invalid JSON, advertised as +/// application/json, echoing attacker input. +#[test] +fn every_response_body_is_valid_json() { + for status in [ + 200u16, 400, 401, 403, 404, 405, 408, 413, 431, 500, 501, 503, + ] { + let raw = response(status, "some_code"); + let text = String::from_utf8(raw).expect("response must be UTF-8"); + let (_, body) = text + .split_once("\r\n\r\n") + .unwrap_or_else(|| panic!("no header/body split for {status}")); + let parsed: serde_json::Value = serde_json::from_str(body) + .unwrap_or_else(|e| panic!("status {status} body is not JSON: {e} ({body})")); + assert_eq!(parsed["status"], "some_code"); + } +} + +/// A code containing quotes and braces — the shape that used to break the JSON — +/// must be escaped, not interpolated. +#[test] +fn response_escapes_hostile_codes() { + let raw = response(404, r#"PathNotAllowed { path: "/evil" }"#); + let text = String::from_utf8(raw).unwrap(); + let (_, body) = text.split_once("\r\n\r\n").unwrap(); + let parsed: serde_json::Value = serde_json::from_str(body).expect("must stay valid JSON"); + assert_eq!(parsed["status"], r#"PathNotAllowed { path: "/evil" }"#); +} + +#[test] +fn response_declares_length_and_closes() { + let text = String::from_utf8(response(200, "accepted")).unwrap(); + let (head, body) = text.split_once("\r\n\r\n").unwrap(); + assert!(head.contains("Connection: close")); + assert!(head.contains(&format!("Content-Length: {}", body.len()))); +} + +/// RFC 9110 9.5: a 405 MUST carry Allow. +#[test] +fn method_not_allowed_carries_allow_header() { + let text = String::from_utf8(response(405, "method_not_allowed")).unwrap(); + assert!(text.contains("Allow: POST"), "405 must advertise Allow"); + assert!(text.starts_with("HTTP/1.1 405 Method Not Allowed")); +} + +/// 503 tells the sender when to come back rather than leaving it to guess. +#[test] +fn service_unavailable_carries_retry_after() { + let text = String::from_utf8(response(503, "queue_full")).unwrap(); + assert!(text.contains("Retry-After:")); +} + +/// Statuses that used to be dead code in the reason table now render properly. +#[test] +fn previously_unreachable_statuses_have_reasons() { + for (status, reason) in [ + (401u16, "Unauthorized"), + (404, "Not Found"), + (405, "Method Not Allowed"), + (413, "Content Too Large"), + (431, "Request Header Fields Too Large"), + (501, "Not Implemented"), + ] { + let text = String::from_utf8(response(status, "c")).unwrap(); + assert!( + text.starts_with(&format!("HTTP/1.1 {status} {reason}")), + "status {status} rendered as {:?}", + text.lines().next() + ); + } +} diff --git a/src/core/tests_webhook_server.rs b/src/core/tests_webhook_server.rs new file mode 100644 index 00000000..d99025c2 --- /dev/null +++ b/src/core/tests_webhook_server.rs @@ -0,0 +1,385 @@ +//! Integration tests for [`crate::core::webhook_server`] over real sockets. +//! +//! These drive the server the way a sender does — connect, write bytes, read the +//! response — because that is the only layer where the framing, the MAC over wire +//! bytes, the status codes and the concurrency behaviour are all simultaneously +//! true or false. The old suite's server tests wrote a small body in one +//! `write_all` and asserted `resp.contains("200 OK")`, which could not observe any +//! of it. + +use super::webhook_server::run_webhook_server; +use super::webhook_sig::{canonical_payload, compute_hmac_hex, unix_now}; +use super::webhook_source::WebhookConfig; +use crate::core::types::InfraEvent; +use std::io::{Read, Write}; +use std::net::{TcpListener, TcpStream}; +use std::sync::atomic::{AtomicBool, Ordering}; +use std::sync::mpsc::{self, Receiver}; +use std::sync::Arc; +use std::time::{Duration, Instant}; + +const SECRET: &str = "test-secret"; + +fn free_port() -> u16 { + let l = TcpListener::bind("127.0.0.1:0").unwrap(); + l.local_addr().unwrap().port() +} + +struct Server { + port: u16, + rx: Receiver, + shutdown: Arc, +} + +impl Drop for Server { + fn drop(&mut self) { + self.shutdown.store(true, Ordering::Relaxed); + } +} + +fn start(config: WebhookConfig) -> Server { + let port = config.port; + let (tx, rx) = mpsc::channel(); + let shutdown = Arc::new(AtomicBool::new(false)); + let s2 = Arc::clone(&shutdown); + std::thread::spawn(move || { + let _ = run_webhook_server(&config, tx, s2); + }); + // Wait for the listener rather than sleeping a fixed amount. + let deadline = Instant::now() + Duration::from_secs(5); + while Instant::now() < deadline { + if TcpStream::connect(("127.0.0.1", port)).is_ok() { + break; + } + std::thread::sleep(Duration::from_millis(10)); + } + Server { port, rx, shutdown } +} + +fn config_for(port: u16) -> WebhookConfig { + WebhookConfig { + port, + secret: Some(SECRET.to_string()), + ..WebhookConfig::default() + } +} + +/// Sign `body` for `path` and return the header value. +fn sign(path: &str, body: &[u8], t: i64) -> String { + let signed = canonical_payload(t, "POST", path, body); + format!("t={t},v1={}", compute_hmac_hex(SECRET.as_bytes(), &signed)) +} + +/// Send raw bytes, optionally in two writes separated by a pause. +fn send(port: u16, first: &[u8], second: Option<&[u8]>) -> String { + let mut s = TcpStream::connect(("127.0.0.1", port)).unwrap(); + s.write_all(first).unwrap(); + s.flush().unwrap(); + if let Some(rest) = second { + std::thread::sleep(Duration::from_millis(150)); + s.write_all(rest).unwrap(); + s.flush().unwrap(); + } + s.set_read_timeout(Some(Duration::from_secs(5))).ok(); + let mut out = Vec::new(); + let _ = s.read_to_end(&mut out); + String::from_utf8_lossy(&out).to_string() +} + +fn request_bytes(path: &str, body: &[u8], sig: Option<&str>) -> (Vec, Vec) { + let mut head = format!( + "POST {path} HTTP/1.1\r\nHost: localhost\r\nContent-Type: application/json\r\nContent-Length: {}\r\n", + body.len() + ); + if let Some(s) = sig { + head.push_str(&format!("X-Forjar-Signature: {s}\r\n")); + } + head.push_str("\r\n"); + (head.into_bytes(), body.to_vec()) +} + +// ── Happy path, including split delivery ───────────────────────────────────── + +#[test] +fn signed_delivery_is_accepted() { + let srv = start(config_for(free_port())); + let body = br#"{"action":"deploy"}"#; + let sig = sign("/webhook", body, unix_now()); + let (head, b) = request_bytes("/webhook", body, Some(&sig)); + let resp = send(srv.port, &[head, b].concat(), None); + + assert!(resp.starts_with("HTTP/1.1 200 OK"), "resp: {resp}"); + let ev = srv.rx.recv_timeout(Duration::from_secs(2)).expect("event"); + assert_eq!(ev.payload.get("action").map(String::as_str), Some("deploy")); +} + +/// A correctly-signed delivery whose body arrives in a SECOND segment. Measured +/// against the old server: 400, and the event never reached the channel. +#[test] +fn signed_delivery_split_across_segments_is_accepted() { + let srv = start(config_for(free_port())); + let body = br#"{"action":"deploy"}"#; + let sig = sign("/webhook", body, unix_now()); + let (head, b) = request_bytes("/webhook", body, Some(&sig)); + let resp = send(srv.port, &head, Some(&b)); + + assert!(resp.starts_with("HTTP/1.1 200 OK"), "resp: {resp}"); + assert!( + srv.rx.recv_timeout(Duration::from_secs(2)).is_ok(), + "event must reach the channel on a split delivery" + ); +} + +/// The MAC must cover the exact octets, including bytes that are not valid UTF-8. +/// The old path ran the body through `from_utf8_lossy` first, so this could never +/// verify. +#[test] +fn non_utf8_body_signed_over_wire_bytes_is_accepted() { + let mut cfg = config_for(free_port()); + cfg.allowed_paths = vec!["/webhook".into()]; + let srv = start(cfg); + // Valid JSON except for a raw 0xFF inside a string — deliberately non-UTF-8. + let body: Vec = b"{\"k\":\"a\xffb\"}".to_vec(); + assert!(std::str::from_utf8(&body).is_err()); + let sig = sign("/webhook", &body, unix_now()); + let (head, b) = request_bytes("/webhook", &body, Some(&sig)); + let resp = send(srv.port, &[head, b].concat(), None); + + // Signature verifies over the raw bytes, so this is NOT 401. The body then + // fails UTF-8/JSON parsing, which is a 400 — the point is that authentication + // succeeded on the exact octets. + assert!( + !resp.contains(" 401 "), + "signature must verify over raw bytes; got {resp}" + ); + assert!( + resp.contains(" 400 "), + "expected a body-parse 400; got {resp}" + ); +} + +// ── Authentication ─────────────────────────────────────────────────────────── + +#[test] +fn unsigned_request_is_401_when_a_secret_is_configured() { + let srv = start(config_for(free_port())); + let (head, b) = request_bytes("/webhook", b"{}", None); + let resp = send(srv.port, &[head, b].concat(), None); + assert!(resp.starts_with("HTTP/1.1 401"), "resp: {resp}"); + assert!(resp.contains("signature_missing")); +} + +/// A signature minted for one allowed path must not verify at another. With the +/// old body-only MAC it did — and `_path` reads like an authorization input. +#[test] +fn signature_for_one_path_is_rejected_at_another() { + let mut cfg = config_for(free_port()); + cfg.allowed_paths = vec!["/hooks/deploy".into(), "/hooks/destroy".into()]; + let srv = start(cfg); + + let body = br#"{"go":true}"#; + let sig = sign("/hooks/deploy", body, unix_now()); + let (head, b) = request_bytes("/hooks/destroy", body, Some(&sig)); + let resp = send(srv.port, &[head, b].concat(), None); + + assert!(resp.starts_with("HTTP/1.1 401"), "resp: {resp}"); + assert!(resp.contains("signature_invalid")); +} + +/// An old capture must not replay, even with a valid MAC. +#[test] +fn stale_timestamp_is_rejected() { + let srv = start(config_for(free_port())); + let body = br#"{"a":1}"#; + let sig = sign("/webhook", body, unix_now() - 3600); + let (head, b) = request_bytes("/webhook", body, Some(&sig)); + let resp = send(srv.port, &[head, b].concat(), None); + assert!(resp.starts_with("HTTP/1.1 401"), "resp: {resp}"); + assert!(resp.contains("signature_stale")); +} + +// ── Idempotency ────────────────────────────────────────────────────────────── + +/// The identical signed request twice: both answered 200, exactly ONE event. +/// A retrying sender did nothing wrong, so a 4xx would only make it retry harder. +#[test] +fn duplicate_delivery_yields_exactly_one_event() { + let srv = start(config_for(free_port())); + let body = br#"{"action":"once"}"#; + let sig = sign("/webhook", body, unix_now()); + let (head, b) = request_bytes("/webhook", body, Some(&sig)); + let raw = [head, b].concat(); + + let first = send(srv.port, &raw, None); + let second = send(srv.port, &raw, None); + + assert!(first.starts_with("HTTP/1.1 200"), "first: {first}"); + assert!(second.starts_with("HTTP/1.1 200"), "second: {second}"); + assert!(second.contains("duplicate_ignored"), "second: {second}"); + + assert!(srv.rx.recv_timeout(Duration::from_secs(2)).is_ok()); + assert!( + srv.rx.recv_timeout(Duration::from_millis(300)).is_err(), + "a duplicate must not produce a second event" + ); +} + +// ── Method and path ────────────────────────────────────────────────────────── + +#[test] +fn get_is_405_with_allow_header() { + let srv = start(config_for(free_port())); + let raw = b"GET /webhook HTTP/1.1\r\nHost: x\r\n\r\n"; + let resp = send(srv.port, raw, None); + assert!(resp.starts_with("HTTP/1.1 405"), "resp: {resp}"); + assert!(resp.contains("Allow: POST"), "resp: {resp}"); +} + +#[test] +fn unknown_path_is_404() { + let srv = start(config_for(free_port())); + let body = b"{}"; + let sig = sign("/nope", body, unix_now()); + let (head, b) = request_bytes("/nope", body, Some(&sig)); + let resp = send(srv.port, &[head, b].concat(), None); + assert!(resp.starts_with("HTTP/1.1 404"), "resp: {resp}"); + // Must NOT echo the requested path back. + assert!(!resp.contains("/nope"), "response reflected input: {resp}"); +} + +/// A query string on an allowed path is still that path. Exact matching on the +/// raw target made any query string a rejection. +#[test] +fn query_string_does_not_break_path_matching() { + let srv = start(config_for(free_port())); + let body = br#"{"a":1}"#; + let sig = sign("/webhook?src=gh", body, unix_now()); + let (head, b) = request_bytes("/webhook?src=gh", body, Some(&sig)); + let resp = send(srv.port, &[head, b].concat(), None); + assert!(resp.starts_with("HTTP/1.1 200"), "resp: {resp}"); +} + +// ── Pre-authentication denial of service ───────────────────────────────────── + +/// Two sockets that connect and send NOTHING must not delay a real delivery. +/// Measured against the old inline handler: one idle socket delayed a legitimate +/// signed request by 5383ms, entirely upstream of signature checking. +#[test] +fn idle_connections_do_not_delay_a_real_delivery() { + let srv = start(config_for(free_port())); + + let mut idle = Vec::new(); + for _ in 0..2 { + idle.push(TcpStream::connect(("127.0.0.1", srv.port)).unwrap()); + } + + let body = br#"{"action":"urgent"}"#; + let sig = sign("/webhook", body, unix_now()); + let (head, b) = request_bytes("/webhook", body, Some(&sig)); + + let started = Instant::now(); + let resp = send(srv.port, &[head, b].concat(), None); + let elapsed = started.elapsed(); + + assert!(resp.starts_with("HTTP/1.1 200"), "resp: {resp}"); + assert!( + elapsed < Duration::from_secs(2), + "idle sockets delayed a real delivery by {elapsed:?}" + ); + drop(idle); +} + +// ── Fail closed at startup ─────────────────────────────────────────────────── + +#[test] +fn refuses_to_start_without_a_secret() { + let cfg = WebhookConfig { + port: free_port(), + secret: None, + ..WebhookConfig::default() + }; + let (tx, _rx) = mpsc::channel(); + let err = run_webhook_server(&cfg, tx, Arc::new(AtomicBool::new(true))) + .expect_err("must refuse to start unauthenticated"); + assert!(err.contains("allow_unauthenticated"), "err: {err}"); +} + +#[test] +fn starts_without_a_secret_when_explicitly_allowed() { + let cfg = WebhookConfig { + port: free_port(), + secret: None, + allow_unauthenticated: true, + ..WebhookConfig::default() + }; + assert!(cfg.validate_startup().is_ok()); +} + +/// An empty allow-list denies everything, so starting with one is a +/// misconfiguration rather than a wide-open endpoint. +#[test] +fn refuses_to_start_with_an_empty_allow_list() { + let cfg = WebhookConfig { + allowed_paths: vec![], + secret: Some(SECRET.into()), + ..WebhookConfig::default() + }; + let err = cfg + .validate_startup() + .expect_err("empty allow-list must fail"); + assert!(err.contains("allowed_paths"), "err: {err}"); +} + +/// Signatures authenticate a sender; they do not encrypt. A non-loopback bind +/// needs the operator to say TLS is terminated in front. +#[test] +fn refuses_a_non_loopback_bind_without_upstream_tls() { + let cfg = WebhookConfig { + bind: "0.0.0.0".into(), + secret: Some(SECRET.into()), + ..WebhookConfig::default() + }; + let err = cfg + .validate_startup() + .expect_err("must refuse plaintext exposure"); + assert!(err.contains("tls_terminated_upstream"), "err: {err}"); + + let ok = WebhookConfig { + bind: "0.0.0.0".into(), + secret: Some(SECRET.into()), + tls_terminated_upstream: true, + ..WebhookConfig::default() + }; + assert!(ok.validate_startup().is_ok()); +} + +/// The secret must never reach a log line or a panic message. +#[test] +fn debug_redacts_the_secret() { + let cfg = WebhookConfig { + secret: Some("super-secret-value".into()), + ..WebhookConfig::default() + }; + let shown = format!("{cfg:?}"); + assert!(!shown.contains("super-secret-value"), "leaked: {shown}"); + assert!(shown.contains(""), "shown: {shown}"); +} + +/// Startup validation runs BEFORE the listener binds, so a bad config cannot +/// leave a socket open. +#[test] +fn bad_config_does_not_bind() { + let port = free_port(); + let cfg = WebhookConfig { + port, + secret: None, + ..WebhookConfig::default() + }; + let (tx, _rx) = mpsc::channel(); + assert!(run_webhook_server(&cfg, tx, Arc::new(AtomicBool::new(true))).is_err()); + // The port must still be bindable, proving nothing was left listening. + assert!( + TcpListener::bind(("127.0.0.1", port)).is_ok(), + "port {port} was left bound after a rejected config" + ); +} diff --git a/src/core/tests_webhook_sig.rs b/src/core/tests_webhook_sig.rs new file mode 100644 index 00000000..7c913206 --- /dev/null +++ b/src/core/tests_webhook_sig.rs @@ -0,0 +1,307 @@ +//! Tests for [`crate::core::webhook_sig`]. +//! +//! # Why these are known-answer tests +//! +//! The predecessor of this module was documented and named HMAC-SHA256 but +//! computed a keyed BLAKE3 hash, and shipped that way for months with ~6 passing +//! signature tests. Every one of them derived its expected value by calling the +//! function under test: +//! +//! ```ignore +//! let sig = compute_hmac_hex(secret, body); // f(x) +//! req.headers.insert("x-forjar-signature", sig); +//! assert!(validate_request(&config, &req).is_valid()); // == f(x) +//! ``` +//! +//! `f(x) == f(x)` holds for ANY function, so no amount of that could detect the +//! wrong algorithm. `hmac_deterministic` asserted only `h1 == h2` and +//! `h1.len() == 64` — and BLAKE3-256 hex is also 64 chars, so even the length +//! could not discriminate. +//! +//! Every expected value below therefore comes from **outside this crate**: RFC +//! 4231's published vectors, and digests generated with +//! `openssl dgst -sha256 -hmac`. If someone swaps the primitive again, these fail. + +use super::webhook_sig::*; + +// ── Known-answer vectors (RFC 4231) ────────────────────────────────────────── + +/// RFC 4231 test case 1: key = 20×0x0b, data = "Hi There". +#[test] +fn rfc4231_tc1() { + let key = [0x0bu8; 20]; + assert_eq!( + compute_hmac_hex(&key, b"Hi There"), + "b0344c61d8db38535ca8afceaf0bf12b881dc200c9833da726e9376c2e32cff7" + ); +} + +/// RFC 4231 test case 2: key = "Jefe". +/// +/// This is the vector that exposes the substituted primitive: the old +/// keyed-BLAKE3 implementation returned `30f3b0f1f2b72e19eefe2a08fc3af2bc…`. +/// Independently reproducible: +/// printf 'what do ya want for nothing?' | openssl dgst -sha256 -hmac 'Jefe' +#[test] +fn rfc4231_tc2() { + assert_eq!( + compute_hmac_hex(b"Jefe", b"what do ya want for nothing?"), + "5bdcc146bf60754e6a042426089575c75a003f089d2739839dec58b964ec3843" + ); +} + +/// RFC 4231 test case 3: key = 20×0xaa, data = 50×0xdd. +#[test] +fn rfc4231_tc3() { + let key = [0xaau8; 20]; + let data = [0xddu8; 50]; + assert_eq!( + compute_hmac_hex(&key, &data), + "773ea91e36800e46854db8ebd09181a72959098b3ef8c122d9635514ced565fe" + ); +} + +/// A key longer than the 64-byte block must be hashed first (RFC 4231 TC6). +#[test] +fn rfc4231_tc6_key_longer_than_block() { + let key = [0xaau8; 131]; + assert_eq!( + compute_hmac_hex( + &key, + b"Test Using Larger Than Block-Size Key - Hash Key First" + ), + "60e431591ee0b67f0d8a26aacbf5b77f8e0bc6213728c5140546040f0ee37f54" + ); +} + +/// Distinct DATA must give a distinct MAC. The old suite varied only the key, +/// so a function ignoring its data argument entirely would have passed. +#[test] +fn hmac_different_data() { + let a = compute_hmac_hex(b"key", b"data-one"); + let b = compute_hmac_hex(b"key", b"data-two"); + assert_ne!(a, b); +} + +/// Empty key and empty data are still well-defined, not a panic. +#[test] +fn hmac_handles_empty_inputs() { + // openssl dgst -sha256 -hmac '' = br#"{"a":"#.to_vec(); + body.push(0xff); + body.push(b'}'); + assert!( + std::str::from_utf8(&body).is_err(), + "fixture must be non-UTF-8" + ); + let sig = compute_hmac_hex(b"s3cret", &body); + assert!(verify_hex(b"s3cret", &body, &sig)); + + // And the lossy form must NOT verify, which is what the old code compared. + let lossy = String::from_utf8_lossy(&body).into_owned(); + assert!(!verify_hex(b"s3cret", lossy.as_bytes(), &sig)); +} + +// ── Canonical payload ──────────────────────────────────────────────────────── + +#[test] +fn canonical_payload_is_newline_separated() { + let p = canonical_payload(1785350000, "POST", "/webhook", b"{\"a\":1}"); + assert_eq!( + String::from_utf8(p).unwrap(), + "t=1785350000\nPOST\n/webhook\n{\"a\":1}" + ); +} + +/// The whole point of binding the path: a signature minted for one allowed path +/// must not verify at another. With a body-only MAC it did. +#[test] +fn signature_does_not_transfer_between_paths() { + let secret = b"s3cret"; + let body = b"{\"action\":\"go\"}"; + let t = 1785350000; + + let deploy = compute_hmac_hex(secret, &canonical_payload(t, "POST", "/hooks/deploy", body)); + let destroy_payload = canonical_payload(t, "POST", "/hooks/destroy", body); + + assert!(!verify_hex(secret, &destroy_payload, &deploy)); +} + +/// Likewise the method, so a signed POST cannot be replayed as another verb. +#[test] +fn signature_does_not_transfer_between_methods() { + let secret = b"s3cret"; + let body = b"{}"; + let t = 1785350000; + let post = compute_hmac_hex(secret, &canonical_payload(t, "POST", "/webhook", body)); + let put_payload = canonical_payload(t, "PUT", "/webhook", body); + assert!(!verify_hex(secret, &put_payload, &post)); +} + +/// A `.` in the path must not be able to shift the field boundary — the reason +/// for newline separators rather than Stripe's `t.payload`. +#[test] +fn dotted_path_cannot_shift_the_boundary() { + let a = canonical_payload(1, "POST", "/a.b", b"x"); + let b = canonical_payload(1, "POST", "/a", b".b\nx"); + assert_ne!(a, b); +} + +// ── Header parsing ─────────────────────────────────────────────────────────── + +#[test] +fn parse_signature_header_extracts_t_and_v1() { + let h = parse_forjar_signature("t=1785350000,v1=deadbeef"); + assert_eq!(h.timestamp, Some(1785350000)); + assert_eq!(h.v1, vec!["deadbeef".to_string()]); + assert!(h.has_v1()); +} + +/// Multiple v1 elements support secret rotation without a flag day. +#[test] +fn parse_signature_header_keeps_every_v1() { + let h = parse_forjar_signature("t=1,v1=aaa,v1=bbb"); + assert_eq!(h.v1, vec!["aaa".to_string(), "bbb".to_string()]); +} + +/// Unknown elements are ignored so the scheme can grow, but a header carrying no +/// v1 at all must be visible as such — NOT silently treated as unsigned. +#[test] +fn parse_signature_header_without_v1_is_not_signed() { + let h = parse_forjar_signature("t=1,v2=future,junk"); + assert!(!h.has_v1()); + assert_eq!(h.timestamp, Some(1)); +} + +#[test] +fn parse_signature_header_tolerates_whitespace_and_bad_t() { + let h = parse_forjar_signature(" t = 12 , v1 = abc "); + assert_eq!(h.timestamp, Some(12)); + assert_eq!(h.v1, vec!["abc".to_string()]); + assert_eq!(parse_forjar_signature("t=notanumber,v1=a").timestamp, None); +} + +#[test] +fn parse_github_signature_strips_the_prefix() { + assert_eq!( + parse_github_signature("sha256=abc123").as_deref(), + Some("abc123") + ); + assert!(parse_github_signature("sha1=abc123").is_none()); + assert!(parse_github_signature("abc123").is_none()); +} + +// ── Freshness ──────────────────────────────────────────────────────────────── + +#[test] +fn timestamp_freshness_window() { + let now = 1_000_000; + assert!(timestamp_is_fresh(now, now, 300)); + assert!(timestamp_is_fresh(now - 300, now, 300)); + assert!(!timestamp_is_fresh(now - 301, now, 300)); +} + +/// A future-dated timestamp is rejected too. Otherwise a sender with a fast +/// clock — or an attacker choosing `t` — could extend a captured request's life. +#[test] +fn timestamp_in_the_future_is_rejected() { + let now = 1_000_000; + assert!(timestamp_is_fresh(now + 300, now, 300)); + assert!(!timestamp_is_fresh(now + 301, now, 300)); +} + +// ── Replay guard ───────────────────────────────────────────────────────────── + +#[test] +fn replay_guard_admits_once() { + let mut g = ReplayGuard::new(300, 16); + assert!(g.admit("sig-a", 1000)); + assert!(!g.admit("sig-a", 1000), "second admit must be refused"); + assert!(g.admit("sig-b", 1000)); + assert_eq!(g.len(), 2); +} + +#[test] +fn replay_guard_expires_outside_the_window() { + let mut g = ReplayGuard::new(300, 16); + assert!(g.admit("sig-a", 1000)); + // 400s later the entry is outside the 300s window and is dropped, so the + // same digest is admissible again — by then the freshness check rejects it + // anyway, which is why persistence buys nothing. + assert!(g.admit("sig-b", 1400)); + assert!(!g.seen_contains("sig-a")); +} + +/// Bounded: an attacker sending distinct signatures must not grow memory without +/// limit. +#[test] +fn replay_guard_is_bounded() { + let mut g = ReplayGuard::new(300, 8); + for i in 0..100 { + assert!(g.admit(&format!("sig-{i}"), 1000)); + } + assert!(g.len() <= 8, "grew to {}", g.len()); +} + +#[test] +fn replay_guard_starts_empty() { + let g = ReplayGuard::new(300, 4); + assert!(g.is_empty()); + assert_eq!(g.len(), 0); +} diff --git a/src/core/webhook_http.rs b/src/core/webhook_http.rs new file mode 100644 index 00000000..f1a6e942 --- /dev/null +++ b/src/core/webhook_http.rs @@ -0,0 +1,295 @@ +//! FJ-3105: HTTP framing and response construction for the webhook receiver. +//! +//! # Why this module exists +//! +//! The previous server did ONE `stream.read()` and treated whatever followed the +//! first `\r\n\r\n` in that buffer as the whole body. `Content-Length` was parsed +//! into the header map and never read back. Measured: writing the head, flushing, +//! sleeping 200ms, then writing a correct body returned +//! `400 {"status":"invalid JSON: EOF while parsing a value..."}` and no event +//! reached the channel. With a secret configured the same truncation degraded into +//! `SignatureInvalid`, because the MAC was computed over the prefix. +//! +//! It passed its tests because they wrote a ~19-byte body in a single +//! `write_all` on loopback, which always lands in the first segment. +//! +//! Responses were also hand-built: `format!(r#"{{"status":"{message}"}}"#)` with +//! `{validation:?}` interpolated, so a rejection emitted +//! `{"status":"PathNotAllowed { path: "/evil" }"}` — invalid JSON, under +//! `Content-Type: application/json`, reflecting attacker-controlled input. And +//! every failure collapsed to 403 while the 401/405/413 arms of `status_reason` +//! sat unused. + +use std::io::Read; +use std::time::{Duration, Instant}; + +/// Largest request head (request line + headers) we will buffer, before 431. +pub const MAX_HEAD_BYTES: usize = 8 * 1024; + +/// How a read attempt ended. +#[derive(Debug, PartialEq, Eq)] +pub enum ReadOutcome { + /// A complete request. + Complete { + /// Request method, uppercased by the caller if needed. + method: String, + /// Request target exactly as sent, query string included. + path: String, + /// Header names lowercased; values trimmed. + headers: Vec<(String, String)>, + /// Exact body octets. NOT a String — the MAC must cover what was sent. + body: Vec, + }, + /// Client sent nothing / closed early. No response is owed. + Empty, + /// Malformed enough to answer with a status and close. + Rejected { + /// HTTP status to return. + status: u16, + /// Stable machine-readable reason code (never echoes input). + code: &'static str, + }, +} + +/// Read one HTTP request from `stream` under a whole-connection deadline. +/// +/// Framing rules, in order: +/// 1. Read until `\r\n\r\n`, capped at [`MAX_HEAD_BYTES`] → 431 if exceeded. +/// 2. Reject `Transfer-Encoding` with 501 — chunked decoding is not implemented, +/// and silently mis-framing a chunked body is worse than refusing it. +/// 3. Parse `Content-Length`: absent → 0; duplicate or non-numeric → 400; +/// greater than `max_body_bytes` → **413 before reading the body**, so the cap +/// actually bounds what we buffer. +/// 4. Read exactly that many octets, 408 if the deadline passes first. +pub fn read_request( + stream: &mut R, + max_body_bytes: usize, + deadline: Duration, +) -> ReadOutcome { + let started = Instant::now(); + let mut buf: Vec = Vec::with_capacity(1024); + let mut chunk = [0u8; 1024]; + + // ── 1. head ────────────────────────────────────────────────────────────── + let head_end = loop { + if let Some(pos) = find_head_end(&buf) { + break pos; + } + if buf.len() > MAX_HEAD_BYTES { + return ReadOutcome::Rejected { + status: 431, + code: "head_too_large", + }; + } + if started.elapsed() >= deadline { + return if buf.is_empty() { + ReadOutcome::Empty + } else { + ReadOutcome::Rejected { + status: 408, + code: "request_timeout", + } + }; + } + match stream.read(&mut chunk) { + Ok(0) => { + return if buf.is_empty() { + ReadOutcome::Empty + } else { + ReadOutcome::Rejected { + status: 400, + code: "incomplete_head", + } + }; + } + Ok(n) => buf.extend_from_slice(&chunk[..n]), + Err(ref e) if e.kind() == std::io::ErrorKind::WouldBlock => { + if started.elapsed() >= deadline { + return ReadOutcome::Rejected { + status: 408, + code: "request_timeout", + }; + } + } + Err(_) => { + return if buf.is_empty() { + ReadOutcome::Empty + } else { + ReadOutcome::Rejected { + status: 400, + code: "read_error", + } + }; + } + } + }; + + let head = String::from_utf8_lossy(&buf[..head_end]).to_string(); + let Some((method, path, headers)) = parse_head(&head) else { + return ReadOutcome::Rejected { + status: 400, + code: "malformed_request_line", + }; + }; + + // ── 2. no chunked ──────────────────────────────────────────────────────── + if headers.iter().any(|(k, _)| k == "transfer-encoding") { + return ReadOutcome::Rejected { + status: 501, + code: "transfer_encoding_unsupported", + }; + } + + // ── 3. Content-Length ──────────────────────────────────────────────────── + let content_length = match parse_content_length(&headers) { + Ok(n) => n, + Err(code) => { + return ReadOutcome::Rejected { status: 400, code }; + } + }; + if content_length > max_body_bytes { + return ReadOutcome::Rejected { + status: 413, + code: "body_too_large", + }; + } + + // ── 4. body ────────────────────────────────────────────────────────────── + let mut body: Vec = buf[head_end + 4..].to_vec(); + if body.len() > content_length { + body.truncate(content_length); + } + while body.len() < content_length { + if started.elapsed() >= deadline { + return ReadOutcome::Rejected { + status: 408, + code: "request_timeout", + }; + } + match stream.read(&mut chunk) { + Ok(0) => { + return ReadOutcome::Rejected { + status: 400, + code: "incomplete_body", + }; + } + Ok(n) => { + let want = content_length - body.len(); + body.extend_from_slice(&chunk[..n.min(want)]); + } + Err(ref e) if e.kind() == std::io::ErrorKind::WouldBlock => {} + Err(_) => { + return ReadOutcome::Rejected { + status: 400, + code: "read_error", + }; + } + } + } + + ReadOutcome::Complete { + method, + path, + headers, + body, + } +} + +/// Request line and headers, as parsed from the head block. +type ParsedHead = (String, String, Vec<(String, String)>); + +/// Locate the `\r\n\r\n` head/body boundary. +/// +/// CRLFCRLF only. The old code also accepted a bare `\n\n`, which is not a valid +/// HTTP message framing and creates a needless divergence from whatever proxy sits +/// in front. +fn find_head_end(buf: &[u8]) -> Option { + buf.windows(4).position(|w| w == b"\r\n\r\n") +} + +/// Split the request line and headers. +fn parse_head(head: &str) -> Option { + let mut lines = head.lines(); + let request_line = lines.next()?; + let mut parts = request_line.split_whitespace(); + let method = parts.next()?.to_string(); + let path = parts.next()?.to_string(); + + let mut headers = Vec::new(); + for line in lines { + let line = line.trim_end(); + if line.is_empty() { + break; + } + if let Some((k, v)) = line.split_once(':') { + headers.push((k.trim().to_lowercase(), v.trim().to_string())); + } + } + Some((method, path, headers)) +} + +/// Parse `Content-Length`, rejecting duplicates and non-numeric values. +/// +/// A duplicate `Content-Length` is a request-smuggling primitive when a proxy and +/// an origin disagree about which one wins, so it is refused rather than resolved. +/// The old header map silently kept the LAST occurrence. +fn parse_content_length(headers: &[(String, String)]) -> Result { + let mut found: Option = None; + for (k, v) in headers { + if k != "content-length" { + continue; + } + if found.is_some() { + return Err("duplicate_content_length"); + } + found = Some(v.parse::().map_err(|_| "invalid_content_length")?); + } + Ok(found.unwrap_or(0)) +} + +/// Build an HTTP response with a well-formed JSON body. +/// +/// `code` is a fixed reason code from this crate, never request-derived, so the +/// response cannot reflect attacker input. `serde_json` does the encoding, so the +/// body is valid JSON for every code path. +#[must_use] +pub fn response(status: u16, code: &str) -> Vec { + let body = serde_json::json!({ "status": code }).to_string(); + let reason = status_reason(status); + let mut out = format!( + "HTTP/1.1 {status} {reason}\r\n\ + Content-Type: application/json\r\n\ + Content-Length: {}\r\n\ + Connection: close\r\n", + body.len() + ); + // RFC 9110 9.5: 405 MUST carry Allow. Only POST is accepted. + if status == 405 { + out.push_str("Allow: POST\r\n"); + } + // 503 means the event pipeline is saturated; tell the sender to come back. + if status == 503 { + out.push_str("Retry-After: 5\r\n"); + } + out.push_str("\r\n"); + out.push_str(&body); + out.into_bytes() +} + +fn status_reason(code: u16) -> &'static str { + match code { + 200 => "OK", + 400 => "Bad Request", + 401 => "Unauthorized", + 403 => "Forbidden", + 404 => "Not Found", + 405 => "Method Not Allowed", + 408 => "Request Timeout", + 413 => "Content Too Large", + 431 => "Request Header Fields Too Large", + 500 => "Internal Server Error", + 501 => "Not Implemented", + 503 => "Service Unavailable", + _ => "Unknown", + } +} diff --git a/src/core/webhook_server.rs b/src/core/webhook_server.rs index 40a83aff..c5dd2853 100644 --- a/src/core/webhook_server.rs +++ b/src/core/webhook_server.rs @@ -1,391 +1,240 @@ -//! FJ-3105: Minimal webhook HTTP server using std::net. +//! FJ-3105: webhook HTTP server. //! -//! Accepts POST requests and converts them to [`InfraEvent`] values -//! via the validation pipeline in [`webhook_source`]. +//! # Concurrency +//! +//! Connections used to be handled INLINE on the accept loop behind a 5s read +//! timeout, so one client that connected and sent nothing stalled every other +//! delivery. Measured: a single idle socket delayed a legitimate signed request by +//! 5383ms, entirely upstream of `validate_request` — so the secret was no defence. +//! +//! Now a bounded pool of worker threads drains a bounded queue. Bounded rather +//! than thread-per-connection so an attacker cannot spawn threads, and a fixed +//! pool rather than tokio because this is a loopback endpoint taking a handful of +//! deliveries an hour: a synchronous accept loop with N workers is easier to +//! reason about, and easier to state as a contract, than dragging an async runtime +//! into it. use crate::core::types::InfraEvent; -use crate::core::webhook_source::{self, ack_response, WebhookConfig, WebhookRequest}; +use crate::core::webhook_http::{self, ReadOutcome}; +use crate::core::webhook_sig::{unix_now, ReplayGuard}; +use crate::core::webhook_source::{ + self, validate_request_at, ValidationResult, WebhookConfig, WebhookRequest, SIG_HEADER, +}; use std::collections::HashMap; -use std::io::{Read, Write}; -use std::net::TcpListener; +use std::io::Write; +use std::net::{TcpListener, TcpStream}; use std::sync::atomic::{AtomicBool, Ordering}; -use std::sync::mpsc::Sender; -use std::sync::Arc; +use std::sync::mpsc::{Sender, SyncSender, TrySendError}; +use std::sync::{Arc, Mutex}; +use std::time::Duration; + +/// Worker threads handling connections. +const WORKERS: usize = 4; +/// Queued connections awaiting a worker before we shed load with 503. +const QUEUE_DEPTH: usize = 32; +/// Whole-connection deadline. +const CONN_DEADLINE: Duration = Duration::from_secs(5); +/// Retained replay entries. +const REPLAY_CAPACITY: usize = 4096; + +/// Shared state each worker needs. +struct Shared { + config: WebhookConfig, + sender: Sender, + replay: Mutex, +} -/// Start the webhook server on the configured port. +/// Start the webhook server, blocking until `shutdown` is set. /// -/// Blocks the calling thread, accepting connections until `shutdown` -/// is set to `true`. Valid events are forwarded through `sender`. +/// Returns `Err` without binding if the configuration would expose an +/// unauthenticated or plaintext endpoint. pub fn run_webhook_server( config: &WebhookConfig, sender: Sender, shutdown: Arc, ) -> Result<(), String> { - let addr = format!("127.0.0.1:{}", config.port); + config.validate_startup()?; + + let addr = format!("{}:{}", config.bind, config.port); let listener = TcpListener::bind(&addr).map_err(|e| format!("bind {addr}: {e}"))?; listener .set_nonblocking(true) .map_err(|e| format!("nonblocking: {e}"))?; + let shared = Arc::new(Shared { + config: config.clone(), + sender, + replay: Mutex::new(ReplayGuard::new( + config.signature_tolerance_secs, + REPLAY_CAPACITY, + )), + }); + + let (tx, rx) = std::sync::mpsc::sync_channel::<(TcpStream, String)>(QUEUE_DEPTH); + let rx = Arc::new(Mutex::new(rx)); + let mut handles = Vec::with_capacity(WORKERS); + for _ in 0..WORKERS { + let rx = Arc::clone(&rx); + let shared = Arc::clone(&shared); + handles.push(std::thread::spawn(move || { + loop { + // Hold the lock only to dequeue, never while serving. + let next = { + let guard = match rx.lock() { + Ok(g) => g, + Err(_) => break, + }; + guard.recv() + }; + match next { + Ok((stream, peer)) => serve(stream, &peer, &shared), + Err(_) => break, // sender dropped: shutting down + } + } + })); + } + + accept_loop(&listener, &tx, &shutdown); + + drop(tx); + for h in handles { + let _ = h.join(); + } + Ok(()) +} + +/// Accept connections and hand them to the pool, shedding load when it is full. +fn accept_loop( + listener: &TcpListener, + tx: &SyncSender<(TcpStream, String)>, + shutdown: &Arc, +) { + let mut backoff = Duration::from_millis(50); while !shutdown.load(Ordering::Relaxed) { match listener.accept() { Ok((stream, peer)) => { - handle_connection(stream, &peer.to_string(), config, &sender); + backoff = Duration::from_millis(50); + if let Err(TrySendError::Full((mut stream, _))) = + tx.try_send((stream, peer.to_string())) + { + // Shed rather than queue without limit, and SAY so — silently + // dropping would look identical to a lost packet. + let _ = stream.write_all(&webhook_http::response(503, "queue_full")); + let _ = stream.flush(); + } } Err(ref e) if e.kind() == std::io::ErrorKind::WouldBlock => { - std::thread::sleep(std::time::Duration::from_millis(50)); + std::thread::sleep(Duration::from_millis(50)); } Err(e) => { + // Back off. A persistent accept error (EMFILE) used to spin this + // loop at full tilt with no delay. eprintln!("webhook accept error: {e}"); + std::thread::sleep(backoff); + backoff = (backoff * 2).min(Duration::from_secs(5)); } } } - Ok(()) } -/// Process a single accepted connection. -fn handle_connection( - mut stream: std::net::TcpStream, - peer: &str, - config: &WebhookConfig, - sender: &Sender, -) { - stream - .set_read_timeout(Some(std::time::Duration::from_secs(5))) - .ok(); - let buf_size = config.max_body_bytes.min(65536) + 4096; - let mut buf = vec![0u8; buf_size]; - let n = match stream.read(&mut buf) { - Ok(n) if n > 0 => n, - _ => return, +/// Read, validate and dispatch one connection. +fn serve(mut stream: TcpStream, peer: &str, shared: &Shared) { + // Propagated, not `.ok()`-swallowed: without a read timeout the deadline below + // cannot be enforced and a worker could block forever. + if let Err(e) = stream.set_read_timeout(Some(CONN_DEADLINE)) { + eprintln!("webhook: set_read_timeout failed for {peer}: {e}"); + let _ = stream.write_all(&webhook_http::response(500, "internal_error")); + return; + } + + let outcome = + webhook_http::read_request(&mut stream, shared.config.max_body_bytes, CONN_DEADLINE); + + let reply = match outcome { + ReadOutcome::Empty => return, // nothing sent; nothing owed + ReadOutcome::Rejected { status, code } => webhook_http::response(status, code), + ReadOutcome::Complete { + method, + path, + headers, + body, + } => { + let request = WebhookRequest { + method, + path, + headers: headers.into_iter().collect::>(), + body, + // Address only. The old code stored `ip:port`, so any documented + // IP match in a rulebook could never fire. + source_ip: Some(peer.rsplit_once(':').map_or(peer, |(ip, _)| ip).to_string()), + }; + dispatch(&request, shared) + } }; - let raw = String::from_utf8_lossy(&buf[..n]); - match parse_http_to_webhook(&raw, peer) { - Ok(req) => { - let resp = dispatch_request(&req, config, sender); - let _ = stream.write_all(resp.as_bytes()); - } - Err(resp) => { - let _ = stream.write_all(resp.as_bytes()); - } - } + let _ = stream.write_all(&reply); + let _ = stream.flush(); } -/// Validate the request and dispatch to the event pipeline. -/// Returns the HTTP response string. -fn dispatch_request( - req: &WebhookRequest, - config: &WebhookConfig, - sender: &Sender, -) -> String { - let validation = webhook_source::validate_request(config, req); +/// Validate, de-duplicate, and forward one request. +fn dispatch(request: &WebhookRequest, shared: &Shared) -> Vec { + let now = unix_now(); + let validation = validate_request_at(&shared.config, request, now); if !validation.is_valid() { - return ack_response(403, &format!("{validation:?}")); + // Debug form server-side only; the response carries a fixed code. + eprintln!("webhook: rejected {:?}", validation); + return webhook_http::response(validation.status(), validation.code()); + } + + // Idempotency. Because `t` is inside the signed payload, the v1 digest is + // unique per send, so it doubles as the delivery id — the sender needs no + // extra header to specify or get wrong. A duplicate is answered 200, never + // 4xx: a retrying sender did nothing wrong, and a 4xx would make it retry + // harder. + let event_id = delivery_id(request); + if let Some(id) = &event_id { + match shared.replay.lock() { + Ok(mut guard) => { + if !guard.admit(id, now) { + return webhook_http::response(200, "duplicate_ignored"); + } + } + Err(_) => return webhook_http::response(500, "internal_error"), + } } - match webhook_source::request_to_event(req) { + match webhook_source::request_to_event( + request, + shared.config.machine.as_deref(), + event_id.as_deref(), + ) { Ok(event) => { - let _ = sender.send(event); - ack_response(200, "accepted") + // A dropped receiver used to be acknowledged as success: the old code + // did `let _ = sender.send(event)` then returned 200, so a sender was + // told its delivery was accepted when nothing would ever process it. + if shared.sender.send(event).is_err() { + return webhook_http::response(503, "event_pipeline_closed"); + } + webhook_http::response(200, "accepted") + } + Err(e) => { + eprintln!("webhook: body rejected: {e}"); + webhook_http::response(400, "invalid_body") } - Err(e) => ack_response(400, &e), } } -/// Parse raw HTTP text into a [`WebhookRequest`]. -/// -/// Returns `Err(response)` with a pre-formatted HTTP error on parse failure. -fn parse_http_to_webhook(raw: &str, peer: &str) -> Result { - let (head, body) = split_head_body(raw); - let mut lines = head.lines(); - - // Request line: "POST /webhook HTTP/1.1" - let request_line = lines +/// Delivery id: the first `v1` digest, which is unique per send. +fn delivery_id(request: &WebhookRequest) -> Option { + let raw = request.headers.get(SIG_HEADER)?; + crate::core::webhook_sig::parse_forjar_signature(raw) + .v1 + .into_iter() .next() - .ok_or_else(|| ack_response(400, "empty request"))?; - let parts: Vec<&str> = request_line.split_whitespace().collect(); - if parts.len() < 2 { - return Err(ack_response(400, "malformed request line")); - } - let method = parts[0].to_string(); - let path = parts[1].to_string(); - - // Headers - let headers = parse_headers(lines); - - Ok(WebhookRequest { - method, - path, - headers, - body: body.to_string(), - source_ip: Some(peer.to_string()), - }) -} - -/// Split raw HTTP at the `\r\n\r\n` boundary into head and body. -fn split_head_body(raw: &str) -> (&str, &str) { - if let Some(pos) = raw.find("\r\n\r\n") { - (&raw[..pos], &raw[pos + 4..]) - } else if let Some(pos) = raw.find("\n\n") { - (&raw[..pos], &raw[pos + 2..]) - } else { - (raw, "") - } } -/// Parse header lines into a lowercase-keyed map. -fn parse_headers<'a>(lines: impl Iterator) -> HashMap { - let mut headers = HashMap::new(); - for line in lines { - let trimmed = line.trim(); - if trimmed.is_empty() { - break; - } - if let Some((k, v)) = trimmed.split_once(':') { - headers.insert(k.trim().to_lowercase(), v.trim().to_string()); - } - } - headers -} - -#[cfg(test)] -mod tests { - use super::*; - use std::net::TcpStream; - use std::sync::atomic::AtomicBool; - use std::sync::mpsc; - - /// Find an available port by binding to :0. - fn free_port() -> u16 { - let listener = TcpListener::bind("127.0.0.1:0").unwrap(); - listener.local_addr().unwrap().port() - } - - /// Spin up the server in a background thread and return (port, rx, shutdown). - fn start_server(config: WebhookConfig) -> (u16, mpsc::Receiver, Arc) { - let port = config.port; - let (tx, rx) = mpsc::channel(); - let shutdown = Arc::new(AtomicBool::new(false)); - let shutdown_clone = Arc::clone(&shutdown); - std::thread::spawn(move || { - let _ = run_webhook_server(&config, tx, shutdown_clone); - }); - // Give the listener time to bind. - std::thread::sleep(std::time::Duration::from_millis(100)); - (port, rx, shutdown) - } - - fn send_raw(port: u16, raw: &str) -> String { - let mut stream = TcpStream::connect(format!("127.0.0.1:{port}")).unwrap(); - stream.write_all(raw.as_bytes()).unwrap(); - stream - .set_read_timeout(Some(std::time::Duration::from_secs(2))) - .ok(); - let mut buf = vec![0u8; 4096]; - let n = stream.read(&mut buf).unwrap_or(0); - String::from_utf8_lossy(&buf[..n]).to_string() - } - - fn post_json(port: u16, path: &str, body: &str) -> String { - let raw = format!( - "POST {path} HTTP/1.1\r\n\ - Host: localhost\r\n\ - Content-Type: application/json\r\n\ - Content-Length: {}\r\n\ - \r\n\ - {body}", - body.len() - ); - send_raw(port, &raw) - } - - #[test] - fn server_accepts_valid_post() { - let port = free_port(); - let config = WebhookConfig { - port, - ..WebhookConfig::default() - }; - let (_port, rx, shutdown) = start_server(config); - - let resp = post_json(port, "/webhook", r#"{"action":"deploy"}"#); - assert!(resp.contains("200 OK"), "response: {resp}"); - - let event = rx.recv_timeout(std::time::Duration::from_secs(1)).unwrap(); - assert_eq!(event.payload.get("action").unwrap(), "deploy"); - - shutdown.store(true, Ordering::Relaxed); - } - - #[test] - fn server_rejects_bad_path() { - let port = free_port(); - let config = WebhookConfig { - port, - ..WebhookConfig::default() - }; - let (_port, _rx, shutdown) = start_server(config); - - let resp = post_json(port, "/evil", r#"{}"#); - assert!(resp.contains("403"), "response: {resp}"); - - shutdown.store(true, Ordering::Relaxed); - } - - #[test] - fn server_rejects_get() { - let port = free_port(); - let config = WebhookConfig { - port, - ..WebhookConfig::default() - }; - let (_port, _rx, shutdown) = start_server(config); - - let raw = "GET /webhook HTTP/1.1\r\nHost: localhost\r\n\r\n".to_string(); - let resp = send_raw(port, &raw); - assert!(resp.contains("403"), "response: {resp}"); - - shutdown.store(true, Ordering::Relaxed); - } - - #[test] - fn server_rejects_invalid_json_body() { - let port = free_port(); - let config = WebhookConfig { - port, - ..WebhookConfig::default() - }; - let (_port, _rx, shutdown) = start_server(config); - - let resp = post_json(port, "/webhook", "not json"); - assert!(resp.contains("400"), "response: {resp}"); - - shutdown.store(true, Ordering::Relaxed); - } - - #[test] - fn server_shutdown_flag() { - let port = free_port(); - let config = WebhookConfig { - port, - ..WebhookConfig::default() - }; - let (tx, _rx) = mpsc::channel(); - let shutdown = Arc::new(AtomicBool::new(true)); // pre-set - let result = run_webhook_server(&config, tx, shutdown); - assert!(result.is_ok()); - } - - // -- Unit tests for parse helpers -- - - #[test] - fn parse_http_valid() { - let raw = "POST /webhook HTTP/1.1\r\n\ - Host: localhost\r\n\ - X-Custom: hello\r\n\ - \r\n\ - {\"a\":1}"; - let req = parse_http_to_webhook(raw, "1.2.3.4:5678").unwrap(); - assert_eq!(req.method, "POST"); - assert_eq!(req.path, "/webhook"); - assert_eq!(req.headers.get("x-custom").unwrap(), "hello"); - assert_eq!(req.body, r#"{"a":1}"#); - assert_eq!(req.source_ip.as_deref(), Some("1.2.3.4:5678")); - } - - #[test] - fn parse_http_empty() { - let result = parse_http_to_webhook("", "peer"); - assert!(result.is_err()); - } - - #[test] - fn parse_http_malformed_request_line() { - let result = parse_http_to_webhook("BADLINE", "peer"); - assert!(result.is_err()); - } - - #[test] - fn split_head_body_crlf() { - let (head, body) = split_head_body("HEAD\r\n\r\nBODY"); - assert_eq!(head, "HEAD"); - assert_eq!(body, "BODY"); - } - - #[test] - fn split_head_body_lf_only() { - let (head, body) = split_head_body("HEAD\n\nBODY"); - assert_eq!(head, "HEAD"); - assert_eq!(body, "BODY"); - } - - #[test] - fn split_head_body_no_separator() { - let (head, body) = split_head_body("HEADONLY"); - assert_eq!(head, "HEADONLY"); - assert_eq!(body, ""); - } - - #[test] - fn parse_headers_lowercased() { - let lines = vec!["Content-Type: text/plain", "X-UPPER: VALUE"]; - let headers = parse_headers(lines.into_iter()); - assert_eq!(headers.get("content-type").unwrap(), "text/plain"); - assert_eq!(headers.get("x-upper").unwrap(), "VALUE"); - } - - #[test] - fn parse_headers_stops_on_blank() { - let lines = vec!["Key: Val", "", "After: Blank"]; - let headers = parse_headers(lines.into_iter()); - assert_eq!(headers.len(), 1); - } - - #[test] - fn dispatch_valid_request() { - let config = WebhookConfig::default(); - let (tx, rx) = mpsc::channel(); - let req = WebhookRequest { - method: "POST".into(), - path: "/webhook".into(), - headers: HashMap::new(), - body: r#"{"k":"v"}"#.into(), - source_ip: None, - }; - let resp = dispatch_request(&req, &config, &tx); - assert!(resp.contains("200")); - let event = rx.try_recv().unwrap(); - assert_eq!(event.payload.get("k").unwrap(), "v"); - } - - #[test] - fn dispatch_forbidden_path() { - let config = WebhookConfig::default(); - let (tx, _rx) = mpsc::channel(); - let req = WebhookRequest { - method: "POST".into(), - path: "/nope".into(), - headers: HashMap::new(), - body: r#"{}"#.into(), - source_ip: None, - }; - let resp = dispatch_request(&req, &config, &tx); - assert!(resp.contains("403")); - } - - #[test] - fn dispatch_bad_json() { - let config = WebhookConfig::default(); - let (tx, _rx) = mpsc::channel(); - let req = WebhookRequest { - method: "POST".into(), - path: "/webhook".into(), - headers: HashMap::new(), - body: "not json".into(), - source_ip: None, - }; - let resp = dispatch_request(&req, &config, &tx); - assert!(resp.contains("400")); - } +/// Validation outcome for a request, exposed for the CLI's dry-run path. +#[must_use] +pub fn validate_for_display(config: &WebhookConfig, request: &WebhookRequest) -> ValidationResult { + validate_request_at(config, request, unix_now()) } diff --git a/src/core/webhook_sig.rs b/src/core/webhook_sig.rs new file mode 100644 index 00000000..6a897201 --- /dev/null +++ b/src/core/webhook_sig.rs @@ -0,0 +1,296 @@ +//! FJ-3104: webhook request authentication. +//! +//! # What replaced what +//! +//! This module exists because `webhook_source::compute_hmac_hex` was documented +//! as HMAC-SHA256 — its doc comment even described the ipad/opad construction — +//! while its body computed a keyed BLAKE3 hash. Measured against RFC 4231 TC2 +//! (`key = "Jefe"`, `data = "what do ya want for nothing?"`) the old function +//! returned `30f3b0f1…` where HMAC-SHA256 is `5bdcc146…`, so no sender using +//! standard tooling could ever authenticate. +//! +//! The keyed-BLAKE3 was a sound MAC; the defect was truthfulness and +//! interoperability. It survived because **every** signature test computed its +//! expected value by calling the function under test — `f(x) == f(x)`, which +//! cannot detect a wrong algorithm. The tests here are pinned to RFC 4231 +//! vectors and to literals generated by `openssl dgst -sha256 -hmac`, i.e. to an +//! oracle outside this crate. +//! +//! # Canonical signed payload +//! +//! A MAC over the body alone replays forever, and with more than one entry in +//! `allowed_paths` a signature minted for `/hooks/deploy` verifies unchanged at +//! `/hooks/destroy`. So the signed bytes bind the timestamp, method and path: +//! +//! ```text +//! t=\n\n\n +//! ``` +//! +//! Newline-separated rather than Stripe's `t.payload`, because a request path may +//! contain `.` and must not be able to shift the boundary. Neither the method nor +//! the path can contain a newline (HTTP forbids it in the request line), so the +//! encoding is unambiguous. +//! +//! Header, matching Stripe's shape so a comma-separated list stays extensible: +//! +//! ```text +//! X-Forjar-Signature: t=1785350000,v1=<64 hex chars> +//! ``` +//! +//! GitHub's `X-Hub-Signature-256: sha256=` is also accepted, over the bare +//! body, because GitHub cannot be told to sign a custom canonical form. + +use hmac::{Hmac, Mac}; +use sha2::Sha256; +use std::collections::{HashSet, VecDeque}; + +type HmacSha256 = Hmac; + +/// Length of a SHA-256 MAC in bytes. +const MAC_LEN: usize = 32; + +/// Default replay/freshness window in seconds. +pub const DEFAULT_TOLERANCE_SECS: u64 = 300; + +/// Compute HMAC-SHA256 of `data` under `key`, lowercase hex. +/// +/// Takes bytes, not `&str`: the MAC must cover the exact octets the sender +/// signed. The previous `&str` signature made that impossible by construction, +/// because the server had already run the body through `from_utf8_lossy`. +#[must_use] +pub fn compute_hmac_hex(key: &[u8], data: &[u8]) -> String { + let mut mac = HmacSha256::new_from_slice(key).expect("HMAC accepts any key length"); + mac.update(data); + to_hex(&mac.finalize().into_bytes()) +} + +/// Verify `provided_hex` against the MAC of `data` under `key`. +/// +/// Constant-time: the digest is hex-decoded and handed to `Mac::verify_slice`, +/// which compares under `subtle`'s constant-time equality. The previous code did +/// `sig != &expected` on `String`, which short-circuits on the first differing +/// byte and leaks the length of the matching prefix. +/// +/// Hex-decoding first also makes comparison case-insensitive, so an uppercase-hex +/// signature from a correct implementation is accepted rather than rejected. +#[must_use] +pub fn verify_hex(key: &[u8], data: &[u8], provided_hex: &str) -> bool { + let Some(provided) = decode_hex(provided_hex) else { + return false; + }; + let mut mac = HmacSha256::new_from_slice(key).expect("HMAC accepts any key length"); + mac.update(data); + mac.verify_slice(&provided).is_ok() +} + +/// Build the canonical signed payload: `t=\n\n\n`. +#[must_use] +pub fn canonical_payload(timestamp: i64, method: &str, path: &str, body: &[u8]) -> Vec { + let mut out = Vec::with_capacity(body.len() + method.len() + path.len() + 32); + out.extend_from_slice(format!("t={timestamp}\n").as_bytes()); + out.extend_from_slice(method.as_bytes()); + out.push(b'\n'); + out.extend_from_slice(path.as_bytes()); + out.push(b'\n'); + out.extend_from_slice(body); + out +} + +/// Build an `X-Forjar-Signature` header value for a request. +/// +/// The counterpart to verification, exposed because a SENDER needs it: without +/// it, every caller re-derives the canonical form by hand and any divergence +/// shows up as an opaque 401. Three test files had each grown their own copy. +#[must_use] +pub fn sign_request( + secret: &[u8], + method: &str, + path: &str, + body: &[u8], + timestamp: i64, +) -> String { + let signed = canonical_payload(timestamp, method, path, body); + format!("t={timestamp},v1={}", compute_hmac_hex(secret, &signed)) +} + +/// A parsed `X-Forjar-Signature` header. +#[derive(Debug, Clone, Default, PartialEq, Eq)] +pub struct SignatureHeader { + /// The `t=` element, if present and numeric. + pub timestamp: Option, + /// Every `v1=` element, in order. More than one is allowed so a sender can + /// rotate secrets without a flag day. + pub v1: Vec, +} + +impl SignatureHeader { + /// Whether the header carried at least one `v1=` element. + #[must_use] + pub fn has_v1(&self) -> bool { + !self.v1.is_empty() + } +} + +/// Parse `t=,v1=[,v1=…]`. +/// +/// Unknown elements are ignored so the scheme can grow (Stripe does the same), +/// but a header with no `v1` at all yields `has_v1() == false` and must be +/// rejected by the caller rather than treated as unsigned. +#[must_use] +pub fn parse_forjar_signature(raw: &str) -> SignatureHeader { + let mut out = SignatureHeader::default(); + for element in raw.split(',') { + let Some((k, v)) = element.split_once('=') else { + continue; + }; + match k.trim() { + "t" => out.timestamp = v.trim().parse::().ok(), + "v1" => out.v1.push(v.trim().to_string()), + _ => {} + } + } + out +} + +/// Extract the hex digest from GitHub's `X-Hub-Signature-256: sha256=`. +#[must_use] +pub fn parse_github_signature(raw: &str) -> Option { + raw.trim() + .strip_prefix("sha256=") + .map(|h| h.trim().to_string()) +} + +/// Whether `timestamp` is inside `tolerance_secs` of `now`. +/// +/// Absolute difference, so a clock ahead of the receiver is rejected too — a +/// future-dated timestamp would otherwise extend a captured request's validity. +#[must_use] +pub fn timestamp_is_fresh(timestamp: i64, now: i64, tolerance_secs: u64) -> bool { + let skew = now.saturating_sub(timestamp).unsigned_abs(); + skew <= tolerance_secs +} + +/// Seconds since the Unix epoch. +#[must_use] +pub fn unix_now() -> i64 { + std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .map(|d| i64::try_from(d.as_secs()).unwrap_or(i64::MAX)) + .unwrap_or(0) +} + +/// Bounded, expiring set of already-seen signatures. +/// +/// Because the timestamp is inside the signed payload, the `v1` digest is unique +/// per send, so it doubles as the delivery id and the sender needs no extra +/// header to get wrong. `CooldownTracker` is not a substitute: it rate-limits a +/// rulebook, it does not make a delivery exactly-once. +/// +/// In-memory rather than persisted: any replay older than the tolerance window is +/// already rejected by the freshness check, so persistence would only cover a +/// ≤`tolerance` sliver after a restart, in exchange for a schema and a migration. +#[derive(Debug)] +pub struct ReplayGuard { + seen: HashSet, + order: VecDeque<(i64, String)>, + tolerance_secs: u64, + capacity: usize, +} + +impl ReplayGuard { + /// Create a guard holding at most `capacity` entries. + #[must_use] + pub fn new(tolerance_secs: u64, capacity: usize) -> Self { + Self { + seen: HashSet::new(), + order: VecDeque::new(), + tolerance_secs, + capacity: capacity.max(1), + } + } + + /// Record `signature`; returns `false` if it was already seen. + pub fn admit(&mut self, signature: &str, now: i64) -> bool { + self.expire(now); + if !self.seen.insert(signature.to_string()) { + return false; + } + self.order.push_back((now, signature.to_string())); + while self.order.len() > self.capacity { + if let Some((_, old)) = self.order.pop_front() { + self.seen.remove(&old); + } + } + true + } + + /// Number of retained entries. + #[must_use] + pub fn len(&self) -> usize { + self.seen.len() + } + + /// Whether the guard is empty. + #[must_use] + pub fn is_empty(&self) -> bool { + self.seen.is_empty() + } + + /// Whether `signature` is currently retained. Lets a caller distinguish + /// "refused as a duplicate" from "expired and admissible again". + #[must_use] + pub fn seen_contains(&self, signature: &str) -> bool { + self.seen.contains(signature) + } + + fn expire(&mut self, now: i64) { + let cutoff = now.saturating_sub(i64::try_from(self.tolerance_secs).unwrap_or(i64::MAX)); + while let Some((seen_at, _)) = self.order.front() { + if *seen_at >= cutoff { + break; + } + if let Some((_, old)) = self.order.pop_front() { + self.seen.remove(&old); + } + } + } +} + +/// Lowercase hex encoding. +fn to_hex(bytes: &[u8]) -> String { + let mut s = String::with_capacity(bytes.len() * 2); + for b in bytes { + use std::fmt::Write; + let _ = write!(s, "{b:02x}"); + } + s +} + +/// Decode exactly `MAC_LEN` bytes of hex, accepting either case. +/// +/// Rejects the wrong length and any non-hex character BEFORE the comparison, so +/// a malformed signature is a clean `false` rather than a panic or a truncated +/// compare. +fn decode_hex(s: &str) -> Option> { + let s = s.trim(); + if s.len() != MAC_LEN * 2 { + return None; + } + let bytes = s.as_bytes(); + let mut out = Vec::with_capacity(MAC_LEN); + for pair in bytes.chunks(2) { + let hi = hex_val(pair[0])?; + let lo = hex_val(pair[1])?; + out.push((hi << 4) | lo); + } + Some(out) +} + +fn hex_val(c: u8) -> Option { + match c { + b'0'..=b'9' => Some(c - b'0'), + b'a'..=b'f' => Some(c - b'a' + 10), + b'A'..=b'F' => Some(c - b'A' + 10), + _ => None, + } +} diff --git a/src/core/webhook_source.rs b/src/core/webhook_source.rs index 951a7cbd..2f36a0dc 100644 --- a/src/core/webhook_source.rs +++ b/src/core/webhook_source.rs @@ -1,48 +1,148 @@ -//! FJ-3104: Webhook event source. +//! FJ-3104: Webhook event source — config, request type, and validation policy. //! -//! Parses incoming HTTP webhook requests and converts them to -//! InfraEvent values for the rules engine. Provides request -//! validation (HMAC signatures) and payload extraction. +//! Signature primitives live in [`crate::core::webhook_sig`]; framing and response +//! construction in [`crate::core::webhook_http`]. +//! +//! # Fail closed +//! +//! This receiver turns an inbound HTTP request into an [`InfraEvent`] that drives +//! the rules engine, which can run scripts on machines. Two independent fail-open +//! defaults used to guard that: `secret` defaulted to `None` (skipping signature +//! checking entirely), and an EMPTY `allowed_paths` meant allow-every-path — +//! measured, `POST /anything-at-all` returned `Valid`. So the configuration an +//! operator would write to lock the endpoint down was the least restrictive one +//! available. Both now deny. use crate::core::types::{EventType, InfraEvent}; +use crate::core::webhook_sig::{ + self, canonical_payload, timestamp_is_fresh, unix_now, DEFAULT_TOLERANCE_SECS, +}; use std::collections::HashMap; +/// Header carrying forjar's own `t=…,v1=…` signature. +pub const SIG_HEADER: &str = "x-forjar-signature"; +/// GitHub's signature header, accepted for interoperability. +pub const GITHUB_SIG_HEADER: &str = "x-hub-signature-256"; + /// Configuration for a webhook endpoint. -#[derive(Debug, Clone)] +#[derive(Clone)] pub struct WebhookConfig { + /// Address to bind. Defaults to loopback; a non-loopback bind must be paired + /// with `tls_terminated_upstream`. + pub bind: String, /// Port to listen on. pub port: u16, - /// Optional HMAC-SHA256 secret for request validation. + /// HMAC-SHA256 shared secret. `None` is only legal with + /// `allow_unauthenticated`. pub secret: Option, /// Maximum request body size in bytes. pub max_body_bytes: usize, - /// Allowed source paths (e.g., "/hooks/deploy"). + /// Allowed request paths. EMPTY MEANS DENY ALL. pub allowed_paths: Vec, + /// Freshness window for `t=` in seconds. + pub signature_tolerance_secs: u64, + /// Operator must say so out loud to run without a secret. + pub allow_unauthenticated: bool, + /// Operator asserts TLS is terminated in front of a non-loopback bind. + pub tls_terminated_upstream: bool, + /// Machine name stamped onto produced events. + pub machine: Option, } impl Default for WebhookConfig { fn default() -> Self { Self { + bind: "127.0.0.1".to_string(), port: 8484, secret: None, max_body_bytes: 1024 * 64, // 64 KiB allowed_paths: vec!["/webhook".to_string()], + signature_tolerance_secs: DEFAULT_TOLERANCE_SECS, + allow_unauthenticated: false, + tls_terminated_upstream: false, + machine: None, + } + } +} + +/// Hand-written so the shared secret never reaches a log or a panic message. +/// `derive(Debug)` printed it verbatim, and `dispatch_request` used to interpolate +/// `{validation:?}` straight into the HTTP response body. +impl std::fmt::Debug for WebhookConfig { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("WebhookConfig") + .field("bind", &self.bind) + .field("port", &self.port) + .field( + "secret", + &self.secret.as_ref().map(|_| "").unwrap_or("None"), + ) + .field("max_body_bytes", &self.max_body_bytes) + .field("allowed_paths", &self.allowed_paths) + .field("signature_tolerance_secs", &self.signature_tolerance_secs) + .field("allow_unauthenticated", &self.allow_unauthenticated) + .field("tls_terminated_upstream", &self.tls_terminated_upstream) + .field("machine", &self.machine) + .finish() + } +} + +impl WebhookConfig { + /// Reject a configuration that would expose an unauthenticated or plaintext + /// endpoint, BEFORE the listener binds. + /// + /// Refusing at startup rather than warning: the failure mode is an + /// arbitrary-event-injection endpoint feeding a script-executing rules engine, + /// and a warning in a log nobody reads is not a control. + pub fn validate_startup(&self) -> Result<(), String> { + if self.secret.is_none() && !self.allow_unauthenticated { + return Err("webhook: no `secret` configured. Set one, or set \ + `allow_unauthenticated: true` to accept unsigned requests \ + (every accepted request can trigger rulebook actions)." + .to_string()); + } + if let Some(s) = &self.secret { + if s.is_empty() { + return Err("webhook: `secret` is empty; omit it or set a real value".to_string()); + } } + let loopback = self.bind == "127.0.0.1" || self.bind == "::1" || self.bind == "localhost"; + if !loopback && !self.tls_terminated_upstream { + return Err(format!( + "webhook: refusing to bind {} without `tls_terminated_upstream: true`. \ + Signatures authenticate the sender but do not encrypt the payload.", + self.bind + )); + } + if self.allowed_paths.is_empty() { + return Err( + "webhook: `allowed_paths` is empty, which denies every request. \ + List the paths you intend to serve." + .to_string(), + ); + } + Ok(()) } } /// A parsed incoming webhook request. #[derive(Debug, Clone)] pub struct WebhookRequest { - /// HTTP method (POST, PUT, etc.). + /// HTTP method. pub method: String, /// Request path. pub path: String, - /// Request headers. + /// Lowercase-keyed headers. pub headers: HashMap, - /// Request body as UTF-8 string. - pub body: String, - /// Source IP address (if available). + /// EXACT body octets as received. + /// + /// `Vec`, not `String`. The server used to hand over + /// `String::from_utf8_lossy(..)`, so any non-UTF-8 byte became U+FFFD (3 + /// bytes) before the MAC was computed — a sender that correctly signed the + /// wire bytes could never match, and `body.len()` measured the inflated + /// replacement string rather than the octets received. + pub body: Vec, + /// Peer address, if known. pub source_ip: Option, } @@ -51,35 +151,99 @@ pub struct WebhookRequest { pub enum ValidationResult { /// Request is valid. Valid, - /// Request body exceeds max size. - BodyTooLarge { size: usize, max: usize }, - /// Path is not in the allowed list. - PathNotAllowed { path: String }, - /// HMAC signature is missing when secret is configured. + /// Body exceeds the configured maximum. + BodyTooLarge { + /// Observed size in bytes. + size: usize, + /// Configured maximum. + max: usize, + }, + /// Path is not in the allow-list (or the list is empty). + PathNotAllowed { + /// The rejected path. + path: String, + }, + /// A secret is configured but no signature header was sent. SignatureMissing, - /// HMAC signature does not match. + /// Signature present but does not verify. SignatureInvalid, - /// HTTP method not allowed (only POST accepted). - MethodNotAllowed { method: String }, + /// `t=` is outside the tolerance window (replay or badly-skewed clock). + SignatureStale { + /// Absolute skew in seconds. + skew_secs: u64, + }, + /// Only POST is accepted. + MethodNotAllowed { + /// The rejected method. + method: String, + }, } impl ValidationResult { /// Whether the request passed validation. + #[must_use] pub fn is_valid(&self) -> bool { matches!(self, Self::Valid) } + + /// HTTP status for this outcome. + /// + /// Every failure used to collapse to 403 while the 401/405/413 arms of the + /// status table sat unreachable. + #[must_use] + pub fn status(&self) -> u16 { + match self { + Self::Valid => 200, + Self::BodyTooLarge { .. } => 413, + Self::PathNotAllowed { .. } => 404, + Self::SignatureMissing | Self::SignatureInvalid | Self::SignatureStale { .. } => 401, + Self::MethodNotAllowed { .. } => 405, + } + } + + /// Stable reason code for the response body. + /// + /// Fixed strings: the old code put `format!("{validation:?}")` in the body, + /// which both echoed the attacker's path back and produced invalid JSON + /// (`{"status":"PathNotAllowed { path: "/evil" }"}`). The Debug form is still + /// logged server-side, where operators can see it and attackers cannot. + #[must_use] + pub fn code(&self) -> &'static str { + match self { + Self::Valid => "accepted", + Self::BodyTooLarge { .. } => "body_too_large", + Self::PathNotAllowed { .. } => "not_found", + Self::SignatureMissing => "signature_missing", + Self::SignatureInvalid => "signature_invalid", + Self::SignatureStale { .. } => "signature_stale", + Self::MethodNotAllowed { .. } => "method_not_allowed", + } + } } /// Validate an incoming webhook request against the configuration. +/// +/// Order matters: cheap structural checks first, then the MAC, so an unauthorized +/// sender cannot make the receiver do expensive work. +#[must_use] pub fn validate_request(config: &WebhookConfig, request: &WebhookRequest) -> ValidationResult { - // Only POST allowed - if request.method.to_uppercase() != "POST" { + validate_request_at(config, request, unix_now()) +} + +/// [`validate_request`] with an injected clock, so freshness is testable without +/// sleeping or mocking time. +#[must_use] +pub fn validate_request_at( + config: &WebhookConfig, + request: &WebhookRequest, + now: i64, +) -> ValidationResult { + if !request.method.eq_ignore_ascii_case("POST") { return ValidationResult::MethodNotAllowed { method: request.method.clone(), }; } - // Check body size if request.body.len() > config.max_body_bytes { return ValidationResult::BodyTooLarge { size: request.body.len(), @@ -87,34 +251,80 @@ pub fn validate_request(config: &WebhookConfig, request: &WebhookRequest) -> Val }; } - // Check allowed paths - if !config.allowed_paths.is_empty() && !config.allowed_paths.iter().any(|p| p == &request.path) - { + // Empty allow-list denies. Compare against the path WITHOUT its query string, + // so `/webhook?x=1` matches `/webhook` — exact matching on the raw target made + // any query string a rejection. + let path_only = request.path.split('?').next().unwrap_or(&request.path); + if !config.allowed_paths.iter().any(|p| p == path_only) { return ValidationResult::PathNotAllowed { - path: request.path.clone(), + path: path_only.to_string(), }; } - // Check HMAC signature if secret is configured - if let Some(ref secret) = config.secret { - match request.headers.get("x-forjar-signature") { - None => return ValidationResult::SignatureMissing, - Some(sig) => { - let expected = compute_hmac_hex(secret, &request.body); - if sig != &expected { - return ValidationResult::SignatureInvalid; - } - } + match &config.secret { + None => ValidationResult::Valid, + Some(secret) => verify_signature(config, request, secret, now), + } +} + +/// Check forjar's own signature, falling back to GitHub's header. +fn verify_signature( + config: &WebhookConfig, + request: &WebhookRequest, + secret: &str, + now: i64, +) -> ValidationResult { + let key = secret.as_bytes(); + + if let Some(raw) = request.headers.get(SIG_HEADER) { + let header = webhook_sig::parse_forjar_signature(raw); + // A header with a `t` but no `v1` is malformed, not unsigned — treating it + // as unsigned would let a sender opt out of authentication by sending + // junk. + if !header.has_v1() { + return ValidationResult::SignatureInvalid; + } + let Some(t) = header.timestamp else { + return ValidationResult::SignatureInvalid; + }; + if !timestamp_is_fresh(t, now, config.signature_tolerance_secs) { + return ValidationResult::SignatureStale { + skew_secs: now.saturating_sub(t).unsigned_abs(), + }; } + let signed = canonical_payload(t, &request.method, &request.path, &request.body); + // Accept if ANY v1 verifies, so a secret rotation can overlap. + if header + .v1 + .iter() + .any(|sig| webhook_sig::verify_hex(key, &signed, sig)) + { + return ValidationResult::Valid; + } + return ValidationResult::SignatureInvalid; } - ValidationResult::Valid + // GitHub cannot be told to sign a custom canonical form, so its header is + // verified over the bare body. That means no timestamp binding and no path + // binding for GitHub senders — acceptable because GitHub delivers to one + // configured URL, and the replay guard still makes a delivery single-use. + if let Some(raw) = request.headers.get(GITHUB_SIG_HEADER) { + return match webhook_sig::parse_github_signature(raw) { + Some(hex) if webhook_sig::verify_hex(key, &request.body, &hex) => { + ValidationResult::Valid + } + _ => ValidationResult::SignatureInvalid, + }; + } + + ValidationResult::SignatureMissing } -/// Parse a JSON webhook body into key-value payload for InfraEvent. -pub fn parse_json_payload(body: &str) -> Result, String> { +/// Parse a JSON webhook body into a flat key-value payload. +pub fn parse_json_payload(body: &[u8]) -> Result, String> { + let text = std::str::from_utf8(body).map_err(|e| format!("body is not valid UTF-8: {e}"))?; let value: serde_json::Value = - serde_json::from_str(body).map_err(|e| format!("invalid JSON: {e}"))?; + serde_json::from_str(text).map_err(|e| format!("invalid JSON: {e}"))?; let mut payload = HashMap::new(); match value { @@ -133,267 +343,31 @@ pub fn parse_json_payload(body: &str) -> Result, String> Ok(payload) } -/// Convert a validated webhook request into an InfraEvent. -pub fn request_to_event(request: &WebhookRequest) -> Result { +/// Convert a validated webhook request into an [`InfraEvent`]. +pub fn request_to_event( + request: &WebhookRequest, + machine: Option<&str>, + event_id: Option<&str>, +) -> Result { let mut payload = parse_json_payload(&request.body)?; - // Add metadata from the request + // Written AFTER parsing, so these always win over same-named body keys. payload.insert("_path".to_string(), request.path.clone()); if let Some(ref ip) = request.source_ip { payload.insert("_source_ip".to_string(), ip.clone()); } + if let Some(id) = event_id { + payload.insert("_event_id".to_string(), id.to_string()); + } Ok(InfraEvent { event_type: EventType::WebhookReceived, - timestamp: now_iso8601(), - machine: None, + // The one real clock. This module used to carry its own `now_iso8601` + // returning `format!("{}Z", secs)` — epoch seconds with a Z suffix, not + // ISO 8601 — so webhook events wrote `1785348550Z` into the same + // rulebook-log field where `cli::trigger` wrote `2026-07-29T…Z`. + timestamp: crate::tripwire::eventlog::now_iso8601(), + machine: machine.map(str::to_string), payload, }) } - -/// Compute HMAC-SHA256 of `data` using `key`, returned as hex string. -/// -/// Uses a simple HMAC construction: H((key XOR opad) || H((key XOR ipad) || data)) -/// For production, prefer ring or hmac crate. This is a minimal implementation -/// to avoid adding heavyweight crypto dependencies. -pub fn compute_hmac_hex(key: &str, data: &str) -> String { - // Use BLAKE3 keyed hash as HMAC substitute (faster, simpler, sovereign) - let key_bytes = blake3::hash(key.as_bytes()); - let mut hasher = blake3::Hasher::new_keyed(key_bytes.as_bytes()); - hasher.update(data.as_bytes()); - hasher.finalize().to_hex().to_string() -} - -/// Format an HTTP response for a webhook acknowledgment. -pub fn ack_response(status: u16, message: &str) -> String { - let body = format!(r#"{{"status":"{message}"}}"#); - let reason = status_reason(status); - let len = body.len(); - format!( - "HTTP/1.1 {status} {reason}\r\n\ - Content-Type: application/json\r\n\ - Content-Length: {len}\r\n\ - \r\n\ - {body}", - ) -} - -fn status_reason(code: u16) -> &'static str { - match code { - 200 => "OK", - 400 => "Bad Request", - 401 => "Unauthorized", - 403 => "Forbidden", - 405 => "Method Not Allowed", - 413 => "Payload Too Large", - 500 => "Internal Server Error", - _ => "Unknown", - } -} - -fn now_iso8601() -> String { - // Minimal ISO 8601 without external crate - let dur = std::time::SystemTime::now() - .duration_since(std::time::UNIX_EPOCH) - .unwrap_or_default(); - format!("{}Z", dur.as_secs()) -} - -#[cfg(test)] -mod tests { - use super::*; - - fn default_config() -> WebhookConfig { - WebhookConfig::default() - } - - fn post_request(path: &str, body: &str) -> WebhookRequest { - WebhookRequest { - method: "POST".into(), - path: path.into(), - headers: HashMap::new(), - body: body.into(), - source_ip: Some("127.0.0.1".into()), - } - } - - #[test] - fn validate_valid_post() { - let config = default_config(); - let req = post_request("/webhook", r#"{"action":"deploy"}"#); - assert!(validate_request(&config, &req).is_valid()); - } - - #[test] - fn validate_method_not_allowed() { - let config = default_config(); - let req = WebhookRequest { - method: "GET".into(), - path: "/webhook".into(), - headers: HashMap::new(), - body: String::new(), - source_ip: None, - }; - assert_eq!( - validate_request(&config, &req), - ValidationResult::MethodNotAllowed { - method: "GET".into() - } - ); - } - - #[test] - fn validate_body_too_large() { - let config = WebhookConfig { - max_body_bytes: 10, - ..default_config() - }; - let req = post_request("/webhook", "a]long body that exceeds limit"); - match validate_request(&config, &req) { - ValidationResult::BodyTooLarge { size, max } => { - assert!(size > max); - } - other => panic!("expected BodyTooLarge, got {other:?}"), - } - } - - #[test] - fn validate_path_not_allowed() { - let config = default_config(); - let req = post_request("/admin/hack", r#"{}"#); - assert_eq!( - validate_request(&config, &req), - ValidationResult::PathNotAllowed { - path: "/admin/hack".into() - } - ); - } - - #[test] - fn validate_signature_missing() { - let config = WebhookConfig { - secret: Some("mysecret".into()), - ..default_config() - }; - let req = post_request("/webhook", r#"{}"#); - assert_eq!( - validate_request(&config, &req), - ValidationResult::SignatureMissing - ); - } - - #[test] - fn validate_signature_valid() { - let secret = "test-secret"; - let body = r#"{"deploy":true}"#; - let sig = compute_hmac_hex(secret, body); - - let config = WebhookConfig { - secret: Some(secret.into()), - ..default_config() - }; - let mut req = post_request("/webhook", body); - req.headers.insert("x-forjar-signature".into(), sig); - assert!(validate_request(&config, &req).is_valid()); - } - - #[test] - fn validate_signature_invalid() { - let config = WebhookConfig { - secret: Some("real-secret".into()), - ..default_config() - }; - let mut req = post_request("/webhook", r#"{}"#); - req.headers - .insert("x-forjar-signature".into(), "bad-sig".into()); - assert_eq!( - validate_request(&config, &req), - ValidationResult::SignatureInvalid - ); - } - - #[test] - fn parse_json_object() { - let payload = parse_json_payload(r#"{"action":"deploy","env":"prod"}"#).unwrap(); - assert_eq!(payload.get("action").unwrap(), "deploy"); - assert_eq!(payload.get("env").unwrap(), "prod"); - } - - #[test] - fn parse_json_nested() { - let payload = parse_json_payload(r#"{"count":42,"nested":{"a":1}}"#).unwrap(); - assert_eq!(payload.get("count").unwrap(), "42"); - assert!(payload.get("nested").unwrap().contains("\"a\":1")); - } - - #[test] - fn parse_json_invalid() { - assert!(parse_json_payload("not json").is_err()); - } - - #[test] - fn parse_json_non_object() { - assert!(parse_json_payload("[1,2,3]").is_err()); - } - - #[test] - fn request_to_event_valid() { - let req = post_request("/webhook", r#"{"action":"restart"}"#); - let event = request_to_event(&req).unwrap(); - assert_eq!(event.event_type, EventType::WebhookReceived); - assert_eq!(event.payload.get("action").unwrap(), "restart"); - assert_eq!(event.payload.get("_path").unwrap(), "/webhook"); - assert_eq!(event.payload.get("_source_ip").unwrap(), "127.0.0.1"); - } - - #[test] - fn request_to_event_invalid_body() { - let req = post_request("/webhook", "not json"); - assert!(request_to_event(&req).is_err()); - } - - #[test] - fn hmac_deterministic() { - let h1 = compute_hmac_hex("key", "data"); - let h2 = compute_hmac_hex("key", "data"); - assert_eq!(h1, h2); - assert_eq!(h1.len(), 64); - } - - #[test] - fn hmac_different_keys() { - let h1 = compute_hmac_hex("key1", "data"); - let h2 = compute_hmac_hex("key2", "data"); - assert_ne!(h1, h2); - } - - #[test] - fn ack_response_format() { - let resp = ack_response(200, "accepted"); - assert!(resp.starts_with("HTTP/1.1 200 OK")); - assert!(resp.contains("application/json")); - assert!(resp.contains("accepted")); - } - - #[test] - fn ack_response_error() { - let resp = ack_response(400, "bad request"); - assert!(resp.contains("400 Bad Request")); - } - - #[test] - fn validation_result_is_valid() { - assert!(ValidationResult::Valid.is_valid()); - assert!(!ValidationResult::SignatureMissing.is_valid()); - assert!(!ValidationResult::SignatureInvalid.is_valid()); - } - - #[test] - fn default_webhook_config() { - let config = WebhookConfig::default(); - assert_eq!(config.port, 8484); - assert!(config.secret.is_none()); - assert_eq!(config.max_body_bytes, 64 * 1024); - assert_eq!(config.allowed_paths, vec!["/webhook"]); - } -} diff --git a/tests/falsification_event_sources_b.rs b/tests/falsification_event_sources_b.rs index bf985ec1..db069ced 100644 --- a/tests/falsification_event_sources_b.rs +++ b/tests/falsification_event_sources_b.rs @@ -9,9 +9,10 @@ #![allow(dead_code)] use forjar::core::types::EventType; +use forjar::core::webhook_sig::{compute_hmac_hex, sign_request, unix_now}; use forjar::core::webhook_source::{ - ack_response, compute_hmac_hex, parse_json_payload, request_to_event, validate_request, - ValidationResult, WebhookConfig, WebhookRequest, + parse_json_payload, request_to_event, validate_request, ValidationResult, WebhookConfig, + WebhookRequest, }; use std::collections::HashMap; @@ -28,7 +29,7 @@ fn post_request(path: &str, body: &str) -> WebhookRequest { method: "POST".into(), path: path.into(), headers: HashMap::new(), - body: body.into(), + body: body.as_bytes().to_vec(), source_ip: Some("127.0.0.1".into()), } } @@ -51,7 +52,7 @@ fn webhook_method_not_allowed() { method: "GET".into(), path: "/webhook".into(), headers: HashMap::new(), - body: String::new(), + body: Vec::new(), source_ip: None, }; assert_eq!( @@ -106,7 +107,13 @@ fn webhook_signature_missing() { fn webhook_signature_valid() { let secret = "test-secret"; let body = r#"{"deploy":true}"#; - let sig = compute_hmac_hex(secret, body); + let sig = sign_request( + secret.as_bytes(), + "POST", + "/webhook", + body.as_bytes(), + unix_now(), + ); let config = WebhookConfig { secret: Some(secret.into()), @@ -138,23 +145,23 @@ fn webhook_signature_invalid() { #[test] fn hmac_deterministic() { - let h1 = compute_hmac_hex("key", "data"); - let h2 = compute_hmac_hex("key", "data"); + let h1 = compute_hmac_hex(b"key", b"data"); + let h2 = compute_hmac_hex(b"key", b"data"); assert_eq!(h1, h2); assert_eq!(h1.len(), 64); } #[test] fn hmac_different_keys() { - let h1 = compute_hmac_hex("key1", "data"); - let h2 = compute_hmac_hex("key2", "data"); + let h1 = compute_hmac_hex(b"key1", b"data"); + let h2 = compute_hmac_hex(b"key2", b"data"); assert_ne!(h1, h2); } #[test] fn hmac_different_data() { - let h1 = compute_hmac_hex("key", "data1"); - let h2 = compute_hmac_hex("key", "data2"); + let h1 = compute_hmac_hex(b"key", b"data1"); + let h2 = compute_hmac_hex(b"key", b"data2"); assert_ne!(h1, h2); } @@ -164,31 +171,31 @@ fn hmac_different_data() { #[test] fn parse_json_object() { - let payload = parse_json_payload(r#"{"action":"deploy","env":"prod"}"#).unwrap(); + let payload = parse_json_payload(r#"{"action":"deploy","env":"prod"}"#.as_bytes()).unwrap(); assert_eq!(payload.get("action").unwrap(), "deploy"); assert_eq!(payload.get("env").unwrap(), "prod"); } #[test] fn parse_json_nested_stringified() { - let payload = parse_json_payload(r#"{"count":42,"nested":{"a":1}}"#).unwrap(); + let payload = parse_json_payload(r#"{"count":42,"nested":{"a":1}}"#.as_bytes()).unwrap(); assert_eq!(payload.get("count").unwrap(), "42"); assert!(payload.get("nested").unwrap().contains("\"a\":1")); } #[test] fn parse_json_invalid() { - assert!(parse_json_payload("not json").is_err()); + assert!(parse_json_payload("not json".as_bytes()).is_err()); } #[test] fn parse_json_non_object() { - assert!(parse_json_payload("[1,2,3]").is_err()); + assert!(parse_json_payload("[1,2,3]".as_bytes()).is_err()); } #[test] fn parse_json_empty_object() { - let payload = parse_json_payload("{}").unwrap(); + let payload = parse_json_payload("{}".as_bytes()).unwrap(); assert!(payload.is_empty()); } @@ -199,7 +206,7 @@ fn parse_json_empty_object() { #[test] fn request_to_event_valid() { let req = post_request("/webhook", r#"{"action":"restart"}"#); - let event = request_to_event(&req).unwrap(); + let event = request_to_event(&req, None, None).unwrap(); assert_eq!(event.event_type, EventType::WebhookReceived); assert_eq!(event.payload.get("action").unwrap(), "restart"); assert_eq!(event.payload.get("_path").unwrap(), "/webhook"); @@ -212,17 +219,17 @@ fn request_to_event_no_source_ip() { method: "POST".into(), path: "/webhook".into(), headers: HashMap::new(), - body: r#"{"action":"deploy"}"#.into(), + body: r#"{"action":"deploy"}"#.as_bytes().to_vec(), source_ip: None, }; - let event = request_to_event(&req).unwrap(); + let event = request_to_event(&req, None, None).unwrap(); assert!(!event.payload.contains_key("_source_ip")); } #[test] fn request_to_event_invalid_body() { let req = post_request("/webhook", "not json"); - assert!(request_to_event(&req).is_err()); + assert!(request_to_event(&req, None, None).is_err()); } // ============================================================================ @@ -231,7 +238,7 @@ fn request_to_event_invalid_body() { #[test] fn ack_response_200() { - let resp = ack_response(200, "accepted"); + let resp = String::from_utf8(forjar::core::webhook_http::response(200, "accepted")).unwrap(); assert!(resp.starts_with("HTTP/1.1 200 OK")); assert!(resp.contains("application/json")); assert!(resp.contains("accepted")); @@ -239,13 +246,14 @@ fn ack_response_200() { #[test] fn ack_response_400() { - let resp = ack_response(400, "bad request"); + let resp = String::from_utf8(forjar::core::webhook_http::response(400, "bad request")).unwrap(); assert!(resp.contains("400 Bad Request")); } #[test] fn ack_response_401() { - let resp = ack_response(401, "unauthorized"); + let resp = + String::from_utf8(forjar::core::webhook_http::response(401, "unauthorized")).unwrap(); assert!(resp.contains("401 Unauthorized")); } diff --git a/tests/falsification_migrate_webhook.rs b/tests/falsification_migrate_webhook.rs index 903c0b6e..cbe8ac9f 100644 --- a/tests/falsification_migrate_webhook.rs +++ b/tests/falsification_migrate_webhook.rs @@ -12,9 +12,10 @@ use forjar::core::migrate::{docker_to_pepita, migrate_config}; use forjar::core::parser::parse_config; use forjar::core::types::*; +use forjar::core::webhook_sig::{compute_hmac_hex, sign_request, unix_now}; use forjar::core::webhook_source::{ - ack_response, compute_hmac_hex, parse_json_payload, request_to_event, validate_request, - ValidationResult, WebhookConfig, WebhookRequest, + parse_json_payload, request_to_event, validate_request, ValidationResult, WebhookConfig, + WebhookRequest, }; use std::collections::HashMap; @@ -250,7 +251,7 @@ fn webhook_request(method: &str, path: &str, body: &str) -> WebhookRequest { method: method.into(), path: path.into(), headers: HashMap::new(), - body: body.into(), + body: body.as_bytes().to_vec(), source_ip: None, } } @@ -339,7 +340,7 @@ fn webhook_hmac_invalid() { method: "POST".into(), path: "/webhook".into(), headers, - body: "{}".into(), + body: "{}".as_bytes().to_vec(), source_ip: None, }; assert!(matches!( @@ -352,7 +353,13 @@ fn webhook_hmac_invalid() { fn webhook_hmac_valid() { let secret = "mysecret"; let body = r#"{"action":"deploy"}"#; - let sig = compute_hmac_hex(secret, body); + let sig = sign_request( + secret.as_bytes(), + "POST", + "/webhook", + body.as_bytes(), + unix_now(), + ); let config = WebhookConfig { secret: Some(secret.into()), @@ -364,7 +371,7 @@ fn webhook_hmac_valid() { method: "POST".into(), path: "/webhook".into(), headers, - body: body.into(), + body: body.as_bytes().to_vec(), source_ip: None, }; assert!(validate_request(&config, &req).is_valid()); @@ -376,15 +383,15 @@ fn webhook_hmac_valid() { #[test] fn hmac_deterministic() { - let h1 = compute_hmac_hex("key", "data"); - let h2 = compute_hmac_hex("key", "data"); + let h1 = compute_hmac_hex(b"key", b"data"); + let h2 = compute_hmac_hex(b"key", b"data"); assert_eq!(h1, h2); } #[test] fn hmac_different_keys_different_hashes() { - let h1 = compute_hmac_hex("key1", "data"); - let h2 = compute_hmac_hex("key2", "data"); + let h1 = compute_hmac_hex(b"key1", b"data"); + let h2 = compute_hmac_hex(b"key2", b"data"); assert_ne!(h1, h2); } @@ -394,28 +401,28 @@ fn hmac_different_keys_different_hashes() { #[test] fn payload_parse_object() { - let payload = parse_json_payload(r#"{"action":"deploy","env":"prod"}"#).unwrap(); + let payload = parse_json_payload(r#"{"action":"deploy","env":"prod"}"#.as_bytes()).unwrap(); assert_eq!(payload["action"], "deploy"); assert_eq!(payload["env"], "prod"); } #[test] fn payload_parse_non_string_values() { - let payload = parse_json_payload(r#"{"count":42,"active":true}"#).unwrap(); + let payload = parse_json_payload(r#"{"count":42,"active":true}"#.as_bytes()).unwrap(); assert_eq!(payload["count"], "42"); assert_eq!(payload["active"], "true"); } #[test] fn payload_reject_non_object() { - let result = parse_json_payload("[1,2,3]"); + let result = parse_json_payload("[1,2,3]".as_bytes()); assert!(result.is_err()); assert!(result.unwrap_err().contains("JSON object")); } #[test] fn payload_reject_invalid_json() { - let result = parse_json_payload("not json"); + let result = parse_json_payload("not json".as_bytes()); assert!(result.is_err()); } @@ -429,10 +436,10 @@ fn request_to_event_adds_metadata() { method: "POST".into(), path: "/hooks/deploy".into(), headers: HashMap::new(), - body: r#"{"action":"deploy"}"#.into(), + body: r#"{"action":"deploy"}"#.as_bytes().to_vec(), source_ip: Some("10.0.0.1".into()), }; - let event = request_to_event(&req).unwrap(); + let event = request_to_event(&req, None, None).unwrap(); assert_eq!(event.event_type, EventType::WebhookReceived); assert_eq!(event.payload["action"], "deploy"); assert_eq!(event.payload["_path"], "/hooks/deploy"); @@ -445,7 +452,7 @@ fn request_to_event_adds_metadata() { #[test] fn ack_response_format() { - let resp = ack_response(200, "accepted"); + let resp = String::from_utf8(forjar::core::webhook_http::response(200, "accepted")).unwrap(); assert!(resp.contains("HTTP/1.1 200 OK")); assert!(resp.contains("application/json")); assert!(resp.contains("accepted")); @@ -453,6 +460,7 @@ fn ack_response_format() { #[test] fn ack_response_error() { - let resp = ack_response(401, "unauthorized"); + let resp = + String::from_utf8(forjar::core::webhook_http::response(401, "unauthorized")).unwrap(); assert!(resp.contains("401 Unauthorized")); } diff --git a/tests/falsification_undo_webhook.rs b/tests/falsification_undo_webhook.rs index e1a89c29..7f6b7c0f 100644 --- a/tests/falsification_undo_webhook.rs +++ b/tests/falsification_undo_webhook.rs @@ -2,6 +2,7 @@ //! Usage: cargo test --test falsification_undo_webhook use forjar::core::types::*; +use forjar::core::webhook_sig::{compute_hmac_hex, sign_request, unix_now}; use forjar::core::webhook_source::*; use std::collections::HashMap; @@ -215,7 +216,7 @@ fn post_request(path: &str, body: &str) -> WebhookRequest { method: "POST".into(), path: path.into(), headers: HashMap::new(), - body: body.into(), + body: body.as_bytes().to_vec(), source_ip: Some("127.0.0.1".into()), } } @@ -234,7 +235,7 @@ fn webhook_method_not_allowed() { method: "GET".into(), path: "/webhook".into(), headers: HashMap::new(), - body: String::new(), + body: Vec::new(), source_ip: None, }; assert_eq!( @@ -287,7 +288,13 @@ fn webhook_signature_missing() { fn webhook_signature_valid() { let secret = "test-secret"; let body = r#"{"deploy":true}"#; - let sig = compute_hmac_hex(secret, body); + let sig = sign_request( + secret.as_bytes(), + "POST", + "/webhook", + body.as_bytes(), + unix_now(), + ); let config = WebhookConfig { secret: Some(secret.into()), ..Default::default() @@ -318,25 +325,25 @@ fn webhook_signature_invalid() { #[test] fn parse_json_object() { - let payload = parse_json_payload(r#"{"action":"deploy","env":"prod"}"#).unwrap(); + let payload = parse_json_payload(r#"{"action":"deploy","env":"prod"}"#.as_bytes()).unwrap(); assert_eq!(payload["action"], "deploy"); assert_eq!(payload["env"], "prod"); } #[test] fn parse_json_nested_stringified() { - let payload = parse_json_payload(r#"{"count":42}"#).unwrap(); + let payload = parse_json_payload(r#"{"count":42}"#.as_bytes()).unwrap(); assert_eq!(payload["count"], "42"); } #[test] fn parse_json_invalid() { - assert!(parse_json_payload("not json").is_err()); + assert!(parse_json_payload("not json".as_bytes()).is_err()); } #[test] fn parse_json_array_rejected() { - assert!(parse_json_payload("[1,2,3]").is_err()); + assert!(parse_json_payload("[1,2,3]".as_bytes()).is_err()); } // ============================================================================ @@ -346,7 +353,7 @@ fn parse_json_array_rejected() { #[test] fn request_to_event_valid() { let req = post_request("/webhook", r#"{"action":"restart"}"#); - let event = request_to_event(&req).unwrap(); + let event = request_to_event(&req, None, None).unwrap(); assert_eq!(event.event_type, EventType::WebhookReceived); assert_eq!(event.payload["action"], "restart"); assert_eq!(event.payload["_path"], "/webhook"); @@ -356,7 +363,7 @@ fn request_to_event_valid() { #[test] fn request_to_event_invalid() { let req = post_request("/webhook", "not json"); - assert!(request_to_event(&req).is_err()); + assert!(request_to_event(&req, None, None).is_err()); } #[test] @@ -365,10 +372,10 @@ fn request_to_event_no_source_ip() { method: "POST".into(), path: "/webhook".into(), headers: HashMap::new(), - body: r#"{"key":"val"}"#.into(), + body: r#"{"key":"val"}"#.as_bytes().to_vec(), source_ip: None, }; - let event = request_to_event(&req).unwrap(); + let event = request_to_event(&req, None, None).unwrap(); assert!(!event.payload.contains_key("_source_ip")); } @@ -378,8 +385,8 @@ fn request_to_event_no_source_ip() { #[test] fn hmac_deterministic() { - let h1 = compute_hmac_hex("key", "data"); - let h2 = compute_hmac_hex("key", "data"); + let h1 = compute_hmac_hex(b"key", b"data"); + let h2 = compute_hmac_hex(b"key", b"data"); assert_eq!(h1, h2); assert_eq!(h1.len(), 64); } @@ -387,14 +394,17 @@ fn hmac_deterministic() { #[test] fn hmac_different_keys() { assert_ne!( - compute_hmac_hex("k1", "data"), - compute_hmac_hex("k2", "data") + compute_hmac_hex(b"k1", b"data"), + compute_hmac_hex(b"k2", b"data") ); } #[test] fn hmac_different_data() { - assert_ne!(compute_hmac_hex("key", "d1"), compute_hmac_hex("key", "d2")); + assert_ne!( + compute_hmac_hex(b"key", b"d1"), + compute_hmac_hex(b"key", b"d2") + ); } // ============================================================================ @@ -403,7 +413,7 @@ fn hmac_different_data() { #[test] fn ack_response_200() { - let resp = ack_response(200, "accepted"); + let resp = String::from_utf8(forjar::core::webhook_http::response(200, "accepted")).unwrap(); assert!(resp.starts_with("HTTP/1.1 200 OK")); assert!(resp.contains("application/json")); assert!(resp.contains("accepted")); @@ -411,13 +421,14 @@ fn ack_response_200() { #[test] fn ack_response_400() { - let resp = ack_response(400, "bad"); + let resp = String::from_utf8(forjar::core::webhook_http::response(400, "bad")).unwrap(); assert!(resp.contains("400 Bad Request")); } #[test] fn ack_response_401() { - let resp = ack_response(401, "unauthorized"); + let resp = + String::from_utf8(forjar::core::webhook_http::response(401, "unauthorized")).unwrap(); assert!(resp.contains("401 Unauthorized")); }