[FEATURE] Static code security assessment

[HannahVernon]

Which component(s) does this affect?

• Full Dashboard
• Lite
• SQL collection scripts
• Installer
• Documentation

Problem Statement

Performance Monitor handles SQL Server credentials, executes dynamic SQL, manages local DuckDB databases with parquet files, and spawns external processes. There has not been a formal security review of the codebase to identify potential vulnerabilities across these attack surfaces.

As the project matures and gains adoption, a static code security assessment would help identify and remediate issues before they become risks for users running the tool against production SQL Server instances.

Proposed Solution

Conduct a static code security assessment covering the following areas, roughly in priority order:

1. Credential & Connection String Handling (High Priority)

• Audit how SQL Server credentials are stored, transmitted, and persisted in both Dashboard (in-memory) and Lite (DuckDB) editions
• Verify connection strings use Encrypt=true / TrustServerCertificate appropriately
• Check for credentials in logs, exception messages, or telemetry output

2. SQL Injection / Dynamic SQL (High Priority)

• Review all parameterized vs. concatenated SQL in collection scripts and application code
• Audit DuckDB query construction in LocalDataService methods
• Check MCP query tools (McpQueryTools.cs) for injection vectors

3. File Path & Process Execution (Medium Priority)

• Audit all Process.Start call sites for command injection via user-influenced paths
• Review parquet/DuckDB file path construction for path traversal
• Check installer script paths and ScriptProvider for traversal risks

4. Local Data Security (Medium Priority)

• Assess DuckDB file permissions (data at rest)
• Review parquet archive file handling and cleanup
• Check dismissed_archive_alerts sidecar table for data leakage

5. Concurrency & Race Conditions (Medium Priority)

• Review ReaderWriterLockSlim usage for TOCTOU vulnerabilities
• Audit ArchiveService.IsArchiving flag for race conditions
• Check schema migration locking

6. Dependency Supply Chain (Lower Priority)

• Audit NuGet packages for known CVEs at current pinned versions
• Review transitive dependencies

7. Installer Security (Lower Priority)

• Review DependencyInstaller.cs for privilege escalation paths
• Check SQL script execution permissions model
• Audit upgrade script ordering and injection

Use Case

A contributor (assignee) will perform the static code review using automated tooling and manual inspection, filing any findings as separate issues (or via the security reporting process in SECURITY.md for actual vulnerabilities). The goal is a clean bill of health or a prioritized remediation list.

Alternatives Considered

• External pen test engagement -- more thorough (includes runtime, network, and memory analysis) but higher cost. Could follow this initial static review if warranted.
• Automated SAST tooling only -- tools like CodeQL or Semgrep could catch common patterns but miss logic-level issues. Worth running in parallel but not sufficient alone.

Additional Context

• Assessment scope: current dev branch as of v2.7.0
• Estimated effort: 2-3 days for static code review
• No SQL Server version dependency -- this is a code-level review
• No schema changes required -- this is an audit, not an implementation
• Findings that constitute actual vulnerabilities will be reported privately per SECURITY.md policy, not posted publicly

[HannahVernon]

Security Assessment Results

Date: 2026-04-15
Scope: Static code review of dev branch (v2.7.0)
Method: Automated pattern matching + manual code analysis across all C#, SQL, and config files

Summary

Area	Rating	Critical	High	Medium	Low
Credentials & Auth	STRONG	0	0	0	1
SQL Injection	NEEDS WORK	1	3	1	0
File/Process Execution	NEEDS WORK	0	2	1	0
Local Data Security	NEEDS WORK	0	3	2	0
Concurrency	MODERATE	0	2	2	2
Installer	MODERATE	0	0	5	2
Supply Chain (NuGet)	STRONG	0	0	1	1
TOTAL		1	10	12	6

Strengths

• Credential handling is exemplary DPAPI via Windows Credential Manager, [JsonIgnore] on sensitive properties, Encrypt=Mandatory default, TrustServerCertificate=false default
• SQL Server queries properly use SqlConnectionStringBuilder, sp_executesql, and QUOTENAME()
• NuGet supply chain is clean 0 known CVEs, all packages pinned to exact versions, official nuget.org source only
• Core dismiss operations use proper DuckDB parameterization ($1, $2, etc.)

Top Findings by Priority

P0 Fix Immediately (1 finding)

ID	Finding	Location
S-1	File paths interpolated into read_parquet() SQL without escaping single quotes enables SQL injection via crafted parquet filenames	DuckDbInitializer.cs, ArchiveService.cs

P1 Fix This Sprint (6 findings)

