Skip to content

Add initial security documentation (attack tree & threat model)#2659

Open
therealnb wants to merge 3 commits intomainfrom
docs/security-documentation
Open

Add initial security documentation (attack tree & threat model)#2659
therealnb wants to merge 3 commits intomainfrom
docs/security-documentation

Conversation

@therealnb
Copy link

Summary

This PR adds foundational security documentation for ToolHive, including an attack tree and threat model. This is just a starting point and definitely needs review from the security team. It may not be useful at all in its current form, but figured it's better to have something than nothing as a baseline for discussion.

What's Included

📊 Attack Tree (docs/security/attack-tree.md)

  • Visual attack vectors across Local, Kubernetes, and Remote MCP deployments
  • Distinguishes between ToolHive-specific attacks vs generic infrastructure
  • 8 separate diagrams (horizontal layout for readability)
  • Cost estimates for ~50 ToolHive-specific attack paths
  • 6 detailed attack chains with mitigations

🛡️ Threat Model (docs/security/threat-model.md)

  • STRIDE analysis (Spoofing, Tampering, Repudiation, Info Disclosure, DoS, Privilege Escalation)
  • Coverage of 11 major components (CLI, Desktop UI, Operator, Proxy Runner, MCP Containers, etc.)
  • Data flow diagrams for each deployment mode
  • 80+ specific threats with existing/missing mitigations
  • Top 10 critical threats (P0)
  • Security control recommendations by category

📖 Supporting Docs

  • docs/security/README.md - Index, quick reference, checklists
  • docs/security/SUMMARY.md - Executive summary of what was created

⚠️ Important Caveats

This definitely has some superfluous guff in it. There's probably too much generic security advice that applies to any containerized system, and some of the threat scenarios might be unrealistic or over-stated.

However, there are also some good bits:

  • ToolHive-specific attack vectors (RunConfig tampering, MCPRegistry poisoning, Cedar policy bypass, RFC 9728 discovery exploitation)
  • Concrete attack chains showing how vulnerabilities chain together
  • Actionable mitigations linked to actual ToolHive features
  • Cross-references to existing documentation

What This Needs

  • Security team review - Validate threats and priorities
  • Architecture team review - Confirm technical accuracy
  • Pruning - Remove generic content that doesn't add value
  • Gap analysis - Identify what security controls are actually missing
  • Integration - Link to actual implementation work (issues/epics)

Why This Might Be Useful

Even if 70% of this gets thrown away, having structured security documentation can:

  • Help with architectural decisions (e.g., "what's the threat model for X feature?")
  • Support security audits and compliance efforts
  • Provide a framework for security discussions
  • Identify gaps in existing security controls

Bottom line: This is a draft, not gospel. Use what's helpful, ignore or delete the rest. Looking for feedback on whether this approach is even worth pursuing.

Add comprehensive security documentation including:
- Attack tree with ToolHive-specific attack vectors
- STRIDE-based threat model covering all components
- Security best practices and review checklists
- Index and summary documents

This is an initial draft that needs security team review.
Some content may be superfluous, but provides a foundation
to build upon. Better to have something than nothing.
@codecov
Copy link

codecov bot commented Nov 19, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 55.24%. Comparing base (5f1a532) to head (bd660f9).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2659   +/-   ##
=======================================
  Coverage   55.24%   55.24%           
=======================================
  Files         315      315           
  Lines       30294    30294           
=======================================
  Hits        16736    16736           
  Misses      12097    12097           
  Partials     1461     1461           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@github-actions github-actions bot added the size/XL Extra large PR: 1000+ lines changed label Jan 13, 2026
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Large PR Detected

This PR exceeds 1000 lines of changes and requires justification before it can be reviewed.

How to unblock this PR:

Add a section to your PR description with the following format:

## Large PR Justification

[Explain why this PR must be large, such as:]
- Generated code that cannot be split
- Large refactoring that must be atomic
- Multiple related changes that would break if separated
- Migration or data transformation

Alternative:

Consider splitting this PR into smaller, focused changes (< 1000 lines each) for easier review and reduced risk.

See our Contributing Guidelines for more details.


This review will be automatically dismissed once you add the justification section.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/XL Extra large PR: 1000+ lines changed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants