Skip to content

Commit 96bd38e

Browse files
committed
fix: security bug fixes
Security Fixes: 1. **Fix Cookie Handling Bug ** - Fixed improper Set-Cookie header concatenation using \n - Now properly sends multiple cookies as separate headers - Affects: jsweb/response.py 2. **Add Request Size Limits (DoS Protection)** - Implemented 10MB max body size to prevent DoS attacks - Enforced during body() parsing with clear error messages - Affects: jsweb/request.py 3. **Add Security Headers Middleware** - New SecurityHeadersMiddleware with production-ready defaults - Protects against XSS, clickjacking, MIME sniffing - Headers: CSP, X-Frame-Options, HSTS, X-Content-Type-Options - Affects: jsweb/middleware.py 4. **Strengthen CSRF Protection** - Changed SameSite from 'Lax' to 'Strict' for better protection - Added detailed logging (IP, path, method) without exposing tokens - Affects: jsweb/app.py, jsweb/middleware.py 5. **Add Route Parameter Validation** - Integer parameters: max 15 digits, ±2^31-1 range - String parameters: max 1000 chars - Path parameters: max 2000 chars - Prevents overflow and DoS attacks - Affects: jsweb/routing.py Impact: - Fixes 5 critical/high severity vulnerabilities - Adds DoS protection (request size limits, param validation) - Establishes contributor workflow and quality gates - Sets up automated code quality checks
1 parent a9b1db4 commit 96bd38e

8 files changed

Lines changed: 306 additions & 19 deletions

File tree

.pre-commit-config.yaml

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
# Pre-commit hooks for JsWeb
2+
# Install: pip install pre-commit && pre-commit install
3+
# Run manually: pre-commit run --all-files
4+
5+
repos:
6+
# Standard pre-commit hooks
7+
- repo: https://github.com/pre-commit/pre-commit-hooks
8+
rev: v4.5.0
9+
hooks:
10+
- id: trailing-whitespace
11+
args: [--markdown-linebreak-ext=md]
12+
- id: end-of-file-fixer
13+
- id: check-yaml
14+
- id: check-added-large-files
15+
args: ['--maxkb=1000']
16+
- id: check-json
17+
- id: check-toml
18+
- id: check-merge-conflict
19+
- id: debug-statements
20+
- id: mixed-line-ending
21+
22+
# Black - code formatting
23+
- repo: https://github.com/psf/black
24+
rev: 24.8.0
25+
hooks:
26+
- id: black
27+
language_version: python3.11
28+
args: [--line-length=127]
29+
30+
# isort - import sorting
31+
- repo: https://github.com/PyCQA/isort
32+
rev: 5.13.2
33+
hooks:
34+
- id: isort
35+
args: [--profile=black, --line-length=127]
36+
37+
# Flake8 - linting
38+
- repo: https://github.com/PyCQA/flake8
39+
rev: 7.0.0
40+
hooks:
41+
- id: flake8
42+
args: [--max-line-length=127, --extend-ignore=E203,W503]
43+
additional_dependencies: [flake8-bugbear, flake8-comprehensions]
44+
45+
# MyPy - type checking (optional, can be slow)
46+
- repo: https://github.com/pre-commit/mirrors-mypy
47+
rev: v1.11.2
48+
hooks:
49+
- id: mypy
50+
args: [--ignore-missing-imports, --no-strict-optional]
51+
additional_dependencies: [types-all]
52+
exclude: ^(tests/|docs/)
53+
# Set to manual stage if too slow
54+
# stages: [manual]
55+
56+
# Bandit - security linting
57+
- repo: https://github.com/PyCQA/bandit
58+
rev: 1.7.9
59+
hooks:
60+
- id: bandit
61+
args: [-r, jsweb/, -f, screen]
62+
exclude: ^tests/
63+
64+
# Prettier - for YAML, JSON, Markdown formatting
65+
- repo: https://github.com/pre-commit/mirrors-prettier
66+
rev: v4.0.0-alpha.8
67+
hooks:
68+
- id: prettier
69+
types_or: [yaml, markdown, json]
70+
71+
# Configuration
72+
default_language_version:
73+
python: python3.11
74+
75+
# Performance: cache validation results
76+
default_stages: [commit, push]

CONTRIBUTING.md

Lines changed: 53 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,28 @@
1-
# Contributing to JsWeb.
1+
# Contributing to JsWeb
22

33
First off, thank you for considering contributing. It's people like you that make open-source such a great community. Any contributions you make are **greatly appreciated**.
44

