Skip to content

Commit db67790

Browse files
ndrenondreno
committed
feat: add request-size-limit and ip-restriction middleware plugins
- request-size-limit: Rejects requests exceeding configurable body size with 413 Payload Too Large - ip-restriction: Allow/deny requests by IP address or CIDR ranges - Deterministic artifact builds: Sort warnings for reproducible compiler output - Move E1015 unknown extension check to compile phase - Update middleware documentation with new plugins
1 parent 7e7b743 commit db67790

15 files changed

Lines changed: 1073 additions & 59 deletions

File tree

‎BACKLOG.md‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ Items are tagged with their source ADR or SPEC for traceability.
1414
|--------|------|-------------|--------|
1515
| `request-transformer` | Middleware | Modify headers, query params, body before upstream | Competitive analysis |
1616
| `response-transformer` | Middleware | Modify response headers/body before client | Competitive analysis |
17-
| `ip-restriction` | Middleware | Allow/deny by IP or CIDR range | Competitive analysis |
17+
| ~~`ip-restriction`~~ | ~~Middleware~~ | ~~Allow/deny by IP or CIDR range~~ | **DONE** |
1818
| `basic-auth` | Middleware | Username/password authentication | Competitive analysis |
1919
| `http-log` | Middleware | Send request/response logs to HTTP endpoint | Competitive analysis |
2020
| ~~`correlation-id`~~ | ~~Middleware~~ | ~~Propagate/generate X-Correlation-ID header~~ | **DONE** |
@@ -25,10 +25,9 @@ Items are tagged with their source ADR or SPEC for traceability.
2525
|--------|------|-------------|--------|
2626
| `observability` | Middleware | Trace sampling, detailed validation logs, latency SLO monitoring | ADR-0010 |
2727
| `acl` | Middleware | Access control by consumer/group after auth | Competitive analysis |
28-
| `request-size-limit` | Middleware | Reject requests exceeding size (per-route) | Competitive analysis |
28+
| ~~`request-size-limit`~~ | ~~Middleware~~ | ~~Reject requests exceeding size (per-route)~~ | **DONE** |
2929
| `bot-detection` | Middleware | Block known bots by User-Agent patterns | Competitive analysis |
3030
| `redirect` | Middleware | URL redirections (301/302) | Competitive analysis |
31-
| `request-termination` | Middleware | Return static error without calling upstream | Competitive analysis |
3231

3332
### P2 — Nice to Have
3433

@@ -122,11 +121,11 @@ Items are tagged with their source ADR or SPEC for traceability.
122121
| ~~Schema complexity limits~~ | E1051/E1052: Depth (32) and property (256) limits | P0 | **DONE** |
123122
| ~~Circular `$ref` detection~~ | E1053: Detect circular JSON Schema references | P0 | **DONE** |
124123
| ~~Move E1011 to compile~~ | E1011: Missing middleware name validation | P1 | **DONE** |
125-
| Move E1015 to compile | Move unknown extension warning from `validate` to `compile` | P1 | Tech review |
124+
| ~~Move E1015 to compile~~ | ~~Move unknown extension warning from `validate` to `compile`~~ | ~~P1~~ | **DONE** |
126125
| ~~Path template syntax validation~~ | E1054: Validate braces, param names, duplicates | P2 | **DONE** |
127126
| ~~Duplicate operationId detection~~ | E1055: Detect non-unique operationId | P2 | **DONE** |
128127
| Spec pointers in errors | Add JSON Pointer (e.g., `#/paths/~1users/get`) to all compile errors | P2 | Tech review |
129-
| Deterministic artifact builds | Sort plugin/spec/route collections before serialization | P2 | Tech review |
128+
| ~~Deterministic artifact builds~~ | ~~Sort plugin/spec/route collections before serialization~~ | ~~P2~~ | **DONE** |
130129

131130
**Test cases added:**
132131
- `compile_detects_ambiguous_routes` ✓

