Add THREAT_MODEL.md per the Apache security threat-model rubric#15664
Add THREAT_MODEL.md per the Apache security threat-model rubric#15664jamesfredley wants to merge 5 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
Adds an Apache Grails project threat model document at the repository root, following the ASF Security threat-model rubric. The document defines what the framework defends against, what it disclaims, and how to triage reports. Marked as a DRAFT with 22 open questions for the PMC.
Changes:
- New
THREAT_MODEL.md(501 lines) defining scope, trust boundaries, claimed/disclaimed security properties, misuse patterns, known non-findings, and a closed set of triage dispositions. - New
threat-model.yamlmachine-readable companion mirroring the prose for automated triage tooling. SECURITY.mdcross-references the new threat model.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| THREAT_MODEL.md | New top-level threat-model document covering §1–§15 plus appendix back-mapping to existing security guide. |
| threat-model.yaml | YAML sidecar with components, knobs, entry points, properties, disclaimers, false friends, known non-findings, and dispositions. |
| SECURITY.md | Adds a short paragraph pointing reporters/triagers at THREAT_MODEL.md. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Replace 13 absolute file:/// URLs that pointed at the PR author's local workstation (file:///C:/Users/james/...) with ./-relative paths matching the convention used elsewhere in the document. Affects the §2 component- family table and the §11a BeforeValidateHelper reference. Addresses Copilot PR review feedback on apache#15664 (comment) and apache#15664 (comment) Assisted-by: claude-code:claude-opus-4-7
Adds a project threat model that defines the implicit security contract between the Grails framework and its downstream users: what is in scope, what is out, which security properties the framework claims, which it explicitly disclaims, and how inbound vulnerability reports and tool findings are triaged. The document follows the structure mandated by the threat-model-producer rubric maintained by the ASF Security team (public mirror at https://gist.github.com/potiuk/da14a826283038ddfe38cc9fe6310573), covering sections 1 through 15 inclusive, with provenance tags on every non-trivial claim. Existing security guidance in grails-doc/src/en/guide/security/ is mined as the documented source-of-truth and back-mapped in Appendix A; nothing in the existing documentation is dropped, weakened, or contradicted. This is a DRAFT. The document is published in draft-first mode (rubric section 3.2). 22 open questions for the PMC are collected in section 14, grouped into three waves and framed as proposed answers for confirmation or correction. Inferred claims should be promoted to maintainer-confirmed as those answers land. Additions: - THREAT_MODEL.md: prose document, 15 sections plus a back-map appendix. - threat-model.yaml: machine-readable companion (rubric section 15) for automated and AI-assisted triage tooling. Derived from the prose document; the prose remains canonical. - SECURITY.md: cross-reference paragraph pointing at THREAT_MODEL.md, per rubric section 1. Assisted-by: claude-code:claude-opus-4-7
1d7a53b to
6a760c4
Compare
Convergent findings from five parallel reviewers (oracle rubric audit, librarian comparative review against apache/airflow + apache/apisix + apache/logging-site, two explore agents for ground-truth verification, and an editorial deep pass) prompted these changes. Every factual claim added here was verified directly against grails-core 8.0.x source. Factual corrections - §5a multipart upload defaults were stated as Spring Boot's 1 MB / 10 MB. Actual default is 128000 bytes (~125 KB) for both maxFileSize and maxRequestSize, set by ControllersAutoConfiguration. Fixed in prose and YAML. Structural additions - §7 adversary model now opens with two crisp documented trust statements: the existing URL-reachability quote, plus a new plugin/classpath/ application.groovy trust statement that discharges the largest single class of inbound reports (modeled after apache/airflow's DAG-author trust line). - §7 adds an explicit "Not applicable - single-process, no consensus protocol" line for distributed-system Byzantine adversaries, per rubric §7. - §8 properties table gains a CWE column (P1-P9 each mapped to its CWE) so automated scanners can route findings directly to model properties. - §9 false-friend list expands with two new entries: GRAILS_ENV=development is not a security boundary, and grails.config.locations is a code- execution path rather than a configuration-file path. The existing bindable=false entry now states clearly that it is neither an access control nor a validation constraint. Surface-inventory completeness (§11, §11a) - Adds §11 misuse for Liquibase Groovy change-log paths that may be externally influenced (GroovyChangeLogParser in grails-data-hibernate5/ dbmigration evaluates change-log Groovy via GroovyShell.parse(...).run()). - §11a now cites the §6 trust assumption or §8 invariant that discharges each non-finding, per rubric §11a. The seven entries that previously lacked a citation now have one. - §11a is expanded to cover newly inventoried framework surfaces: every @GroovyASTTransformation implementation in shipped JARs (the full 25-entry set, including RollbackTransform and OrderedGormTransformation that earlier drafts missed); the additional Class.forName callers (DatabaseMigrationCommand.passwordEncryptionCodec, ConfigSupport, ExecuteStep profile-YAML className); GrailsApplicationScriptRunner; the SpEL usage in GroovyEclipseCompilationHelper (compile-time IDE helper, not runtime); SynchronizerTokensHolder Serializable; the absence of any other custom readObject methods beyond BeforeValidateHelper; the absence of any framework-shipped @RestController / @controller beans; and the absence of any production path-traversal sink. Editorial cleanup - §3 drops the CodeQL / CycloneDX paragraph (build-hygiene drift - rubric §1 explicitly forbids these in a threat model). - §11 drops the Maven-HTTPS / checksum bullet (same reason). - Hedge tags normalized. The non-standard forms `*(documented: framework default)*` and `*(inferred - canonical OWASP class)*` and `*(inferred - §14 wave N)*` are replaced with the three permitted tags only, with citations or §14 cross-references moved outside the tag. - §11 entries for HQL concatenation and params.id are tightened to state the concrete harm and the minimal safe replacement, per rubric §11. - §9 tutorial parenthetical removed from the well-known-attack-classes preamble. - §8 P9 severity cell now contains only the tier label; the resource- consumption framing remains in the dedicated "Resource consumption line" sub-section. - §1 draft-confidence count updated to ~48 documented / 0 maintainer / ~64 inferred to track the added claims. YAML companion alignment - Added missing §5a knobs (dateFormats, serverURL) and corrected the multipart defaults to 128000 bytes. - Added conditions: and cwe: fields to every §8 property entry so the YAML mirrors the prose table. - Added discharged_by: and disposition: fields to every §11a entry. - Added two new false-friend entries to match the prose. - Added an inventory: list naming every shipped @GroovyASTTransformation implementation so SAST suppression configs can consume it directly. - Updated provenance counts to track the prose. No claim is added without provenance. No claim is added without being verified against 8.0.x source at the cited file path. Assisted-by: claude-code:claude-opus-4-7
Assisted-by: claude-code:claude-4.7-opus
Assisted-by: claude-code:claude-4.7-opus
|
This PR is the last step before Mythos review of grails-core. |
🚨 TestLens detected 2 failed tests 🚨Here is what you can do:
Test Summary
🏷️ Commit: 7696e32 Test FailuresGroovyChangeLogSpec > outputs a warning message by calling the warn method (:grails-data-hibernate5-dbmigration:test in CI - Groovy Joint Validation Build / build_grails)GroovyChangeLogSpec > updates a database with Groovy Change (:grails-data-hibernate5-dbmigration:test in CI - Groovy Joint Validation Build / build_grails)Muted TestsNote Checks are currently running using the configuration below. Select tests to mute in this pull request: 🔲 GroovyChangeLogSpec > outputs a warning message by calling the warn method Reuse successful test results: 🔲 ♻️ Only rerun the tests that failed or were muted before Click the checkbox to trigger a rerun: 🔲 Rerun jobs Learn more about TestLens at testlens.app. |
Summary
Adds an Apache Grails project threat model at the repository root, matching the section structure (§1-§15) and provenance-tagging discipline mandated by the ASF Security team's threat-model-producer rubric (public mirror: https://gist.github.com/potiuk/da14a826283038ddfe38cc9fe6310573).
The document defines the implicit contract between the framework and its downstream users:
grails-shell-cli.grails-console), the start.grails.org HTTP service, third-party plugins, scaffolded application output, build/CI hygiene, Spring/Hibernate/JDK internals, and side-channel attacks.useTokenCSRF,bindable=falseenforcement, AST-transform input domain, configuration-load integrity, and bounded data-binding memory. Each property carries a CWE for automated scanner alignment.useToken, XSS with codec disabled, HQL string concatenation, object-level authorization on guessable IDs, rate limiting, constant-time crypto, Groovy sandboxing, profile-JAR provenance, cross-tenant isolation, and transport security. Eight false-friend properties (@Secured,bindable=false,useToken,constraints {}, GSP HTML comments,grails.serverURL,GRAILS_ENV=development,grails.config.locations) are called out separately - the rubric flags §9 as the highest-value section for integrators.application.groovytrust statement modeled afterapache/airflow's DAG-author convention.VALID,VALID-HARDENING,OUT-OF-MODEL: trusted-input,OUT-OF-MODEL: adversary-not-in-scope,OUT-OF-MODEL: unsupported-component,OUT-OF-MODEL: non-default-build,BY-DESIGN: property-disclaimed,KNOWN-NON-FINDING,MODEL-GAP), each citing the section that licenses it.@GroovyASTTransformationimplementations, every Class.forName / GroovyShell / SpEL caller verified at its actual source path, and three negative claims independently verified (no other customreadObjectbeyondBeforeValidateHelper, no framework-shipped@RestController/@Controllerbeans, no production path-traversal sink).Why
The repository previously had only a disclosure-policy
SECURITY.md(routes reporters to the ASF Security Team) and developer-facing guidance undergrails-doc/src/en/guide/security/. Neither tells a triager or downstream integrator which threats the framework has taken on and which it has left to the application. Findings from SAST, fuzzers, and AI-assisted analyzers therefore lack a documented basis for closure - every report gets re-derived from scratch.This document fixes that. It also gives a downstream integrator the answer to "if I drop Grails into my system, which threats am I now responsible for, and which does the framework own?"
Files added / changed
THREAT_MODEL.md(new, 516 lines) - the prose document, 15 sections plus a back-map appendix.threat-model.yaml(new, 439 lines) - machine-readable companion (§15), the suggested sidecar for automated triage tooling. Mirrors components, config knobs, entry points, properties (with CWE + conditions + violation symptoms), disclaimers, false friends, known non-findings (with discharged-by + disposition labels), and the closed set of disposition labels. The prose document is canonical.SECURITY.md- adds a one-paragraph cross-reference toTHREAT_MODEL.md, per rubric §1.Existing security guidance in
grails-doc/src/en/guide/security/is mined as the (documented) source-of-truth and back-mapped in Appendix A. Nothing in the existing documentation is dropped, weakened, or contradicted.Status: DRAFT
This is a draft-first model in the rubric's sense (rubric §3.2). 22 open questions for the PMC are collected in §14, grouped into three waves of 3-7 questions each, and framed as proposed answers for the PMC to confirm, correct, or strike. Wave 1 covers scope and intended use - the load-bearing decisions for the rest of the document. Provenance count at draft: ~48 documented / 0 maintainer / ~64 inferred. As maintainer answers land,
*(inferred)*tags should be promoted to*(maintainer)*and the matching open questions removed.Verification of every factual claim
Every file path, class name, default value, and behavioral assertion was verified directly against
8.0.xsource before commit:ControllersAutoConfiguration.java: confirmed@Value("${...maxFileSize:128000}")and@Value("${...maxRequestSize:128000}")- both default to 128000 bytes (~125 KB), not Spring Boot's 1 MB / 10 MB.SimpleDataBinder.groovy: confirmedautoGrowCollectionLimit = 256.DefaultLinkGenerator.groovy: confirmedURLEncoder.encode()is used for both key and value in link generation.BeforeValidateHelper.java: confirmed it implementsSerializablewith a customreadObjectmethod; confirmed there is no other customreadObjectin framework runtime code.@GroovyASTTransformationimplementation classes verified by name and path.GroovyChangeLogParser,GrailsApplicationScriptRunner,ExternalConfigRunListener,GroovyEclipseCompilationHelper,SynchronizerTokensHolder,DatabaseMigrationCommand.passwordEncryptionCodec,ConfigSupport,ExecuteStep- all confirmed at their stated paths.@RestControlleror@ControllerSpring bean shipped in framework JARs - every HTTP entry point is user-defined.Review checklist
grails-console,grails-forge, andgrails-test-examples(all marked out of model).grails.views.default.codec=htmlis the supported production posture (a deployment with codec disabled is thenOUT-OF-MODEL: non-default-build).bindable=falsemissing?application.groovytrust statement the right framing? Cross-referenced from §3, §9, §13.*(inferred)*->*(maintainer)*as you go.Refinement pass after Copilot review
The initial commit drew Copilot review (two inline comments on absolute path links) and a follow-up multi-agent review pass: rubric self-check (oracle), comparative review against
apache/airflow/apache/apisix/apache/logging-siteexemplars (librarian), ground-truth verification of every file/class reference (explore), missed-attack-surface hunt (explore), and editorial pass (deep-category). Convergent findings were applied infbafcd6a0c:125 KB, not1 MB/10 MB).@GroovyASTTransformationinventory and additional Class.forName / GroovyShell / SpEL callers to §11a with §6 or §8 discharge citations; added one new misuse pattern to §11; verified the negative claims about Java deserialization and Spring HTTP-handler beans.discharged_byanddispositionfields per §11a entry, and a structured inventory list of@GroovyASTTransformationclasses for SAST consumption.Cross-references
grails-doc/src/en/guide/security/Notes for the reviewer
*(documented)*/*(maintainer)*/*(inferred)*. Do not strip these on accepting - they are the audit trail used when closing a report ("not a bug - §9, (maintainer, 2026-01)").Assisted-by: claude-code:claude-opus-4-7