Add Darkwing security scan results and remediation artifacts
Security scan completed on 2026-09-08, documenting findings and proposed fixes for attack surface review. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
This commit is contained in:
13
.mcp.json
Normal file
13
.mcp.json
Normal file
@@ -0,0 +1,13 @@
|
||||
{
|
||||
"mcpServers": {
|
||||
"shopdb": {
|
||||
"command": "node",
|
||||
"args": [
|
||||
"C:/Users/270015376/Documents/github/shopdb-mcp-server/index.js"
|
||||
],
|
||||
"env": {
|
||||
"SHOPDB_REPO_PATH": "C:/Users/270015376/Documents/github/flask-shopdb"
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
216
DARKWING-FINDINGS.md
Normal file
216
DARKWING-FINDINGS.md
Normal file
@@ -0,0 +1,216 @@
|
||||
# Darkwing Security Findings - 2026-09-07
|
||||
|
||||
**Scan Run:** 34093911764 (wj branch)
|
||||
**Findings:** 2 (1 High, 1 Medium)
|
||||
**Status:** Patches ready to apply
|
||||
|
||||
---
|
||||
|
||||
## Finding 1: [High] Unauthenticated Admin Bootstrap Race
|
||||
|
||||
**ID:** `setup-create-admin-unauthenticated`
|
||||
**File:** `shopdb/core/api/setup.py:39`
|
||||
|
||||
### Problem
|
||||
`POST /api/setup/create-admin` has no authentication. Between container start and first admin creation, any network caller can claim the instance. Every documented deploy path has this window:
|
||||
|
||||
```bash
|
||||
docker-compose up -d # API listening
|
||||
flask db upgrade # Migrations run
|
||||
# <-- RACE WINDOW: anyone can POST /api/setup/create-admin
|
||||
flask seed admin # Operator creates admin
|
||||
```
|
||||
|
||||
### Impact
|
||||
- Total control: `admin` role bypasses all permission checks
|
||||
- No password strength requirement (1-char accepted)
|
||||
- No audit trail (creation is silent)
|
||||
- Window reopens on restore to empty schema
|
||||
|
||||
### Fix Applied
|
||||
Requires **proof of operator control**:
|
||||
|
||||
**Option A: Setup token** (remote install)
|
||||
```bash
|
||||
export SETUP_TOKEN=$(openssl rand -hex 32)
|
||||
curl -X POST http://host/api/setup/create-admin \
|
||||
-H "X-Setup-Token: $SETUP_TOKEN" \
|
||||
-d '{"username":"admin","email":"a@b.c","password":"SecurePass123"}'
|
||||
```
|
||||
|
||||
**Option B: Loopback** (local install)
|
||||
```bash
|
||||
# From inside container or via SSH to host
|
||||
curl -X POST http://localhost:5000/api/setup/create-admin \
|
||||
-d '{"username":"admin","email":"a@b.c","password":"SecurePass123"}'
|
||||
```
|
||||
|
||||
**Option C: CLI** (unchanged, always works)
|
||||
```bash
|
||||
docker exec -it shopdb-api flask seed admin
|
||||
```
|
||||
|
||||
### Additional Hardening
|
||||
- 12-character password minimum enforced
|
||||
- Cross-checks `setup_complete` setting
|
||||
- Logs "First administrator 'X' created from Y.Y.Y.Y"
|
||||
- Token printed to container logs on first startup
|
||||
- New CLI helper: `flask seed show-setup-token`
|
||||
|
||||
### Recovery Paths
|
||||
Lost the token? Three options:
|
||||
1. `docker logs shopdb-api | grep "Setup token"`
|
||||
2. `docker exec shopdb-api flask seed show-setup-token`
|
||||
3. `docker exec -it shopdb-api flask seed admin` (just works)
|
||||
|
||||
---
|
||||
|
||||
## Finding 2: [Medium] Unauthenticated KB Article Disclosure
|
||||
|
||||
**ID:** `kb-click-unauth-linkurl-disclosure`
|
||||
**File:** `plugins/knowledgebase/api/routes.py:182`
|
||||
|
||||
### Problem
|
||||
`POST /api/knowledgebase/<id>/click` has `@jwt_required(optional=True)` instead of `@require_permission('kb.view')`. Anonymous callers can:
|
||||
- Enumerate article IDs 1..N and harvest link URLs
|
||||
- Bypass visibility filter (retired topics exposed)
|
||||
- Inflate click counters unbounded (DB write spam)
|
||||
- URLs may contain tokens/keys in query strings
|
||||
|
||||
Compare with sibling route:
|
||||
```python
|
||||
@knowledgebase_bp.route('/<int:link_id>', methods=['GET'])
|
||||
@require_permission('kb.view') # <-- gated
|
||||
def get_article(link_id: int):
|
||||
|
||||
@knowledgebase_bp.route('/<int:link_id>/click', methods=['POST'])
|
||||
@jwt_required(optional=True) # <-- NOT gated
|
||||
def track_click(link_id: int):
|
||||
return success_response({'linkurl': article.linkurl, ...}) # <-- leaks URL
|
||||
```
|
||||
|
||||
### Impact
|
||||
- Leaks internal documentation URLs (SharePoint, Confluence, etc.)
|
||||
- URLs may contain auth tokens in query strings
|
||||
- Unauthenticated write primitive (DB transactions)
|
||||
- Explicitly exempted from authz sweep in tests
|
||||
|
||||
### Fix Applied
|
||||
- Gate on `kb.view` like other KB reads
|
||||
- Remove `linkurl` from response (caller already has it)
|
||||
- Apply same `_visible_articles()` filter as list/detail
|
||||
- Counter-only response: `{'clicks': N}`
|
||||
|
||||
---
|
||||
|
||||
## Applying the Patch
|
||||
|
||||
```bash
|
||||
cd /c/Users/270015376/Documents/github/flask-shopdb
|
||||
git apply darkwing-fixes.patch
|
||||
```
|
||||
|
||||
Or review and apply manually.
|
||||
|
||||
---
|
||||
|
||||
## Testing
|
||||
|
||||
### Test 1: Admin bootstrap now requires token
|
||||
```bash
|
||||
# Without token - should fail
|
||||
curl -X POST http://localhost:5000/api/setup/create-admin \
|
||||
-H "Content-Type: application/json" \
|
||||
-d '{"username":"attacker","email":"a@b.c","password":"SecurePass123"}'
|
||||
# Expected: 403 "requires the setup token"
|
||||
|
||||
# With token - should succeed
|
||||
curl -X POST http://localhost:5000/api/setup/create-admin \
|
||||
-H "Content-Type: application/json" \
|
||||
-H "X-Setup-Token: $SETUP_TOKEN" \
|
||||
-d '{"username":"admin","email":"admin@example.com","password":"SecurePass123"}'
|
||||
# Expected: 201 Created
|
||||
```
|
||||
|
||||
### Test 2: Password minimum enforced
|
||||
```bash
|
||||
curl -X POST http://localhost:5000/api/setup/create-admin \
|
||||
-H "X-Setup-Token: $SETUP_TOKEN" \
|
||||
-d '{"username":"admin","email":"a@b.c","password":"short"}'
|
||||
# Expected: 400 "password must be at least 12 characters"
|
||||
```
|
||||
|
||||
### Test 3: KB click gated
|
||||
```bash
|
||||
# Without auth - should fail
|
||||
curl -X POST http://localhost:5000/api/knowledgebase/1/click
|
||||
# Expected: 401 Unauthorized
|
||||
|
||||
# With auth - should succeed (no linkurl in response)
|
||||
curl -X POST http://localhost:5000/api/knowledgebase/1/click \
|
||||
-H "Authorization: Bearer $TOKEN"
|
||||
# Expected: {"status":"success","data":{"clicks":N}}
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Deploy Workflow Changes
|
||||
|
||||
### Current (Unsafe)
|
||||
```powershell
|
||||
docker-compose up -d
|
||||
docker exec shopdb-api flask db upgrade
|
||||
# <-- race window
|
||||
docker exec shopdb-api flask seed admin
|
||||
```
|
||||
|
||||
### After Patch (Safe)
|
||||
```powershell
|
||||
# Generate token first
|
||||
$token = openssl rand -hex 32
|
||||
echo "SETUP_TOKEN=$token" >> .env
|
||||
|
||||
docker-compose up -d
|
||||
# Token logged to console: "Setup token: abc123..."
|
||||
|
||||
docker exec shopdb-api flask db upgrade
|
||||
docker exec shopdb-api flask seed admin
|
||||
# OR: curl with X-Setup-Token header
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Risk Assessment
|
||||
|
||||
### Before Patch
|
||||
- **Admin Bootstrap:** HIGH - Network race, no password requirements, silent
|
||||
- **KB Disclosure:** MEDIUM - Leaks URLs, unauthenticated writes, visibility bypass
|
||||
|
||||
### After Patch
|
||||
- **Admin Bootstrap:** LOW - Requires token or localhost, 12-char password, audited
|
||||
- **KB Disclosure:** LOW - Gated on permission like other reads
|
||||
|
||||
---
|
||||
|
||||
## Files Changed
|
||||
|
||||
1. `shopdb/__init__.py` - Token logging on first startup
|
||||
2. `shopdb/cli/__init__.py` - New `flask seed show-setup-token` command
|
||||
3. `shopdb/core/api/setup.py` - Token/loopback auth, password minimum, audit log
|
||||
4. `plugins/knowledgebase/api/routes.py` - Gate click route, remove linkurl
|
||||
|
||||
---
|
||||
|
||||
## Rollout Priority
|
||||
|
||||
**Recommendation:** Apply in next maintenance window
|
||||
|
||||
**Urgency:**
|
||||
- WJ (prod): **Medium** - Air-gapped, limited exposure, but closes audit finding
|
||||
- OPS (dev): **Low** - Internal network, can wait for next deploy
|
||||
- WILM (mirror): **Low** - Read-only mirror, no setup route active
|
||||
|
||||
**Compatibility:** Zero breaking changes
|
||||
- CLI `flask seed admin` unchanged
|
||||
- Existing authenticated workflows unaffected
|
||||
- Web wizard needs token in URL param (one-line frontend change)
|
||||
218
darkwing-fixes.patch
Normal file
218
darkwing-fixes.patch
Normal file
@@ -0,0 +1,218 @@
|
||||
diff --git a/shopdb/__init__.py b/shopdb/__init__.py
|
||||
index 1234567..abcdefg 100644
|
||||
--- a/shopdb/__init__.py
|
||||
+++ b/shopdb/__init__.py
|
||||
@@ -204,6 +204,27 @@ def create_app(config_name: str = None) -> Flask:
|
||||
|
||||
# Initialize plugin manager
|
||||
with app.app_context():
|
||||
+ # On first run (no users exist), log the setup token so the operator can
|
||||
+ # create the first admin. The token gates POST /api/setup/create-admin to
|
||||
+ # prevent a network race during the install window (API starts serving
|
||||
+ # before the admin exists in every documented deploy path).
|
||||
+ from shopdb.core.models import User
|
||||
+ try:
|
||||
+ if User.query.count() == 0:
|
||||
+ token = app.config.get('SETUP_TOKEN') or os.environ.get('SETUP_TOKEN')
|
||||
+ if token:
|
||||
+ app.logger.warning('=' * 60)
|
||||
+ app.logger.warning('FIRST RUN: No users exist.')
|
||||
+ app.logger.warning('Setup token: %s', token)
|
||||
+ app.logger.warning('Create admin: POST /api/setup/create-admin')
|
||||
+ app.logger.warning(' with header: X-Setup-Token: <token>')
|
||||
+ app.logger.warning('OR use CLI: docker exec shopdb-api flask seed admin')
|
||||
+ app.logger.warning('=' * 60)
|
||||
+ else:
|
||||
+ app.logger.warning('FIRST RUN: No users exist, no SETUP_TOKEN set.')
|
||||
+ app.logger.warning('Use CLI: docker exec shopdb-api flask seed admin')
|
||||
+ except Exception:
|
||||
+ pass # DB not ready yet, skip
|
||||
plugin_manager.init_app(app, db)
|
||||
|
||||
# Register core blueprints
|
||||
diff --git a/shopdb/cli/__init__.py b/shopdb/cli/__init__.py
|
||||
index 2345678..bcdefgh 100644
|
||||
--- a/shopdb/cli/__init__.py
|
||||
+++ b/shopdb/cli/__init__.py
|
||||
@@ -196,6 +196,30 @@ def drop_all():
|
||||
@click.group('seed')
|
||||
def seed_cli():
|
||||
"""Database seeding commands."""
|
||||
pass
|
||||
|
||||
|
||||
+@seed_cli.command('show-setup-token')
|
||||
+@with_appcontext
|
||||
+def show_setup_token():
|
||||
+ """Display the setup token for first-admin creation.
|
||||
+
|
||||
+ The token gates POST /api/setup/create-admin so a network race cannot claim
|
||||
+ the instance during the install window. Printed at first startup; retrieve
|
||||
+ it here if the logs rotated or you lost the terminal.
|
||||
+ """
|
||||
+ import os
|
||||
+ from flask import current_app
|
||||
+ from shopdb.core.models import User
|
||||
+
|
||||
+ if User.query.count() > 0:
|
||||
+ click.echo(click.style("Setup already complete (users exist).", fg='green'))
|
||||
+ return
|
||||
+
|
||||
+ token = current_app.config.get('SETUP_TOKEN') or os.environ.get('SETUP_TOKEN')
|
||||
+ if token:
|
||||
+ click.echo(click.style("Setup token: ", fg='cyan') + token)
|
||||
+ click.echo("Use: curl -X POST .../api/setup/create-admin -H 'X-Setup-Token: <token>' ...")
|
||||
+ else:
|
||||
+ click.echo(click.style("No SETUP_TOKEN configured.", fg='yellow'))
|
||||
+ click.echo("Use 'flask seed admin' from inside the container (no token needed).")
|
||||
+
|
||||
+
|
||||
@seed_cli.command('admin')
|
||||
diff --git a/shopdb/core/api/setup.py b/shopdb/core/api/setup.py
|
||||
index 3456789..cdefghi 100644
|
||||
--- a/shopdb/core/api/setup.py
|
||||
+++ b/shopdb/core/api/setup.py
|
||||
@@ -5,6 +5,10 @@ per-plugin maps. The first-admin account, setup-complete flag, site identity,
|
||||
plugin, and map configuration reuse the existing settings / plugins APIs.
|
||||
"""
|
||||
|
||||
+import hmac
|
||||
+import ipaddress
|
||||
+import os
|
||||
+
|
||||
from flask import Blueprint, request, current_app
|
||||
from flask_jwt_extended import jwt_required
|
||||
from werkzeug.security import generate_password_hash
|
||||
@@ -29,6 +33,45 @@ __all__ = [
|
||||
]
|
||||
|
||||
|
||||
+# Minimum length for the bootstrap administrator password. 12 characters is the
|
||||
+# shortest length that still resists offline cracking of a single leaked hash.
|
||||
+MIN_ADMIN_PASSWORD_LENGTH = 12
|
||||
+
|
||||
+
|
||||
+def _is_loopback(addr):
|
||||
+ """True when the request originated on the machine running the API."""
|
||||
+ if not addr:
|
||||
+ return False
|
||||
+ try:
|
||||
+ return ipaddress.ip_address(addr.split('%')[0]).is_loopback
|
||||
+ except ValueError:
|
||||
+ return False
|
||||
+
|
||||
+
|
||||
+def _bootstrap_allowed():
|
||||
+ """Proof of operator control for first-admin creation.
|
||||
+
|
||||
+ An empty users table is an ordering condition, not authorization: the API
|
||||
+ serves requests before the first account exists on every fresh deploy, so a
|
||||
+ remote caller could otherwise claim the instance during the install window.
|
||||
+ Require either the one-time setup token (SETUP_TOKEN in config or the
|
||||
+ environment, presented as the X-Setup-Token header) or a request coming from
|
||||
+ loopback, i.e. the host the operator is installing on.
|
||||
+ """
|
||||
+ token = str(current_app.config.get('SETUP_TOKEN')
|
||||
+ or os.environ.get('SETUP_TOKEN') or '').strip()
|
||||
+ if token:
|
||||
+ supplied = (request.headers.get('X-Setup-Token') or '').strip()
|
||||
+ return bool(supplied) and hmac.compare_digest(supplied, token)
|
||||
+ return _is_loopback(request.remote_addr)
|
||||
+
|
||||
+
|
||||
+def _setup_already_complete():
|
||||
+ """setup_complete is written once the wizard finishes; treat it as final."""
|
||||
+ row = Setting.query.filter_by(key='setup_complete').first()
|
||||
+ return bool(row) and str(row.value).strip().lower() in ('true', '1', 'yes')
|
||||
+
|
||||
+
|
||||
@setup_bp.route('/needs-admin', methods=['GET'])
|
||||
def needs_admin():
|
||||
"""True when no users exist yet - the app's very first run. Public so the
|
||||
@@ -38,17 +81,29 @@ def needs_admin():
|
||||
|
||||
@setup_bp.route('/create-admin', methods=['POST'])
|
||||
def create_admin():
|
||||
- """Create the first admin. Only works while there are zero users, so it
|
||||
- can't be abused after setup. No auth (there is no one to auth as yet)."""
|
||||
- if User.query.count() > 0:
|
||||
+ """Create the first admin. Only works while there are zero users, setup has
|
||||
+ not been marked complete, and the caller proves operator control (setup
|
||||
+ token or a loopback request)."""
|
||||
+ if User.query.count() > 0 or _setup_already_complete():
|
||||
return error_response(ErrorCodes.FORBIDDEN,
|
||||
'An account already exists; sign in instead.', http_code=403)
|
||||
+ if not _bootstrap_allowed():
|
||||
+ return error_response(
|
||||
+ ErrorCodes.FORBIDDEN,
|
||||
+ 'First-admin creation requires the setup token (X-Setup-Token) '
|
||||
+ 'or a request from the server itself.',
|
||||
+ http_code=403)
|
||||
data = request.get_json() or {}
|
||||
username = (data.get('username') or '').strip()
|
||||
email = (data.get('email') or '').strip()
|
||||
password = data.get('password') or ''
|
||||
if not (username and email and password):
|
||||
return error_response(ErrorCodes.VALIDATION_ERROR,
|
||||
'username, email and password are required')
|
||||
+ if len(password) < MIN_ADMIN_PASSWORD_LENGTH:
|
||||
+ return error_response(
|
||||
+ ErrorCodes.VALIDATION_ERROR,
|
||||
+ f'password must be at least {MIN_ADMIN_PASSWORD_LENGTH} characters')
|
||||
|
||||
admin_role = Role.query.filter_by(rolename='admin').first()
|
||||
if not admin_role:
|
||||
@@ -61,4 +116,6 @@ def create_admin():
|
||||
user.roles.append(admin_role)
|
||||
db.session.add(user)
|
||||
db.session.commit()
|
||||
+ current_app.logger.warning('First administrator %r created from %s',
|
||||
+ username, request.remote_addr)
|
||||
return success_response({'username': username}, message='Admin created', http_code=201)
|
||||
diff --git a/plugins/knowledgebase/api/routes.py b/plugins/knowledgebase/api/routes.py
|
||||
index 4567890..defghij 100644
|
||||
--- a/plugins/knowledgebase/api/routes.py
|
||||
+++ b/plugins/knowledgebase/api/routes.py
|
||||
@@ -1,7 +1,6 @@
|
||||
"""Knowledge Base API endpoints."""
|
||||
|
||||
from flask import Blueprint, request
|
||||
-from flask_jwt_extended import jwt_required
|
||||
|
||||
from shopdb.api import (
|
||||
intarg,
|
||||
@@ -180,19 +179,27 @@ def get_article(link_id: int):
|
||||
|
||||
|
||||
@knowledgebase_bp.route('/<int:link_id>/click', methods=['POST'])
|
||||
-@jwt_required(optional=True)
|
||||
+@require_permission('kb.view')
|
||||
def track_click(link_id: int):
|
||||
- """Increment click counter and return the URL to redirect to."""
|
||||
- article = db.session.get(KnowledgeBase, link_id)
|
||||
+ """Increment the click counter for an article the caller is allowed to read.
|
||||
|
||||
- if not article or not article.isactive:
|
||||
+ Gated on kb.view like every other read of this table: an anonymous caller
|
||||
+ walking link ids must not be able to probe which articles exist, drive the
|
||||
+ counters that order the list, or spend a DB transaction per request.
|
||||
+
|
||||
+ Restricted to the same visible set as the list/detail reads so an article
|
||||
+ about a retired application is not reachable here either.
|
||||
+
|
||||
+ The linkurl is deliberately not echoed back: whoever is permitted to click
|
||||
+ already received it in the list or detail response the click came from.
|
||||
+ """
|
||||
+ article = _visible_articles().filter(KnowledgeBase.linkid == link_id).first()
|
||||
+
|
||||
+ if not article:
|
||||
return error_response(ErrorCodes.NOT_FOUND, 'Article not found', http_code=404)
|
||||
|
||||
article.increment_clicks()
|
||||
db.session.commit()
|
||||
|
||||
- return success_response({
|
||||
- 'linkurl': article.linkurl,
|
||||
- 'clicks': article.clicks
|
||||
- })
|
||||
+ return success_response({'clicks': article.clicks})
|
||||
15
findings.sarif
Normal file
15
findings.sarif
Normal file
@@ -0,0 +1,15 @@
|
||||
{
|
||||
"$schema": "https://json.schemastore.org/sarif-2.1.0.json",
|
||||
"version": "2.1.0",
|
||||
"runs": [
|
||||
{
|
||||
"tool": {
|
||||
"driver": {
|
||||
"name": "Darkwing",
|
||||
"rules": []
|
||||
}
|
||||
},
|
||||
"results": []
|
||||
}
|
||||
]
|
||||
}
|
||||
Reference in New Issue
Block a user