ID	Finding	Location
R-1	ArchiveService.IsArchiving is a plain static bool not volatile, so UI thread may read stale cached value (data race)	ArchiveService.cs:37
S-3	DateTime values interpolated into DuckDB WHERE clauses instead of using parameters	ArchiveService.cs:136,164,174
S-4	Table names interpolated into information_schema queries without parameterization	DataImportService.cs:91
D-1	Teams/Slack webhook URLs stored plaintext in settings.json could enable webhook hijacking	App.xaml.cs:359,364
F-1	Process.Start with UseShellExecute=true on constructed file paths (log viewer, report viewer)	MainWindow.xaml.cs (both editions)
D-2	Raw query text stored in DuckDB tables and slow query logs may contain embedded credentials or PII	Schema.cs, QueryLogger.cs

P2 Fix This Quarter (7 findings)

ID	Finding	Location
S-2	Table/column names from ArchivableTables interpolated into SQL (safe today, fragile pattern)	ArchiveService.cs:136,164,172
R-3	Schema migrations in InitializeAsync() run without write lock	DuckDbInitializer.cs
D-4	Directory.CreateDirectory() uses default ACLs other admins can read database/logs	App.xaml.cs, AppLogger.cs
F-3	Hyperlink navigation via Process.Start with no protocol whitelist	SettingsWindow.xaml.cs
I-4	SQL Auth password passed as CLI argument (visible in process list)	Installer/Program.cs
N-1	ModelContextProtocol at pre-release 0.7.0-preview.1 stable v1.2.0 available	Dashboard.csproj, Lite.csproj
N-2	Microsoft.Data.SqlClient one major version behind (6.1.4 vs 7.0.0)	All projects

Not Covered (requires runtime analysis)

• DLL hijacking / load-order attacks
• Network-level MITM on SQL connections
• Memory scraping / credential persistence in RAM
• Parquet deserialization fuzzing

Notes

• No findings represent immediately exploitable vulnerabilities in the current threat model (single-user desktop app with trusted local storage)
• Findings are about defensive hardening as the project grows
• Any actual vulnerability discovered during remediation will be reported privately per SECURITY.md policy
• Full detailed report with remediation code samples available on request

[HannahVernon]

Security Assessment Prompts

These are the exact prompts used with GitHub Copilot CLI's sub-agent system to conduct the static code security assessment. Seven agents were launched in parallel — 6 as explore agents (read-only codebase investigation) and 1 as a task agent (needed to run dotnet CLI commands for CVE scanning).

The key to good results: give each agent specific files and classes to check, specific search patterns/keywords, and request severity ratings with file paths and line numbers. Domain knowledge of the codebase (architecture, key classes, data flow) makes the prompts far more targeted than a generic "find security issues" request.

Total wall-clock time: ~3 minutes for all 7 agents running in parallel.

---

Agent 1: Credential & Connection String Handling (explore)

You are conducting a security assessment of the PerformanceMonitor repository at C:\Dev\HannahVernon\PerformanceMonitor.

Your task: **Audit credential and connection string handling** across the entire codebase.

Specifically investigate:

1. **Credential storage**: How are SQL Server credentials stored in both Dashboard (in-memory, SQL Server-backed) and Lite (DuckDB-backed) editions? Are passwords stored in plaintext? Check ServerConnection models, AddServerDialog, and any persistence mechanisms.

2. **Connection string construction**: Search for all connection string building. Are they using SqlConnectionStringBuilder or manual concatenation? Do they set `Encrypt=true`? What about `TrustServerCertificate`?

3. **Credential leakage**: Check if credentials appear in:
   - Log output (Logger, AppLogger classes)
   - Exception messages that get displayed to users
   - Telemetry/structured logging (the key=value format)
   - DuckDB tables (could connection strings be stored in local database?)
   - Config files or settings

4. **Windows Auth vs SQL Auth**: How is Windows Authentication handled vs SQL Server authentication? Is SQL Auth discouraged?

Search patterns:
- Files: `**/*.cs`, `**/*.xaml`, `**/*.json`, `**/*.config`
- Keywords: password, Password, credential, Credential, connectionstring, ConnectionString, SqlConnection, IntegratedSecurity, Integrated Security, Encrypt, TrustServerCertificate, SecureString, ProtectedData, DPAPI

Provide a detailed findings report with file paths, line numbers, and severity ratings (Critical/High/Medium/Low/Info).

---

Agent 2: SQL Injection / Dynamic SQL (explore)

You are conducting a security assessment of the PerformanceMonitor repository at C:\Dev\HannahVernon\PerformanceMonitor.

Your task: **Audit for SQL injection and unsafe dynamic SQL** across the entire codebase.

Specifically investigate:

