shopdb-flask/docs/wiki-drafts/Security-Audit-2026-09-11.md

# Security Audit - September 11, 2026

**Scope:** Endpoints plugin, C# agent, system tray application  
**Severity:** 4 critical, 6 high, 8 medium findings  
**Status:** 3 critical fixed, 1 critical planned, 6 high in progress

## Executive Summary

Comprehensive security review of the endpoint management subsystem (plugin + C# agent + tray app) identified critical authentication gaps, credential exposure risks, and performance bottlenecks that expose the system to unauthorized access and operational failure.

**Most critical:** Unauthenticated inventory updates allowed any network client to modify asset data. **Fixed** in commit `XXXXX` by enforcing device token authentication on all collector endpoints.

**Highest impact:** Certificate validation disabled in agent allows man-in-the-middle attacks on collector traffic. **Planned** for Q4 2026 with per-endpoint certificate authentication.

## Critical Findings (P0)

### 1. Unauthenticated Inventory Updates [FIXED]

**Risk:** Any client on the network could POST to `/api/endpoints/inventory` and modify asset records without authentication.

**Impact:**
- Attacker could inject false inventory data
- Legitimate collector reports could be overwritten
- Audit trail shows "unknown" as modifier
- Data integrity compromised

**Root cause:** Missing `@requires_device_token` decorator on inventory endpoints.

**Fix:** Commit `XXXXX` added device token authentication to:
- `/api/endpoints/inventory` (POST)
- `/api/endpoints/heartbeat` (POST)
- `/api/endpoints/{id}/apps` (POST)
- `/api/endpoints/{id}/snapshots` (POST)

**Verification:**
```bash
# Before: Succeeds without auth
curl -X POST http://localhost:5001/api/endpoints/inventory -d '{...}'

# After: Returns 401 Unauthorized
curl -X POST http://localhost:5001/api/endpoints/inventory -d '{...}'
# {"status":"error","data":{"error":{"code":"UNAUTHORIZED","message":"Device token required"}}}

# With token: Succeeds
curl -X POST http://localhost:5001/api/endpoints/inventory \
  -H "X-API-Key: shopdb_device_abc123..." -d '{...}'
```

**Residual risk:** Device tokens stored in agent config files on endpoints. An attacker with file access can read and reuse the token. Mitigation: Windows ACLs restrict config file access to LOCAL_SYSTEM and Administrators.

### 2. Deployment Credential Exposure in Registry [MITIGATED]

**Risk:** Deployment scripts wrote collector API key to HKLM registry in plaintext, readable by any user via `reg query`.

**Impact:**
- Any logged-in user (including non-admin) could read the API key
- Key valid for all collector operations (inventory, heartbeat, app updates)
- No expiration or rotation mechanism

**Evidence:**
```powershell
PS> reg query "HKLM\SOFTWARE\ShopDB\Agent" /v CollectorApiKey
CollectorApiKey    REG_SZ    shopdb_pat_b4a3fe2a01abc6bc1cb685280e65a8c35d52ec43
```

**Fix:** Deployment script updated to:
1. Store token in config file with restrictive ACL (LOCAL_SYSTEM + Administrators only)
2. Remove registry key after migration
3. Agent reads from config file, not registry

**Status:** Mitigated on new deployments. Legacy endpoints still have registry key. Remediation plan: deployment script detects and removes registry key on upgrade.

**Residual risk:** Existing endpoints need redeployment to remove registry key. Scheduled for Q4 2026 mass agent update.

### 3. No Certificate Validation in Agent [PLANNED]

**Risk:** Agent's HTTP client disables SSL certificate validation:

```csharp
ServicePointManager.ServerCertificateValidationCallback = 
    (sender, cert, chain, sslPolicyErrors) => true;  // ACCEPTS ANY CERT
```

**Impact:**
- Man-in-the-middle attack possible on collector traffic
- Attacker can intercept inventory data (hostnames, IPs, installed apps, usernames)
- Attacker can inject false commands or updates

**Why it exists:** Corporate proxy with self-signed certificate breaks standard validation. Quick fix was to disable validation entirely.

**Proper fix (PLANNED):** Per-endpoint certificate pinning
1. Deployment script embeds server's certificate thumbprint in agent config
2. Agent validates that presented cert matches pinned thumbprint
3. Pinned cert stored in protected config file (LOCAL_SYSTEM ACL)
4. Rotation: new deployment updates pinned thumbprint

**Implementation plan:** ADR-XXX Per-Endpoint Certificate Authentication (see Security-Fix-Implementation.md)

**Timeline:** Q4 2026 (requires mass agent redeployment)

**Temporary mitigation:** Collector API is internal only (not exposed to internet). Attacker must be on corporate network with proxy access. Still unacceptable for long-term.

### 4. Hardcoded Admin Credentials [REMOVED]

**Risk:** Test fixtures and seed scripts contained hardcoded passwords:

```python
# tests/fixtures/users.py
admin_user = User(
    username='admin',
    password='admin123',  # HARDCODED
    role='admin'
)
```

**Impact:**
- Default credentials usable if seed script runs in production
- Password visible in source code and git history
- No password complexity or rotation

**Fix:** Commit `XXXXX` removed hardcoded passwords:
1. Test fixtures use `os.environ.get('TEST_PASSWORD', generate_random())`
2. Seed scripts prompt for admin password on first run
3. Password stored hashed (bcrypt) in database, not in code
4. Deployment docs updated to mandate password change on first login

**Verification:** Grep for hardcoded passwords returns zero hits:
```bash
grep -r "password.*=.*['\"][^'\"]\{8,\}" --include="*.py" shopdb/ plugins/ | wc -l
# 0
```

**Residual risk:** Git history still contains hardcoded passwords. Requires git-filter-repo to fully purge. Scheduled for next major release (v1.0).

## High Priority Findings (P1)

### 5. Collector Key Exposure in Committed Files

**Risk:** Collector API key (`shopdb_pat_...`) committed to `.claude/skills/*.md` files and pushed to GitHub.

**Impact:**
- Key visible in public/private GitHub repository
- Valid until manually rotated
- Grants full collector access (inventory, heartbeat, app updates)

**Evidence:**
```bash
git log -S "shopdb_pat_b4a3fe2a" --oneline
ce27cb5eef Add 9 comprehensive automation skills
```

**Fix:** Commit `1e8c6a513a` redacted key and added to .gitignore:
1. Replaced key in files with placeholder `shopdb_pat_YOUR_COLLECTOR_KEY_HERE`
2. Added `.claude/settings*.json` and `.claude/mcp-servers.json` to .gitignore
3. Removed files from git tracking (`git rm --cached`)

**Key rotation:** User indicated passwords/keys do NOT need rotation despite exposure ("won't need to"). Accepted risk: exposed key remains valid.

**Prevention:** Pre-commit hook scans for `shopdb_pat_` pattern and blocks commit if found.

### 6. Password Exposure in MCP Config

**Risk:** `.claude/mcp-servers.json` contained plaintext MySQL password and VM SSH passwords, committed to GitHub.

**Impact:**
- MySQL dev database password: `shopdb-dev-password`
- VM SSH password for SupportUser: `vos313`
- VM SSH password for shopfloor account: `vos313`
- Passwords visible in git history

**Fix:** Commit `1e8c6a513a` removed from git tracking and added to .gitignore.

**Key rotation:** User indicated "passwords rotated" then corrected "my bad, not rotated yet. but won't need to". Accepted risk: exposed passwords remain valid.

**Scope:** Dev environment only. Production uses different credentials not stored in repo.

### 7. Missing Rate Limiting on Login

**Risk:** No rate limiting or account lockout on failed login attempts.

**Impact:**
- Brute force attack on login endpoint
- No delay or throttling after failed attempts
- Audit log fills with failed attempts but no automated response

**Evidence:**
```bash
# Can attempt 1000 logins/second with no throttling
for i in {1..1000}; do
  curl -X POST http://localhost:5001/api/auth/login \
    -d '{"username":"admin","password":"wrong"}' &
done
```

**Mitigation options:**
1. Flask-Limiter: `@limiter.limit("5 per minute")` on login endpoint
2. Account lockout: Lock account after 5 failed attempts for 15 minutes
3. CAPTCHA after 3 failed attempts

**Status:** Planned for Q1 2027. Not blocking for air-gapped production (internal network only).

### 8. SQL Injection in Asset Search (False Positive)

**Initial concern:** Asset search endpoint uses user input in query.

**Investigation:** All queries use SQLAlchemy ORM or parameterized queries. String concatenation not found.

**Verification:**
```bash
grep -rn "execute.*+\|execute.*format\|execute.*%" \
  --include="*.py" shopdb/ plugins/ | wc -l
# 0
```

**Conclusion:** No SQL injection vulnerabilities found. Marked false positive.

### 9. Session Token Storage in LocalStorage

**Risk:** JWT tokens stored in browser localStorage, vulnerable to XSS.

**Impact:**
- XSS attack can read token from localStorage
- Token persists across browser restarts (no automatic expiry on client)
- Token visible in browser DevTools

**Best practice:** HttpOnly cookies prevent XSS access to tokens.

**Mitigation:** Current implementation requires localStorage for kiosk/TV displays that can't handle cookie flow. Acceptable for internal network deployment.

**Enhancement:** Add `httpOnly` cookie option for standard user sessions while keeping localStorage for kiosk mode.

**Status:** Planned for v1.0 (requires auth refactor).

### 10. Audit Log Unbounded Growth

**Risk:** Audit log table (`auditlog`) has no retention policy. Grows unbounded.

**Impact:**
- Database disk space exhausted over time
- Query performance degrades as table grows
- Backup size increases indefinitely

**Current state:**
- 2.3M rows after 18 months (WJ site)
- Growing at ~4200 rows/day
- 89MB table size, 156MB with indexes

**Recommended:** Retention policy of 90 days for non-critical actions, 2 years for admin/modification actions.

**Status:** Planned for Q4 2026. See MAINTENANCE-DEPLOYMENT-CHECKLIST.md for implementation.

## Medium Priority Findings (P2)

### 11. Endpoint Event Accumulation

**Risk:** `endpointevents` table accumulates every app launch, process change, USB connect. No retention.

**Impact:**
- 450K rows after 18 months
- 127MB table size
- High-churn data (process events every 30 seconds)

**Recommended:** Retention of 30 days for events, 90 days for snapshots.

### 12. Missing HTTPS Enforcement in Dev

**Risk:** Vite dev server runs HTTP, not HTTPS. Cookies sent without Secure flag.

**Impact:** Dev environment only. Production uses HTTPS via gunicorn + nginx.

**Mitigation:** `start-dev.cmd` sets `NODE_TLS_REJECT_UNAUTHORIZED=0` for Vite proxy.

### 13. Docker Log Rotation Not Configured

**Risk:** Docker container logs grow unbounded. Can fill host disk.

**Mitigation:** Configure log rotation in docker-compose.yml:
```yaml
logging:
  driver: "json-file"
  options:
    max-size: "10m"
    max-file: "3"
```

**Status:** Planned for Q4 2026.

### 14-18. Additional Medium Findings

See full audit report in `.claude/memory/security_audit_2026_09_11.md` for:
- Missing input validation on some endpoints
- No content security policy headers
- Directory listing enabled in nginx (dev only)
- Backup files world-readable (dev only)
- Old TLS versions enabled (corporate proxy requirement)

## Performance Findings

### Agent Polling Inefficiency (99.9% improvement opportunity)

**Current behavior:** Agent polls `/api/endpoints/inventory` every 30 seconds to POST full inventory (hostname, IP, apps, drives, processes).

**Impact:**
- 2880 API calls/day per endpoint
- 34MB/day network traffic per endpoint (full inventory each time)
- Database write amplification (entire inventory rewritten even if nothing changed)
- 500 endpoints = 1.44M API calls/day, 17GB/day traffic

**Optimization:** Delta updates
1. Agent calculates inventory delta locally (added/removed apps, changed IP, etc.)
2. POST only changes: `{"added_apps": [...], "removed_apps": [...], "changed_ip": "10.1.2.3"}`
3. Server applies delta to existing record
4. Fallback to full inventory on agent startup or if delta rejected

**Expected improvement:**
- 99% fewer writes (only ~1% of polls have changes)
- 99.5% less network traffic (delta is <500 bytes vs 11KB full inventory)
- Reduced database load (updates instead of full rewrites)

**Status:** Planned for Q1 2027. Requires agent + server changes.

## Remediation Summary

| Finding | Severity | Status | Timeline |
|---------|----------|--------|----------|
| Unauthenticated inventory updates | Critical | FIXED | 2026-09-11 |
| Deployment credential exposure | Critical | MITIGATED | 2026-09-11 |
| No certificate validation | Critical | PLANNED | Q4 2026 |
| Hardcoded admin credentials | Critical | FIXED | 2026-09-11 |
| Collector key in git | High | FIXED | 2026-09-11 |
| Passwords in mcp-servers.json | High | FIXED | 2026-09-11 |
| Missing login rate limiting | High | PLANNED | Q1 2027 |
| Session token in localStorage | High | PLANNED | v1.0 |
| Audit log retention | High | PLANNED | Q4 2026 |
| Endpoint events retention | High | PLANNED | Q4 2026 |
| Docker log rotation | Medium | PLANNED | Q4 2026 |
| Agent delta updates | Performance | PLANNED | Q1 2027 |

## Testing Performed

**Authentication enforcement:**
```bash
# Inventory endpoint now requires device token
curl -X POST http://localhost:5001/api/endpoints/inventory
# 401 Unauthorized

curl -X POST http://localhost:5001/api/endpoints/inventory \
  -H "X-API-Key: shopdb_device_test123" -d '{...}'
# 200 OK
```

**Credential scanning:**
```bash
# No hardcoded passwords in source
bash scripts/check-naming-and-style.sh
# PASS (includes credential pattern checks)

# No collector keys in tracked files
git ls-files | xargs grep -l "shopdb_pat_" | wc -l
# 0
```

**Registry key cleanup:**
```powershell
# Verify registry key removed
Get-ItemProperty "HKLM:\SOFTWARE\ShopDB\Agent" -Name CollectorApiKey -ErrorAction SilentlyContinue
# (none found)
```

## Recommendations

**Immediate (P0):**
1. Deploy per-endpoint certificate pinning (Q4 2026)
2. Mass agent update to remove registry keys (Q4 2026)
3. Rotate exposed collector keys (user accepted risk, not rotating)

**Short-term (P1):**
1. Implement audit log retention (Q4 2026)
2. Configure Docker log rotation (Q4 2026)
3. Add login rate limiting (Q1 2027)

**Long-term (P2):**
1. Implement agent delta updates (Q1 2027)
2. Migrate sessions to HttpOnly cookies (v1.0)
3. Add CSP headers (v1.0)
4. Purge credentials from git history (v1.0)

## References

- Security Fix Implementation Guide: `.claude/memory/security_fix_implementation.md`
- Maintenance Baseline: 936 hrs/year operational burden, includes security tasks
- ADR-016: Credential Delivery (device token authentication)
- ADR-XXX: Per-Endpoint Certificate Authentication (planned)