‎crates/barbacane-compiler/src/artifact.rs‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -422,6 +422,11 @@ pub fn compile_with_options(
422422
let encoder = archive.into_inner()?;
423423
encoder.finish()?;
424424

425+
// Sort warnings for deterministic output
426+
warnings.sort_by(|a, b| {
427+
(&a.location, &a.code, &a.message).cmp(&(&b.location, &b.code, &b.message))
428+
});
429+
425430
Ok(CompileResult { manifest, warnings })
426431
}
427432

@@ -744,6 +749,11 @@ pub fn compile_with_manifest(
744749
let encoder = archive.into_inner()?;
745750
encoder.finish()?;
746751

752+
// Sort warnings for deterministic output
753+
warnings.sort_by(|a, b| {
754+
(&a.location, &a.code, &a.message).cmp(&(&b.location, &b.code, &b.message))
755+
});
756+
747757
Ok(CompileResult { manifest, warnings })
748758
}
749759

@@ -1144,6 +1154,11 @@ pub fn compile_with_plugins(
11441154
let encoder = archive.into_inner()?;
11451155
encoder.finish()?;
11461156

1157+
// Sort warnings for deterministic output
1158+
warnings.sort_by(|a, b| {
1159+
(&a.location, &a.code, &a.message).cmp(&(&b.location, &b.code, &b.message))
1160+
});
1161+
11471162
Ok(CompileResult { manifest, warnings })
11481163
}
11491164

‎crates/barbacane-test/src/gateway.rs‎

