Repository navigation
Validate requests by path info and keep runtime env out of the cache - #57
Conversation
0.14.1 cached each area's OpenAPI document at warm-up, freezing %env()% values from Nelmio's documentation: a server URL with a path set at runtime made valid requests fail, because league matches the server path plus the request path. Validate the request's Symfony path info against the document's paths and leave servers out of the cached document at every level; that also fixes a server path that differs from the base URL. Key the cached document on the runtime values of the other env vars in the area's documentation and security config, collected at compile time, so a different value generates its own document once. Log an area the warmer could not cache, and version the file name so 0.14.1 caches are not reused.
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
⛔ Files ignored due to path filters (1)
📒 Files selected for processing (12)
Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review. WalkthroughThe change collects Nelmio environment values for each area and uses them to identify cached OpenAPI documents. It adds warm-up logging and runtime fallback behavior, removes server declarations from generated documents, and validates requests using Symfony path info. ChangesOpenAPI validation and caching
Priority: ⬇️ Low Estimated code review effort: 3 (Moderate) | ~25 minutes Change: Bug fix Sequence Diagram(s)sequenceDiagram
participant OpenApiSpecCache
participant CacheDirectory
participant NelmioGenerator
participant LoggerInterface
OpenApiSpecCache->>CacheDirectory: Look up environment-specific document
OpenApiSpecCache->>NelmioGenerator: Generate document on cache miss
NelmioGenerator-->>OpenApiSpecCache: Return generated JSON
OpenApiSpecCache->>CacheDirectory: Write generated document
NelmioGenerator-->>OpenApiSpecCache: Return generation error
OpenApiSpecCache->>LoggerInterface: Log warning
OpenApiSpecCache->>CacheDirectory: Read warmed fallback outside debug mode
Merge Risk: ⚪ Minimal · up to No actionable issue has been established that would prevent merging after normal checks. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to Normal operation separates cached validation documents by runtime configuration, while application routing remains in control of endpoint reachability. Generation failures deliberately reuse and persist the warm-up document, so runtime configuration may not take effect in that degraded state. No new authorization bypass was established, but recovery and concurrent-write guarantees remain partly unverified. Retained concerns Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
Resilience and Maintainability Implications
Hardening Proposals
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 22.64% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 53 functions across 10 files. (2 skipped: 2 unsupported.)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…only, and fall back to the warmed one Also leave env vars of nested servers out of the cache key.
Strip servers only where the cached document does.
Summary
0.14.1 caches each Nelmio area's OpenAPI document (at
cache:warmupor on first use) and validates requests against it. Values Nelmio resolves from%env()%were frozen into that cache at warm-up time. With the common container pattern (build one image, inject the real configuration at runtime) the document kept build-time values forever in production.That matters for
servers: league's validator matches server path + request path. A server URL set at runtime with a path, such ashttps://api.example.com/v1, therefore made valid requests fail with "no such operation".Validate by path info, ignore
servers:RequestValidatorvalidates the request's Symfony path info, its path within the application, which is exactly what the router matched. Nelmio'spathsare those route paths.OpenApiSpecCacheleavesserversout of the cached document at root, path and operation level.Other runtime env values: env vars used by the rest of Nelmio's
documentation, or by an area's security configuration (for example a security scheme's header name), are collected per area when the container is compiled. Their runtime values are part of the cache file name, so a value that differs from the one at warm-up generates its own document once.default::, so an unset variable doesn't break the container.servers(root, path and operation level) are excluded, since validation no longer uses them.kernel.cache_dirinstead, so it is still generated only once.Also:
cache:warmuplogs a warning on thestixx_openapi_commandchannel when it can't cache an area's document, for example because an env var Nelmio's configuration uses is unset. It used to skip the area silently.openapi.v2.…): documents cached by 0.14.1 aren't reused, which matters because production never regenerates them.docs/validation.md: the 0.14.1 note about frozen runtime values is replaced with the new behaviour, plus advice to define Nelmio's env vars duringcache:warmup.Tests
RuntimeEnvironmentTest): warm withAPI_URL=http://localhost, then run withAPI_URL=https://api.example.com/v1./v1and an app that isn't both validate (201). On the 0.14.1 code the first gets 400./v1validates requests to an app not mounted below it (201; 400 on the 0.14.1 code).API_VERSION(fromdocumentation.info) generates one more document.servers./v1validates against path/test; without the change,NoPath);serversbut not a schema property namedservers.Summary by CodeRabbit