Add draft threat model + SECURITY.md + AGENTS.md for security-model discoverability#397
Open
potiuk wants to merge 1 commit into
Open
Add draft threat model + SECURITY.md + AGENTS.md for security-model discoverability#397potiuk wants to merge 1 commit into
potiuk wants to merge 1 commit into
Conversation
…l discoverability Adds a draft (v0) threat model for the control plane plus the SECURITY.md and AGENTS.md scaffold so an automated scan agent can mechanically discover the model via AGENTS.md -> SECURITY.md -> THREAT_MODEL.md. The model is a proposal for the PMC to review; most claims are (inferred) and route to open questions in section 14. Generated-by: Claude Code (Claude Opus 4.8)
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## unstable #397 +/- ##
============================================
+ Coverage 43.38% 49.69% +6.30%
============================================
Files 37 45 +8
Lines 2971 4103 +1132
============================================
+ Hits 1289 2039 +750
- Misses 1544 1838 +294
- Partials 138 226 +88
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
PragmaTwice
reviewed
May 31, 2026
Comment on lines
+243
to
+268
| **Wave 1 — the central auth question (§2/§5a/§8/§9):** | ||
| 1. Does the controller API have **any built-in authentication/authorization** (token, mTLS, basic-auth), or | ||
| is it **network-trust only**? If the latter, is exposing it behind an operator's own auth proxy the | ||
| *supported* posture, so an "unauthenticated API" report is `BY-DESIGN`? *Proposed:* network-trust only | ||
| today; operator fronts it; unauth-on-trusted-network is by design, unauth-on-untrusted-network is misuse. | ||
| 2. Does the **web UI** authenticate, and does it have **CSRF** protection on state-changing calls? | ||
| *Proposed:* same as the API; CSRF is a hardening item. | ||
| 3. Is **TLS** supported/expected for the API and managed-node connections? *Proposed:* operator-configurable; | ||
| plaintext acceptable only on trusted networks. | ||
|
|
||
| **Wave 2 — store, nodes, SSRF (§4/§6/§9):** | ||
| 4. How are **managed-node admin credentials** stored and used (in the store? in config?), and what protects | ||
| them at rest? *Proposed:* operator-supplied; stored in the metadata store; protected at the store layer. | ||
| 5. Are **node addresses** registered via the API validated/allow-listed, or will the controller connect to | ||
| any host:port given (SSRF)? *Proposed:* currently connects as given; allow-listing is a hardening item. | ||
| 6. Does the controller **trust the metadata store and peer controllers** as honest (out of §7), or should a | ||
| tampered store / Byzantine peer be modelled? *Proposed:* trusted; out of scope. | ||
|
|
||
| **Wave 3 — properties & §11a (§8/§11a):** | ||
| 7. What **failover-correctness / split-brain** guarantees does the controller make, and under what store/peer | ||
| assumptions? *Proposed:* correct given a healthy store and a quorum of honest peers. | ||
| 8. What do scanners/researchers most often report here that the PMC considers a **non-finding**? (Seeds §11a.) | ||
|
|
||
| **Meta:** | ||
| 9. Confirm this model lives as root `THREAT_MODEL.md` referenced from a new `SECURITY.md`, separate from the | ||
| `apache/kvrocks` model. *Proposed:* yes. |
Member
There was a problem hiding this comment.
cc @git-hulk Could you confirm and answer these questions when you have time?
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
3 participants
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.
This is a draft proposal for the Kvrocks PMC to review — please correct, reject, or discuss as needed. Nothing here is a requirement; the maintainers are the decision-makers.
This is the companion to the
apache/kvrocksthreat-model PR, covering the control plane (its trust surface differs from the data node). It addsTHREAT_MODEL.md+SECURITY.md+AGENTS.mdso a scan agent can followAGENTS.md → SECURITY.md → THREAT_MODEL.md.Draft-first, mostly inferred (~12 documented / 0 maintainer / ~40 inferred); every
*(inferred)*claim routes to a numbered §14 question.The wave-1 question is the whole ballgame for a control plane:
config.yamlshows no API-auth knob and defaultaddris127.0.0.1:9379)? If network-trust-only, is fronting it with an operator auth proxy the supported posture — so an "unauthenticated admin API" report isBY-DESIGNrather thanVALID?Also flagged: TLS posture, SSRF via node registration, and how managed-node admin credentials are stored.
Context: the ASF Security team is preparing the project for an automated agentic security scan we're piloting; a discoverable threat model keeps that scan's output signal-rich. Drafted via the threat-model-producer rubric. If you'd rather author it yourselves, close this PR and we'll regroup.