5+
## Development Setup
6+
7+
To contribute to JsWeb, you'll need to set up your development environment:
8+
9+
```bash
10+
# 1. Fork and clone the repository
11+
git clone https://github.com/Jsweb-Tech/jsweb.git
12+
cd jsweb
13+
14+
# 2. Create a virtual environment
15+
python -m venv venv
16+
venv\Scripts\activate # Windows
17+
# source venv/bin/activate # macOS/Linux
18+
19+
# 3. Install JsWeb in editable mode with dev dependencies
20+
pip install -e ".[dev]"
21+
22+
# 4. Install pre-commit hooks
23+
pre-commit install
24+
```
25+
526
## How to Contribute
627

728
We use a standard workflow for contributions. If you have an improvement, please follow these steps:
@@ -15,6 +36,7 @@ We use a standard workflow for contributions. If you have an improvement, please
1536

1637
3. **Make Your Changes and Commit**
1738
* Write your code and make sure to add comments and docstrings where necessary.
39+
* Run tests and linters before committing (see below).
1840
* Commit your changes with a clear and descriptive commit message.
1941
* `git commit -m 'feat: Add some AmazingFeature'`
2042

@@ -25,6 +47,36 @@ We use a standard workflow for contributions. If you have an improvement, please
2547
5. **Open a Pull Request**
2648
* Go to the original repository and you will see a prompt to create a Pull Request from your new branch.
2749
* Provide a clear title and description for your changes, explaining what you've added or fixed.
50+
* Fill out the PR template completely.
51+
52+
## Testing & Code Quality
53+
54+
Before submitting your PR, ensure your code passes all checks:
55+
56+
```bash
57+
# Run tests
58+
pytest Tests/
59+
60+
# Run code formatters
61+
black .
62+
isort .
63+
64+
# Run linters
65+
flake8 jsweb/
66+
67+
# Or run all pre-commit checks at once
68+
pre-commit run --all-files
69+
```
70+
71+
## Branch Protection
72+
73+
The `main` branch is protected and requires:
74+
* ✅ Pull request with at least 1-2 approving reviews
75+
* ✅ All CI checks must pass (tests, linting, type checking)
76+
* ✅ Branch must be up-to-date with main
77+
* ✅ Code owner review for core changes
78+
79+
**Note:** You cannot push directly to `main`. All changes must go through pull requests.
2880

2981
## Reporting Bugs
3082

jsweb/app.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,9 @@ async def _asgi_app_handler(self, scope, receive, send):
109109
raise TypeError(f"View function did not return a Response object (got {type(response).__name__})")
110110

111111
if hasattr(req, 'new_csrf_token_generated') and req.new_csrf_token_generated:
112-
response.set_cookie("csrf_token", req.csrf_token, httponly=False, samesite='Lax')
112+
# Set CSRF token cookie with strict security settings
113+
# Note: httponly=False is required so JavaScript can read it for AJAX requests
114+
response.set_cookie("csrf_token", req.csrf_token, httponly=False, samesite='Strict', secure=False)
113115

114116
await response(scope, receive, send)
115117
return

jsweb/middleware.py

Lines changed: 75 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,14 @@ async def __call__(self, scope, receive, send):
5757
cookie_token = req.cookies.get("csrf_token")
5858

5959
if not form_token or not cookie_token or not secrets.compare_digest(form_token, cookie_token):
60-
logger.error("CSRF VALIDATION FAILED. Tokens do not match or are missing.")
60+
# Log CSRF failure with context (but never log the actual tokens)
61+
client_ip = scope.get("client", ["unknown"])[0]
62+
logger.error(
63+
f"CSRF validation failed - Method: {req.method}, "
64+
f"Path: {req.path}, Client IP: {client_ip}, "
65+
f"Form token present: {bool(form_token)}, "
66+
f"Cookie token present: {bool(cookie_token)}"
67+
)
6168
response = Forbidden("CSRF token missing or invalid.")
6269
await response(scope, receive, send)
6370
return
@@ -137,7 +144,7 @@ async def __call__(self, scope, receive, send):
137144
if scope["type"] != "http":
138145
await self.app(scope, receive, send)
139146
return
140-
147+
141148
from .database import db_session
142149
try:
143150
await self.app(scope, receive, send)
@@ -147,3 +154,69 @@ async def __call__(self, scope, receive, send):
147154
raise
148155
finally:
149156
db_session.remove()
157+
158+
159+
class SecurityHeadersMiddleware(Middleware):
160+
"""
161+
Middleware to inject security headers into all HTTP responses.
162+
163+
This middleware adds essential security headers to protect against common web
164+
vulnerabilities including XSS, clickjacking, MIME sniffing, and more.
165+
166+
Headers added:
167+
- X-Content-Type-Options: nosniff
168+
- X-Frame-Options: DENY
169+
- X-XSS-Protection: 1; mode=block
170+
- Strict-Transport-Security: max-age=31536000; includeSubDomains
171+
- Referrer-Policy: strict-origin-when-cross-origin
172+
- Content-Security-Policy: default-src 'self'
173+
174+
Args:
175+
app: The ASGI application to wrap.
176+
custom_headers (dict, optional): Custom security headers to override defaults.
177+
"""
178+
179+
DEFAULT_HEADERS = {
180+
"x-content-type-options": "nosniff",
181+
"x-frame-options": "DENY",
182+
"x-xss-protection": "1; mode=block",
183+
"strict-transport-security": "max-age=31536000; includeSubDomains",
184+
"referrer-policy": "strict-origin-when-cross-origin",
185+
# Conservative CSP - can be customized per-application
186+
"content-security-policy": "default-src 'self'",
187+
}
188+
189+
def __init__(self, app, custom_headers=None):
190+
super().__init__(app)
191+
self.headers = {**self.DEFAULT_HEADERS, **(custom_headers or {})}
192+
193+
async def __call__(self, scope, receive, send):
194+
"""
195+
Injects security headers into the HTTP response.
196+
197+
Args:
198+
scope (dict): The ASGI connection scope.
199+
receive (callable): An awaitable callable to receive events.
200+
send (callable): An awaitable callable to send events.
201+
"""
202+
if scope["type"] != "http":
203+
await self.app(scope, receive, send)
204+
return
205+
206+
async def send_wrapper(message):
207+
if message["type"] == "http.response.start":
208+
# Add security headers to response
209+
headers = list(message.get("headers", []))
210+
211+
# Only add headers if they don't already exist
212+
existing_header_names = {name.decode().lower() for name, _ in headers}
213+
214+
for header_name, header_value in self.headers.items():
215+
if header_name.lower() not in existing_header_names:
216+
headers.append([header_name.encode(), header_value.encode()])
217+
218+
message["headers"] = headers
219+
220+
await send(message)
221+
222+
await self.app(scope, receive, send_wrapper)