Lines changed: 189 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3302,4 +3302,193 @@ paths:
33023302
"Response should include CORS header from global middleware"
33033303
);
33043304
}
3305+
3306+
// ==================== Request Size Limit Tests ====================
3307+
3308+
#[tokio::test]
3309+
async fn test_request_size_limit_allows_small_body() {
3310+
let gateway = TestGateway::from_spec("../../tests/fixtures/request-size-limit.yaml")
3311+
.await
3312+
.expect("failed to start gateway");
3313+
3314+
// Small body should be allowed (under 100 byte limit)
3315+
let resp = gateway
3316+
.request_builder(reqwest::Method::POST, "/limited")
3317+
.header("Content-Type", "application/json")
3318+
.body(r#"{"msg":"hi"}"#)
3319+
.send()
3320+
.await
3321+
.unwrap();
3322+
3323+
assert_eq!(resp.status(), 200);
3324+
let body: serde_json::Value = resp.json().await.unwrap();
3325+
assert_eq!(body["message"], "ok");
3326+
}
3327+
3328+
#[tokio::test]
3329+
async fn test_request_size_limit_blocks_large_body() {
3330+
let gateway = TestGateway::from_spec("../../tests/fixtures/request-size-limit.yaml")
3331+
.await
3332+
.expect("failed to start gateway");
3333+
3334+
// Large body should be rejected (over 100 byte limit)
3335+
let large_body = r#"{"data":"this is a very long message that exceeds the configured limit of 100 bytes for this endpoint"}"#;
3336+
let resp = gateway
3337+
.request_builder(reqwest::Method::POST, "/limited")
3338+
.header("Content-Type", "application/json")
3339+
.body(large_body)
3340+
.send()
3341+
.await
3342+
.unwrap();
3343+
3344+
assert_eq!(resp.status(), 413);
3345+
let body: serde_json::Value = resp.json().await.unwrap();
3346+
assert_eq!(body["type"], "urn:barbacane:error:payload-too-large");
3347+
assert_eq!(body["status"], 413);
3348+
}
3349+
3350+
#[tokio::test]
3351+
async fn test_request_size_limit_unlimited_endpoint() {
3352+
let gateway = TestGateway::from_spec("../../tests/fixtures/request-size-limit.yaml")
3353+
.await
3354+
.expect("failed to start gateway");
3355+
3356+
// Unlimited endpoint should accept large bodies
3357+
let large_body = r#"{"data":"this is a very long message that would be rejected on the limited endpoint but should pass here"}"#;
3358+
let resp = gateway
3359+
.request_builder(reqwest::Method::POST, "/unlimited")
3360+
.header("Content-Type", "application/json")
3361+
.body(large_body)
3362+
.send()
3363+
.await
3364+
.unwrap();
3365+
3366+
assert_eq!(resp.status(), 200);
3367+
let body: serde_json::Value = resp.json().await.unwrap();
3368+
assert_eq!(body["message"], "unlimited");
3369+
}
3370+
3371+
// ==================== IP Restriction Tests ====================
3372+
3373+
#[tokio::test]
3374+
async fn test_ip_restriction_allowlist_localhost() {
3375+
let gateway = TestGateway::from_spec("../../tests/fixtures/ip-restriction.yaml")
3376+
.await
3377+
.expect("failed to start gateway");
3378+
3379+
// Localhost should be allowed
3380+
let resp = gateway
3381+
.request_builder(reqwest::Method::GET, "/allowlist")
3382+
.send()
3383+
.await
3384+
.unwrap();
3385+
3386+
assert_eq!(resp.status(), 200);
3387+
let body: serde_json::Value = resp.json().await.unwrap();
3388+
assert_eq!(body["message"], "allowed");
3389+
}
3390+
3391+
#[tokio::test]
3392+
async fn test_ip_restriction_allowlist_denied_via_xff() {
3393+
let gateway = TestGateway::from_spec("../../tests/fixtures/ip-restriction.yaml")
3394+
.await
3395+
.expect("failed to start gateway");
3396+
3397+
// Request with X-Forwarded-For from non-allowed IP should be denied
3398+
let resp = gateway
3399+
.request_builder(reqwest::Method::GET, "/allowlist")
3400+
.header("X-Forwarded-For", "203.0.113.50")
3401+
.send()
3402+
.await
3403+
.unwrap();
3404+
3405+
assert_eq!(resp.status(), 403);
3406+
let body: serde_json::Value = resp.json().await.unwrap();
3407+
assert_eq!(body["type"], "urn:barbacane:error:ip-restricted");
3408+
}
3409+
3410+
#[tokio::test]
3411+
async fn test_ip_restriction_denylist_allowed() {
3412+
let gateway = TestGateway::from_spec("../../tests/fixtures/ip-restriction.yaml")
3413+
.await
3414+
.expect("failed to start gateway");
3415+
3416+
// Request from localhost (not in denylist) should be allowed
3417+
let resp = gateway
3418+
.request_builder(reqwest::Method::GET, "/denylist")
3419+
.send()
3420+
.await
3421+
.unwrap();
3422+
3423+
assert_eq!(resp.status(), 200);
3424+
}
3425+
3426+
#[tokio::test]
3427+
async fn test_ip_restriction_denylist_blocked() {
3428+
let gateway = TestGateway::from_spec("../../tests/fixtures/ip-restriction.yaml")
3429+
.await
3430+
.expect("failed to start gateway");
3431+
3432+
// Request from denied CIDR range should be blocked
3433+
let resp = gateway
3434+
.request_builder(reqwest::Method::GET, "/denylist")
3435+
.header("X-Forwarded-For", "10.1.2.3")
3436+
.send()
3437+
.await
3438+
.unwrap();
3439+
3440+
assert_eq!(resp.status(), 403);
3441+
}
3442+
3443+
#[tokio::test]
3444+
async fn test_ip_restriction_cidr_allowlist() {
3445+
let gateway = TestGateway::from_spec("../../tests/fixtures/ip-restriction.yaml")
3446+
.await
3447+
.expect("failed to start gateway");
3448+
3449+
// 127.0.0.1 is in 127.0.0.0/8 CIDR range
3450+
let resp = gateway
3451+
.request_builder(reqwest::Method::GET, "/cidr-allowlist")
3452+
.send()
3453+
.await
3454+
.unwrap();
3455+
3456+
assert_eq!(resp.status(), 200);
3457+
}
3458+
3459+
#[tokio::test]
3460+
async fn test_ip_restriction_custom_message() {
3461+
let gateway = TestGateway::from_spec("../../tests/fixtures/ip-restriction.yaml")
3462+
.await
3463+
.expect("failed to start gateway");
3464+
3465+
// Request from non-allowed IP should get custom status and message
3466+
let resp = gateway
3467+
.request_builder(reqwest::Method::GET, "/custom-message")
3468+
.send()
3469+
.await
3470+
.unwrap();
3471+
3472+
assert_eq!(resp.status(), 401);
3473+
let body: serde_json::Value = resp.json().await.unwrap();
3474+
assert!(body["detail"].as_str().unwrap().contains("not authorized"));
3475+
}
3476+
3477+
#[tokio::test]
3478+
async fn test_ip_restriction_public_endpoint() {
3479+
let gateway = TestGateway::from_spec("../../tests/fixtures/ip-restriction.yaml")
3480+
.await
3481+
.expect("failed to start gateway");
3482+
3483+
// Public endpoint without IP restriction
3484+
let resp = gateway
3485+
.request_builder(reqwest::Method::GET, "/public")
3486+
.send()
3487+
.await
3488+
.unwrap();
3489+
3490+
assert_eq!(resp.status(), 200);
3491+
let body: serde_json::Value = resp.json().await.unwrap();
3492+
assert_eq!(body["message"], "public");
3493+
}
33053494
}

