Skip to content

Commit 5f655dc

Browse files
stevehansenclaude
stevehansen
and
claude
committed
docs: Add STRIDE threat model and security documentation section
Identifies 16 threats across all STRIDE categories. Key findings: SQL injection in DatabaseConnection.GetViewDefinition (T-1, #93), connection string exposure via CLI args (I-1) and config files (I-2). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 3f166f6 commit 5f655dc

2 files changed

Lines changed: 260 additions & 0 deletions

File tree

‎CLAUDE.md‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,3 +116,29 @@ Tests use **NUnit** with **Shouldly** assertions. The standard pattern:
116116
## Documentation
117117

118118
When adding or changing user-facing features, always update **both** `README.md` and `CLAUDE.md` to reflect the changes. README.md is the public documentation for users; CLAUDE.md is the architecture reference for AI-assisted development. Keep them in sync.
119+
120+
## Security Documentation
121+
122+
### STRIDE.md Threat Model
123+
124+
This repository includes a STRIDE threat model (`STRIDE.md`) for security analysis.
125+
126+
**When to update STRIDE.md:**
127+
- Adding new authentication/authorization mechanisms
128+
- Changing data storage, encryption, or secrets handling
129+
- Adding new external integrations or API endpoints
130+
- Modifying trust boundaries (new external connections, database access)
131+
- After security incidents or penetration test findings
132+
- When addressing security recommendations from the document
133+
134+
**How to update:**
135+
1. Add new threats to the relevant STRIDE category (Spoofing, Tampering, Repudiation, Information Disclosure, Denial of Service, Elevation of Privilege)
136+
2. Assess likelihood (Very Low → High) and impact (Low → Critical)
137+
3. Document existing mitigations or add recommendations
138+
4. Link GitHub issues for unresolved findings
139+
5. Update the Review History table
140+
6. Update version if using frontmatter
141+
142+
**Tracking critical findings:**
143+
- Critical/High risk findings should have a linked GitHub issue with `security` label
144+
- Review STRIDE.md annually or after major releases

‎STRIDE.md‎

Lines changed: 234 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,234 @@
1+
# sql-inliner — STRIDE Threat Model
2+
3+
## Document Information
4+
5+
| Field | Value |
6+
|-------|-------|
7+
| Application | sql-inliner |
8+
| Version | v1 |
9+
| Created | 2026-02-28 |
10+
| Last Updated | 2026-02-28 |
11+
| Next Review | 2029-02-28 |
12+
13+
## 1. System Overview
14+
15+
### Application Description
16+
17+
sql-inliner is a .NET CLI tool and NuGet library that optimizes SQL Server views by inlining nested views into a single flattened query, optionally stripping unused columns and joins. It parses SQL using Microsoft's ScriptDom (TSql150Parser) and uses the visitor pattern to analyze and transform the AST.
18+
19+
### User Types
20+
21+
| User Type | Description | Trust Level |
22+
|-----------|-------------|-------------|
23+
| Developer/DBA | Runs the CLI tool locally or in CI/CD to optimize SQL views | High — has direct database access |
24+
| Library Consumer | Uses the NuGet library programmatically in their own application | High — embeds tool in their code |
25+
| CI/CD Pipeline | Automated execution via GitHub Actions or similar | Medium — runs with configured credentials |
26+
27+
### Components
28+
29+
| Component | Technology | Description |
30+
|-----------|-----------|-------------|
31+
| CLI Entry Point | System.CommandLine | Parses arguments, orchestrates commands |
32+
| Inliner Engine | ScriptDom AST | Parses, transforms, and regenerates SQL |
33+
| Database Connection | Dapper / SqlConnection | Queries `sys.views`, `OBJECT_DEFINITION()`, executes validation SQL |
34+
| Optimize Subsystem | Interactive wizard | Step-by-step view optimization with benchmarking |
35+
| Validate Subsystem | Batch processing | Bulk inlines and validates all views |
36+
| Verify Subsystem | Comparison runner | Validates deployed inlined views against originals |
37+
| Analyze Subsystem | Query Store analysis | Identifies inlining candidates by usage stats |
38+
| Config Loader | System.Text.Json | Loads `sqlinliner.json` with connection strings and view paths |
39+
| Session Directory | File I/O | Saves iteration files, execution plans, benchmark reports |
40+
41+
### Data Flow Diagram
42+
43+
```
44+
┌─────────────────┐
45+
│ User (CLI) │
46+
│ --view-name │
47+
│ --conn-string │
48+
│ --config │
49+
└────────┬────────┘
50+
│
51+
┌────────▼────────┐
52+
│ System.Command │
53+
┌────────│ Line Parser │────────┐
54+
│ └────────┬────────┘ │
55+
│ │ │
56+
┌──────▼──────┐ ┌──────▼──────┐ ┌──────▼──────┐
57+
│ sqlinliner │ │ --view-path│ │ --config │
58+
│ .json │ │ (SQL file) │ │ (explicit) │
59+
│ (auto-disc)│ └──────┬──────┘ └──────┬──────┘
60+
└──────┬──────┘ │ │
61+
│ ┌──────▼────────────────▼──────┐
62+
└────────►│ DatabaseConnection │
63+
│ • GetViewDefinition() │
64+
│ • sys.views query │
65+
└──────┬──────────────┬────────┘
66+
│ │
67+
┌──────▼──────┐ ┌─────▼────────┐
68+
│ ScriptDom │ │ SQL Server │
69+
│ AST Parse │ │ Database │
70+
│ & Transform│ │ (Dapper) │
71+
└──────┬──────┘ └─────┬────────┘
72+
│ │
73+
┌──────▼──────────────▼────────┐
74+
│ Output │
75+
│ • stdout (inlined SQL) │
76+
│ • Session files (.sql, .log) │
77+
│ • Benchmark reports (.html) │
78+
└──────────────────────────────┘
79+
```
80+
81+
### Trust Boundaries
82+
83+
| Boundary | From | To | Data Crossing |
84+
|----------|------|----|---------------|
85+
| TB-1 | User | CLI | View names, connection strings, config paths, SQL file paths |
86+
| TB-2 | CLI | SQL Server | SQL queries (OBJECT_DEFINITION, COUNT, EXCEPT, CREATE/DROP VIEW) |
87+
| TB-3 | CLI | File System | Config files (read), SQL files (read/write), session logs, reports |
88+
| TB-4 | SQL Server | CLI | View definitions, query results, execution plans, statistics |
89+
| TB-5 | NuGet | Consumer App | Library API (view SQL, options) — no DB connection in library mode |
90+
91+
### Data Classification
92+
93+
| Data Type | Classification | Examples |
94+
|-----------|---------------|----------|
95+
| Database Credentials | **Sensitive** | Connection strings with passwords in config/CLI |
96+
| View Definitions | Internal | SQL source code from `OBJECT_DEFINITION()` |
97+
| Query Statistics | Internal | CPU time, logical reads, execution plans |
98+
| Environment Metadata | Internal | Machine name, user name (in benchmark reports) |
99+
| Session Files | Internal | Iteration SQL, logs, benchmark HTML |
100+
101+
## 2. STRIDE Analysis
102+
103+
### S — Spoofing
104+
105+
| ID | Threat | Attack Path | Likelihood | Impact | Score | Mitigation |
106+
|----|--------|-------------|------------|--------|-------|------------|
107+
| S-1 | Malicious config file injection | Attacker places a crafted `sqlinliner.json` in the working directory; tool auto-discovers and loads it | 2 | 3 | 6 | Tool auto-discovers only in CWD. Users should verify CWD before running. No remote config loading. |
108+
| S-2 | Connection string substitution | Attacker replaces config file to redirect DB connections to a malicious server | 2 | 3 | 6 | Config files are local; OS file permissions protect them. Users should not run tool in untrusted directories. |
109+
110+
**Countermeasures:**
111+
- Config auto-discovery is limited to the current working directory
112+
- No remote configuration loading — all paths are local
113+
- Connection strings are validated through `SqlConnectionStringBuilder`
114+
- Library build (`ReleaseLibrary`) excludes all DB-connected subsystems
115+
116+
### T — Tampering
117+
118+
| ID | Threat | Attack Path | Likelihood | Impact | Score | Mitigation |
119+
|----|--------|-------------|------------|--------|-------|------------|
120+
| T-1 | SQL injection via `--view-name` | User-supplied view name is interpolated into `OBJECT_DEFINITION(object_id('...'))` without parameterization | 2 | 4 | **8** | **Unmitigated.** String interpolation in `DatabaseConnection.cs:78,111`. Should use parameterized queries. |
121+
| T-2 | Malicious SQL file via `--view-path` or config `views` | Attacker provides a crafted `.sql` file that contains malicious SQL; tool parses and may deploy it | 2 | 3 | 6 | ScriptDom parses the SQL as a view definition (CREATE VIEW). Deployment only occurs in interactive optimize flow with user confirmation. |
122+
| T-3 | Path traversal in config view paths | Config `views` dictionary specifies `../../../sensitive.sql` to read arbitrary files | 2 | 2 | 4 | `Path.GetFullPath()` normalizes paths but does not validate containment within config directory. Impact limited to reading files as SQL (would fail parsing). |
123+
| T-4 | Session file tampering | Attacker modifies iteration files between optimize steps to inject SQL | 1 | 3 | 3 | Session files are in a timestamped directory. SHA256 hash comparison detects edits. User confirms before deployment. |
124+
| T-5 | Deployed view tampering via CREATE/DROP | Tool executes `CREATE VIEW` and `DROP VIEW` DDL on the target database | 2 | 3 | 6 | Only in validate/verify/optimize subsystems. DROP targets only tool-created views (`_Inlined`, `_Validate`, `_Original` suffixes). Bracketed identifiers prevent injection in DDL. |
125+
126+
**Countermeasures:**
127+
- ScriptDom AST parsing rejects non-view SQL structures
128+
- SHA256 hash verification for session file edits
129+
- Bracketed identifiers `[schema].[viewName]` in DDL statements (QueryRunner, ValidateSession, VerifySession)
130+
- Interactive confirmation before deployment in optimize flow
131+
- **Gap:** `GetViewDefinition()` and `TryGetRawViewDefinition()` use string interpolation — should be parameterized
132+
133+
### R — Repudiation
134+
135+
| ID | Threat | Attack Path | Likelihood | Impact | Score | Mitigation |
136+
|----|--------|-------------|------------|--------|-------|------------|
137+
| R-1 | No audit trail for view deployments | Views are deployed (CREATE OR ALTER VIEW) without logging who deployed or when | 2 | 2 | 4 | Session log records operations with timestamps. Git history tracks source changes. SQL Server has its own audit capabilities. |
138+
| R-2 | Validate/verify operations not logged | Batch operations modify database (temporary views) without persistent audit | 2 | 2 | 4 | Validate writes `validate-errors.log`. Verify cleans up temp views in `finally` blocks. Operations are transient. |
139+
140+
**Countermeasures:**
141+
- Session directory logs all optimize operations with timestamps
142+
- Validate writes error logs to output directory
143+
- SQL Server's own auditing captures DDL changes
144+
- Git tracks all source code changes
145+
146+
### I — Information Disclosure
147+
148+
| ID | Threat | Attack Path | Likelihood | Impact | Score | Mitigation |
149+
|----|--------|-------------|------------|--------|-------|------------|
150+
| I-1 | Connection string exposure via CLI arguments | `--connection-string` visible in process list, shell history, CI/CD logs | 3 | 3 | **9** | **Partially mitigated.** Config file and environment-based alternatives exist, but CLI still accepts plaintext. No built-in redaction. |
151+
| I-2 | Connection strings in config files | `sqlinliner.json` stores connection strings with embedded passwords; file not gitignored by default | 3 | 3 | **9** | **Partially mitigated.** Users can use Windows Authentication (no password). Config file not gitignored — could be committed accidentally. |
152+
| I-3 | Session files contain view SQL and metadata | Session directories store full view SQL, execution plans, table names, machine/user names | 2 | 2 | 4 | Files are local to the user's machine. Default OS permissions apply. No automatic cleanup. |
153+
| I-4 | Exception messages may leak paths or SQL | Error messages expose file paths and database errors to the user | 2 | 1 | 2 | Acceptable for a CLI tool. Errors are displayed to the authenticated user only. |
154+
| I-5 | Benchmark reports contain environment info | HTML reports include `Environment.MachineName`, `Environment.UserName`, database version, table names | 2 | 2 | 4 | Reports are local files. Users should treat them as internal artifacts. |
155+
156+
**Countermeasures:**
157+
- Windows Authentication / Integrated Security eliminates password exposure
158+
- `SqlConnectionStringBuilder` normalizes connection strings (does not add passwords)
159+
- Session files are local with OS-level access control
160+
- **Gap:** `sqlinliner.json` is not in `.gitignore` by default — credential leak risk
161+
- **Gap:** No automatic cleanup of session directories
162+
163+
### D — Denial of Service
164+
165+
| ID | Threat | Attack Path | Likelihood | Impact | Score | Mitigation |
166+
|----|--------|-------------|------------|--------|-------|------------|
167+
| D-1 | Deeply nested view chains cause stack overflow | Maliciously crafted view definitions with extreme nesting depth cause recursive inlining to fail | 1 | 2 | 2 | ScriptDom parsing has practical limits. Recursive inlining follows actual view references. |
168+
| D-2 | Large view output overwhelms resources | Inlining a view with hundreds of nested references produces extremely large SQL output | 2 | 2 | 4 | This is expected behavior for complex views. Users can limit scope with `--view-name`. |
169+
| D-3 | Validate/verify timeout abuse | Long-running validation queries consume database resources | 2 | 2 | 4 | Configurable `--timeout` (default 90s). Per-query timeout via `CommandTimeout`. Timeouts are reported, not retried. |
170+
| D-4 | Session directory disk exhaustion | Many optimize sessions accumulate without cleanup | 2 | 1 | 2 | Directories are small (SQL files + HTML). No automatic cleanup, but manageable manually. |
171+
172+
**Countermeasures:**
173+
- Configurable command timeouts for all database operations
174+
- `--stop-on-error` flag to halt batch operations early
175+
- Per-query timeout handling with graceful error reporting
176+
- ScriptDom parsing limits prevent infinite recursion
177+
178+
### E — Elevation of Privilege
179+
180+
| ID | Threat | Attack Path | Likelihood | Impact | Score | Mitigation |
181+
|----|--------|-------------|------------|--------|-------|------------|
182+
| E-1 | SQL execution with connection credentials | Tool executes arbitrary SQL (CREATE VIEW, DROP VIEW, SELECT) with the privileges of the provided connection string | 2 | 3 | 6 | By design — tool requires database access. Users should use least-privilege accounts. DDL limited to view operations. |
183+
| E-2 | SQL injection escalates to DB admin | If T-1 is exploited, injected SQL runs with full connection privileges | 1 | 4 | 4 | Mitigated by fixing T-1 (parameterized queries). Connection should use least-privilege account. |
184+
| E-3 | Process.Start with UseShellExecute | `OptimizeSession.cs:255` opens a file with the default system handler via `Process.Start` | 1 | 2 | 2 | File path is generated by the tool (session directory). No user input in the path construction. |
185+
186+
**Countermeasures:**
187+
- Tool operates with the privileges of the provided connection string — no privilege escalation within the tool
188+
- `Process.Start` paths are tool-generated, not user-supplied
189+
- Library build excludes all database-connected code
190+
- **Recommendation:** Use dedicated SQL Server accounts with minimal permissions (VIEW DEFINITION, SELECT, CREATE VIEW, ALTER VIEW)
191+
192+
## 3. Risk Summary
193+
194+
### High Priority Threats (Score >= 8)
195+
196+
| ID | Threat | Score | Status |
197+
|----|--------|-------|--------|
198+
| T-1 | SQL injection via `--view-name` in `DatabaseConnection.cs` | 8 | Unmitigated |
199+
| I-1 | Connection string exposure via CLI arguments | 9 | Partially mitigated |
200+
| I-2 | Connection strings in config files (not gitignored) | 9 | Partially mitigated |
201+
202+
### Residual Risks
203+
204+
- **T-1 (SQL Injection):** The `--view-name` parameter flows directly into string-interpolated SQL. While the tool is a local CLI used by trusted developers/DBAs, this violates defense-in-depth. Fix: use parameterized queries (`new { viewName }`).
205+
- **I-1/I-2 (Credential Exposure):** Connection strings with embedded passwords can leak via process lists, shell history, CI logs, or accidentally committed config files. The recommended approach is Windows Authentication (no password in connection string). Adding `sqlinliner.json` to the `.gitignore` template would reduce the config file risk.
206+
207+
## 4. Security Controls Summary
208+
209+
| Category | Implementation |
210+
|----------|---------------|
211+
| Authentication | Delegates to SQL Server authentication (Windows Auth or SQL Auth via connection string) |
212+
| Authorization | Delegates to SQL Server permissions; tool requires VIEW DEFINITION, SELECT, and optionally CREATE/ALTER/DROP VIEW |
213+
| Input Validation | ScriptDom AST parsing validates SQL structure; `SqlConnectionStringBuilder` validates connection strings |
214+
| Output Encoding | SQL output generated by `Sql150ScriptGenerator` with proper escaping |
215+
| Secrets Management | Connection strings via CLI args or config file; no built-in secrets vault integration |
216+
| Audit Logging | Session logs with timestamps; `validate-errors.log` for batch operations |
217+
| Error Handling | Exceptions caught at command level; error messages displayed to user |
218+
| Dependency Security | CodeQL analysis in CI; no known vulnerable dependencies |
219+
| Build Security | NuGet OIDC trusted publishing; trimmed single-file release builds |
220+
| Data Protection | Session files use OS-level access control; SHA256 hash verification for edit detection |
221+
222+
## 5. Review History
223+
224+
| Version | Date | Reviewer | Changes |
225+
|---------|------|----------|---------|
226+
| v1 | 2026-02-28 | Claude Code (STRIDE analysis) | Initial threat model |
227+
228+
## 6. References
229+
230+
- [OWASP STRIDE](https://owasp.org/www-community/Threat_Modeling_Process#stride)
231+
- [Microsoft Threat Modeling](https://learn.microsoft.com/en-us/azure/security/develop/threat-modeling-tool)
232+
- [OWASP SQL Injection Prevention](https://cheatsheetseries.owasp.org/cheatsheets/SQL_Injection_Prevention_Cheat_Sheet.html)
233+
- [SQL Server Security Best Practices](https://learn.microsoft.com/en-us/sql/relational-databases/security/sql-server-security-best-practices)
234+
- [Dapper Parameterized Queries](https://www.learndapper.com/parameters)

0 commit comments

Comments
 (0)