jsweb/request.py

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,9 @@
55

66
from werkzeug.formparser import parse_form_data
77

8+
# Maximum request body size (10MB default, configurable)
9+
MAX_REQUEST_BODY_SIZE = 10 * 1024 * 1024 # 10 MB
10+
811

912
class Request:
1013
"""
@@ -77,6 +80,7 @@ async def body(self):
7780
7881
Raises:
7982
RuntimeError: If the stream was already consumed by `stream()`.
83+
ValueError: If the request body exceeds MAX_REQUEST_BODY_SIZE.
8084
"""
8185
if self._body is None:
8286
if self._is_stream_consumed:
@@ -85,8 +89,21 @@ async def body(self):
8589
"Always use body() if you need to access the body multiple times."
8690
)
8791
chunks = []
92+
total_size = 0
93+
8894
async for chunk in self.stream():
95+
chunk_size = len(chunk)
96+
total_size += chunk_size
97+
98+
99+
if total_size > MAX_REQUEST_BODY_SIZE:
100+
raise ValueError(
101+
f"Request body size ({total_size} bytes) exceeds maximum allowed "
102+
f"size of {MAX_REQUEST_BODY_SIZE} bytes"
103+
)
104+
89105
chunks.append(chunk)
106+
90107
self._body = b"".join(chunks)
91108
return self._body
92109

jsweb/response.py

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,7 @@ def __init__(
115115
self.body = body
116116
self.status_code = status_code
117117
self.headers = headers or {}
118+
self._cookies = [] # Store cookies separately to support multiple Set-Cookie headers
118119

119120
final_content_type = content_type or self.default_content_type
120121
if "content-type" not in self.headers:
@@ -162,10 +163,8 @@ def set_cookie(
162163
if httponly:
163164
cookie_val += "; HttpOnly"
164165

165-
if "Set-Cookie" in self.headers:
166-
self.headers["Set-Cookie"] += f"\n{cookie_val}"
167-
else:
168-
self.headers["Set-Cookie"] = cookie_val
166+
# Store cookies separately to properly support multiple Set-Cookie headers
167+
self._cookies.append(cookie_val)
169168

170169
def delete_cookie(self, key: str, path: str = "/", domain: str = None):
171170
"""
@@ -194,10 +193,17 @@ async def __call__(self, scope, receive, send):
194193
if "content-length" not in self.headers:
195194
self.headers["content-length"] = str(len(body_bytes))
196195

196+
# Build headers list, properly handling multiple Set-Cookie headers
197+
headers_list = [[k.encode(), v.encode()] for k, v in self.headers.items()]
198+
199+
# Add each cookie as a separate Set-Cookie header (proper HTTP specification)
200+
for cookie in self._cookies:
201+
headers_list.append([b"set-cookie", cookie.encode()])
202+
197203
await send({
198204
"type": "http.response.start",
199205
"status": self.status_code,
200-
"headers": [[k.encode(), v.encode()] for k, v in self.headers.items()],
206+
"headers": headers_list,
201207
})
202208
await send({
203209
"type": "http.response.body",

0 commit comments

Comments
 (0)