‎crates/barbacane/src/main.rs‎

Lines changed: 4 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -420,7 +420,8 @@ enum Commands {
420420

421421
/// Validate OpenAPI spec(s) without compiling.
422422
///
423-
/// Checks for spec validity (E1001-E1004) and extension validity (E1010-E1015).
423+
/// Checks for spec validity (E1001-E1004) and extension validity (E1010-E1014).
424+
/// E1015 (unknown x-barbacane-* extension) is checked during compile.
424425
/// Does not resolve plugins or produce an artifact.
425426
Validate {
426427
/// Input spec file(s) (YAML or JSON).
@@ -1831,38 +1832,8 @@ fn run_validate(specs: &[String], output_format: &str) -> ExitCode {
18311832
}
18321833
}
18331834

1834-
// Check for unknown x-barbacane-* extensions (E1015 - warning)
1835-
// Note: x-sunset is not a barbacane extension (RFC 8594), so not in this list
1836-
// Note: Middleware functionality (rate-limit, cache, etc.) is configured via
1837-
// x-barbacane-middlewares with the plugin name, not as separate extensions.
1838-
// Backend connections are configured in the http-upstream dispatcher config.
1839-
// Keep in sync with KNOWN_EXTENSIONS in barbacane-compiler/src/artifact.rs
1840-
let known_extensions = [
1841-
"x-barbacane-dispatch", // Operation level
1842-
"x-barbacane-middlewares", // Root or operation level
1843-
];
1844-
1845-
for key in spec.extensions.keys() {
1846-
if !known_extensions.contains(&key.as_str()) {
1847-
warnings.push(ValidationIssue {
1848-
code: "E1015".to_string(),
1849-
message: format!("unknown extension: {}", key),
1850-
location: Some(spec_path.clone()),
1851-
});
1852-
}
1853-
}
1854-
1855-
for op in &spec.operations {
1856-
for key in op.extensions.keys() {
1857-
if !known_extensions.contains(&key.as_str()) {
1858-
warnings.push(ValidationIssue {
1859-
code: "E1015".to_string(),
1860-
message: format!("unknown extension: {}", key),
1861-
location: Some(format!("{}:{} {}", spec_path, op.path, op.method)),
1862-
});
1863-
}
1864-
}
1865-
}
1835+
// Note: E1015 (unknown x-barbacane-* extension) checking is done during compile,
1836+
// not validate, to avoid false positives on non-barbacane extensions.
18661837

18671838
// Store for cross-spec validation
18681839
parsed_specs.push((spec_path.clone(), spec));

0 commit comments

Comments
 (0)