Cognitivegears/issue14 - #29
Merged
Merged
Conversation
Changes to make Java work on M4 mac Added JSON schema generator Added unit testing and some refactoring Changes to generate body schema Make devcontainer work with docker desktop Refactoring part 1 Added Lombok Refactored and cleaned up
There was a problem hiding this comment.
Pull Request Overview
This PR introduces large-scale changes to support JSON Schema generation and enhances test coverage for the Modsecurity3 generator. Key changes include:
- New and expanded unit tests for model processing, JSON schema type mapping, and generator operations.
- New services and utility classes for JSON Schema generation, configuration, and pattern generation.
- Updates to CI/CD pipelines and documentation for testing and integration.
Reviewed Changes
Copilot reviewed 36 out of 41 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/test/java/com/oashield/openapi/generators/modsecurity3/tests/ModelProcessingTest.java | New tests verifying JSON Schema generation and model properties. |
| src/test/java/com/oashield/openapi/generators/modsecurity3/tests/JsonSchemaTypeMapperTest.java | Added tests for case-insensitive type mapping and default behaviors. |
| src/main/java/com/oashield/openapi/generators/modsecurity3/(TemplateManager, PatternGenerationService, OperationProcessor, ModelProcessor, JsonSchemaGenerator, ConfigurationManager) | New service classes for JSON Schema generation, processing operations/models, template management, and configuration. |
| README.md & .github/workflows/*.yml | Updated documentation and CI workflow definitions for unit and integration tests. |
Files not reviewed (5)
- .devcontainer/devcontainer.json: Language not supported
- .vscode/settings.json: Language not supported
- pom.xml: Language not supported
- src/main/resources/modsecurity3/config.mustache: Language not supported
- src/main/resources/modsecurity3/mainconfig.mustache: Language not supported
Comments suppressed due to low confidence (1)
src/test/java/com/oashield/openapi/generators/modsecurity3/tests/ModelProcessingTest.java:75
- The error message for checking the 'tags' property is misleading. Consider updating it to 'Pet model should have tags property' to better reflect the context.
assertTrue(petProperties.has("tags"), "Tag model should be included");
| * @param baseNamePrefix The prefix to add to the property name | ||
| * @return A list of flattened properties | ||
| */ | ||
| public List<CodegenProperty> flattenModel(CodegenProperty currentProperty, String baseNamePrefix) { |
There was a problem hiding this comment.
A similar flattenModel method is also implemented in ModelProcessor. Consider refactoring to a common utility to reduce duplication and improve maintainability.
# Conflicts: # pom.xml
# Conflicts: # pom.xml
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…d Coraza - engineFlavor=modsecurity3|coraza (default modsecurity3); coraza flavor adds @validateSchema (JSON Schema, Coraza-only operator; path via schemaRulePath), modsecurity3 relies on generated per-field rules (both flavors emit them) - Fix root causes of broken body checking: requestBodyProcessor ctl moved to phase 1 (was a phase-2 no-op), Content-Type gate used numeric @eq, REQBODY_ERROR check missing @; unknown-parameter rules referenced nonexistent hasPostParams - Replace Coraza-only @restpath/ARGS_PATH (issue #42) with a single path regex embedding each path parameter's validation pattern - Per-field JSON body rules from flattened models: required presence (nested guarded on parent), type patterns, numeric @lt/@gt ranges, one ARGS_NAMES allowlist incl. container prefixes and both engines' array key forms - flattenModel: resolve $ref via model lookup, arrays via items, bounded depth, clone leaves instead of mutating shared models - Numeric min/max on params via @lt/@gt rules instead of range regexes - Sanitize spec patterns (strip /.../ delimiters, undo backslash doubling) Verified end-to-end against owasp/modsecurity-crs:nginx (libmodsecurity 3.0.16) and coraza-validate-server: 11/11 behavioral checks pass on both. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- ModSecurityContainerManager: owasp/modsecurity-crs:nginx (linux/386) with CRS neutralized via nginx template fixtures; requests proxied to a local upstream so ModSecurity's body phase runs - WafContainerManager interface; fix CorazaContainerManager rules mount (/app/rules, not /etc/coraza/rules - the server never saw the rules before) - Health checks accept 403: with rules loaded, / is an undefined endpoint and default-deny proves the WAF is enforcing - Feature scenarios converted to Scenario Outlines running per engine; new body-object scenario (missing_required/invalid_type/extra_property -> 403) - RuleGenerationUtil: engineFlavor + schemaRulePath plumbing; replace dead skipJsonSchema with validateBodySchema/generateJsonSchema=false - Fix undefined-method step: DELETE /pet/1 is a real petstore operation, use PATCH - Testcontainers 1.19.7 -> 1.21.4 (Docker Engine 29 rejects its API version) - Docs: engineFlavor usage, per-field validation limitations, dual-engine testing prerequisites and troubleshooting mvn verify: 10/10 scenarios green (5 scenarios x coraza+modsecurity3). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
…etup Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The artifact still targets Java 8 (maven-compiler source/target 1.8); only the build/test JDK changes. This was masked because CI had been failing at job setup on the deprecated artifact actions since before the JUnit 6 bump landed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…lies Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Maven (supersedes open dependabot PRs #62-#66): - openapi-generator/-cli 7.19.0 -> 7.23.0, junit-jupiter 6.0.2 -> 6.1.1 - cucumber 7.24.0 -> 7.34.4, rest-assured 5.5.6 -> 6.0.0, testng 7.10.2 -> 7.12.0 - mockito 5.11.0 -> 5.23.0, jackson-databind 2.19.2 -> 2.22.0, lombok 1.18.46 - plugins: surefire/failsafe 3.5.5, compiler 3.15.0, enforcer 3.6.3, jacoco 0.8.15 GitHub Actions (clears Node 20 runner deprecation warnings; supersedes #67/#68): - checkout v2/v4 -> v7, setup-java v2/v3 -> v5, upload-artifact v4 -> v7, download-artifact v4 -> v8, action-gh-release v1 -> v3 - release.yml: replace archived create-release@v1 + upload-release-asset@v1 (whose glob asset path never worked) with one action-gh-release step; build releases on JDK 17/temurin Verified: mvn clean verify green - 227 unit tests, 10/10 integration scenarios against both Coraza and ModSecurity3 containers. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ing) contents: read for build/test workflows; contents: write only where releases are created (maven-build-cli.yml tag releases, release.yml). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Large scale changes for supporting JSON schema and additional test changes.