1. **Application-side SQL (C#)**: Search all .cs files for SQL query construction. Look for:
   - String concatenation or interpolation used to build SQL (`$"SELECT..."`, `"SELECT " + variable`, `string.Format`)
   - Proper use of parameterized queries (DuckDBCommand.Parameters, SqlCommand.Parameters)
   - Any raw user input flowing into SQL strings

2. **DuckDB queries in Lite edition**: Focus on `Lite/Services/LocalDataService*.cs` files. The dismiss logic uses batched UPDATE with VALUES lists - verify parameters are properly bound. Check `read_parquet()` calls for path injection.

3. **MCP Query Tools**: Check `Dashboard/Mcp/McpQueryTools.cs` and `Lite/Mcp/McpQueryTools.cs` - these accept external queries. How are they sanitized?

4. **SQL collection scripts**: Check `install/*.sql` for dynamic SQL construction using EXEC or sp_executesql. Are inputs properly quoted/parameterized?

5. **Dashboard DatabaseService**: Check all `Dashboard/Services/DatabaseService*.cs` files for SQL construction patterns.

Search patterns:
- `string.Format.*SELECT|INSERT|UPDATE|DELETE`
- `\$".*SELECT|INSERT|UPDATE|DELETE`
- `"SELECT.*" \+`
- `EXEC\s*\(|EXECUTE\s*\(`
- `sp_executesql`
- `cmd\.CommandText\s*=`
- `CreateCommand`

Provide a detailed findings report with file paths, line numbers, code snippets, and severity ratings (Critical/High/Medium/Low/Info).

---

Agent 3: File Path & Process Execution (explore)

You are conducting a security assessment of the PerformanceMonitor repository at C:\Dev\HannahVernon\PerformanceMonitor.

Your task: **Audit file path handling and process execution** across the entire codebase.

Specifically investigate:

1. **Process.Start calls**: Find ALL `Process.Start` call sites. Check:
   - Is `UseShellExecute` set to true? (enables shell command injection)
   - Are file paths or arguments user-controlled or derived from user input?
   - Could a malicious file path lead to arbitrary command execution?
   - Check the View Log button implementations in both MainWindow.xaml.cs files
   - Check About dialog links

2. **File path construction**: Search for:
   - Path.Combine with user-influenced components
   - String concatenation to build file paths
   - Parquet file glob patterns (could they be manipulated?)
   - DuckDB database file path construction
   - Temp file creation

3. **Path traversal**: Could any file read/write operations be tricked into accessing files outside the expected directories? Check:
   - Archive service file paths
   - Log file paths
   - Export/import functionality
   - ScriptProvider in Installer.Core

4. **File permissions**: Are created files/directories using appropriate permissions? Check DuckDB database files, parquet files, log files.

Search all .cs files for: Process.Start, ProcessStartInfo, Path.Combine, File.Create, File.Open, File.Write, File.Read, Directory.Create, StreamWriter, StreamReader, read_parquet, glob pattern.

Provide a detailed findings report with file paths, line numbers, code snippets, and severity ratings.

---

Agent 4: Concurrency & Race Conditions (explore)

You are conducting a security assessment of the PerformanceMonitor repository at C:\Dev\HannahVernon\PerformanceMonitor.

Your task: **Audit concurrency and race condition risks** across the codebase.

Specifically investigate:

1. **ReaderWriterLockSlim usage**: Find all lock acquisition in `Lite/Database/DuckDbInitializer.cs` and `Lite/Services/LocalDataService.cs`. Check:
   - Are there TOCTOU (time-of-check-time-of-use) vulnerabilities?
   - Could a reader see inconsistent state during archive operations?
   - Is AcquireWriteLock timeout handling correct?
   - LockedConnection disposal - could a failed dispose leave a lock held?

2. **ArchiveService.IsArchiving**: This is a static bool in `Lite/Services/ArchiveService.cs`. Check:
   - Is it volatile or using Interlocked operations?
   - Could it be set to false before archival actually completes (in exception paths)?
   - Race between checking IsArchiving and starting a dismiss operation

3. **Schema migration**: In `DuckDbInitializer.cs`, check:
   - What prevents two instances from running migrations simultaneously?
   - Could partial migration leave the database in an inconsistent state?

4. **Static mutable state**: Search for `static` fields/properties that are written from multiple threads. Check thread safety.

5. **Async/await patterns**: Look for fire-and-forget async calls, missing ConfigureAwait, or async void methods that could swallow exceptions.

Search all .cs files for: ReaderWriterLockSlim, lock\s*\(, volatile, Interlocked, static.*=, async void, Task.Run, .Result, .Wait(), ConfigureAwait, IsArchiving, _lock, _readLock.

Provide a detailed findings report with severity ratings.

---

Agent 5: NuGet Supply Chain (task)

Note: This agent used the task type instead of explore because it needed to run dotnet CLI commands.

You are conducting a security assessment of the PerformanceMonitor repository at C:\Dev\HannahVernon\PerformanceMonitor.

Your task: **Audit NuGet dependency supply chain** for known vulnerabilities.

1. Find all .csproj files and extract PackageReference entries with their versions.
2. List every direct NuGet dependency, its version, and which project uses it.
3. Run `dotnet list package --vulnerable` for each project to check for known CVEs.
4. Run `dotnet list package --outdated` to identify packages that may have security patches available.
5. Check if packages are pinned to exact versions or using floating versions.
6. Note any packages that seem unusual, unmaintained, or from non-standard sources.

Key projects to check:
- Dashboard/Dashboard.csproj
- Lite/PerformanceMonitorLite.csproj
- Installer/PerformanceMonitorInstaller.csproj
- Installer.Core/Installer.Core.csproj
- InstallerGui/ (find the .csproj)
- Lite.Tests/ (find the .csproj)
- Installer.Tests/ (find the .csproj)

Run the dotnet commands from C:\Dev\HannahVernon\PerformanceMonitor.

Provide a complete dependency inventory and any CVE findings with severity ratings.

---

Agent 6: Installer Security (explore)

You are conducting a security assessment of the PerformanceMonitor repository at C:\Dev\HannahVernon\PerformanceMonitor.

Your task: **Audit installer security** including privilege escalation, script injection, and execution safety.

Specifically investigate:

1. **DependencyInstaller.cs** in Installer.Core: How does it install dependencies? Does it download anything? Run with elevated privileges? Check for unsigned code execution.

2. **ScriptProvider.cs** in Installer.Core: How are SQL scripts loaded and ordered? Could a malicious script file be injected into the installation sequence? Is there path traversal risk?

3. **SQL installation scripts** in `install/`: 
   - Do they use dynamic SQL that could be injection vectors?
   - Do they create logins/users with hardcoded passwords?
   - What permissions do they grant?
   - Check for xp_cmdshell, OPENROWSET, or other dangerous features

4. **Upgrade scripts** in `upgrades/`: Same checks as install scripts. Also check ordering - could an attacker inject a script that runs first?

5. **Installer Program.cs and InstallerGui**: How do they handle user-provided server names, database names, and credentials? Command-line argument parsing security.

6. **build-all.cmd, build-dashboard.cmd, build-lite.cmd, package-release.cmd**: Check for unsafe patterns in build scripts.

Search all relevant files. Provide detailed findings with severity ratings.

---

Agent 7: Local Data Security (explore)

You are conducting a security assessment of the PerformanceMonitor repository at C:\Dev\HannahVernon\PerformanceMonitor.

Your task: **Audit local data security** in the Lite edition's DuckDB storage.

Specifically investigate:

1. **DuckDB database file**: Check `Lite/Database/DuckDbInitializer.cs` and related files:
   - Where is the .duckdb file created? What directory?
   - Are file permissions set on creation?
   - Is the database encrypted at rest?
   - Could another local user read/modify the database?

2. **Parquet archive files**: Check `Lite/Services/ArchiveService.cs`:
   - Where are parquet files stored?
   - File permissions on archives?
   - Could parquet files from an untrusted source be loaded? (parquet deserialization attacks)
   - Are old archives securely deleted?

3. **Sensitive data in DuckDB**: Check `Lite/Database/Schema.cs` and LocalDataService:
   - Are SQL Server credentials stored in any DuckDB table?
   - Are query texts stored that could contain sensitive data?
   - Is the dismissed_archive_alerts sidecar table leaking info?

4. **Settings/config persistence**: Check how Lite persists settings:
   - App.config, user settings, registry?
   - Are connection strings persisted to disk?

5. **Log files**: Check `Lite/Services/AppLogger.cs`:
   - What gets logged? Could sensitive data appear in logs?
   - Log file location and permissions?
   - Log rotation - are old logs securely handled?

Search all Lite/ files plus any shared config. Provide findings with severity ratings.

[erikdarlingdata]

Thanks for this, Hannah — really thorough work. The prompts you shared are a great template for anyone wanting to replicate this kind of review.

I've filed the top actionable findings as separate issues:

• DuckDB: Parameterize file paths in read_parquet() calls #846 — Parameterize file paths in read_parquet() calls (S-1, P0)
• DuckDB: Parameterize DateTime values in ArchiveService queries #847 — Parameterize DateTime values in ArchiveService queries (S-3, P1)
• Encrypt webhook URLs with DPAPI instead of plaintext in settings.json #848 — Encrypt webhook URLs with DPAPI (D-1, P1)
• ArchiveService.IsArchiving should be volatile or use Interlocked #849 — IsArchiving volatile/Interlocked fix (R-1, P1)

The remaining P2 items (installer ACLs, Process.Start hardening, dependency updates) are real but lower urgency for a single-user desktop app — we'll pick those up over time.

Closing this out as complete. 🎉