Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 4 additions & 39 deletions .jules/sentinel.md
Original file line number Diff line number Diff line change
@@ -1,39 +1,4 @@
## 2026-02-16 - SSRF Prevention Pattern
**Vulnerability:** Server-Side Request Forgery (SSRF) in file download functionality.
**Learning:** `httpx` (and `requests`) follow redirects by default, potentially bypassing initial URL validation if a redirect leads to a private IP. Blocking DNS resolution via `socket.getaddrinfo` is effective but requires handling IPv6 scope IDs and fail-secure logic.
**Prevention:**
1. Validate URL scheme (http/https).
2. Resolve hostname to IP(s) and check against private/loopback ranges.
3. Disable automatic redirect following (`follow_redirects=False`).
4. Manually handle redirects, re-validating the URL at each hop.
5. Sanitize URLs before logging to avoid leaking sensitive tokens.

## 2026-02-16 - Zip Slip Prevention in Addon Export
**Vulnerability:** Zip Slip vulnerability in `addon_exporter.py` allowing directory traversal via malicious filenames in exported zip archives.
**Learning:** Using `os.path.join` with unsanitized user input inside `zipfile` operations can lead to files being written outside the intended directory upon extraction. Even if the path doesn't escape the zip root, `../` segments can be dangerous.
**Prevention:**
1. Sanitize all filenames used in `zipfile` operations using `os.path.basename` to strip directory components.
2. Restrict filenames to a safe allowlist (alphanumeric, `._-`).
3. Apply sanitization consistently across all file types (textures, sounds, models, etc.).

## 2026-02-16 - Rate Limiter Race Condition
**Vulnerability:** Race condition in `RateLimitMiddleware` where a shared `RateLimiter` instance's configuration was modified per-request.
**Learning:** In asynchronous frameworks like FastAPI/Starlette, middleware runs concurrently. Modifying shared state (like a singleton service's configuration) based on request attributes leads to isolation violations, where one request's settings bleed into others.
**Prevention:**
1. Never modify shared service state in middleware.
2. Pass request-specific configuration as arguments to service methods (e.g. `override_config`).
3. Use immutable configuration objects where possible or deep copy if modification is needed locally.
## 2024-05-24 - [Hardcoded JWT Secret Key]
**Vulnerability:** A hardcoded `SECRET_KEY` ("your-secret-key-change-in-production") was used in `backend/src/security/auth.py` for signing JWT tokens.
**Learning:** Hardcoded cryptographic keys allow attackers who read the source code to forge valid JWT tokens, completely bypassing authentication.
**Prevention:** Always load secrets from environment variables or a secure secret management system using a utility like `get_secret` from `core.secrets`.

## 2024-05-24 - [Fail-Secure Missing Secrets]
**Vulnerability:** A hardcoded `SECRET_KEY` was used as a default, or the application might fall back to an insecure default if the secret was not available.
**Learning:** When removing hardcoded cryptographic secrets to use environment variables, the application must fail securely if the variable is unset. Falling back to an insecure default negates the purpose of externalized secrets.
**Prevention:** Raise an explicit error (e.g., `ValueError`) during initialization if critical cryptographic secrets are missing, rather than using fallback values.

## 2024-05-24 - [CRITICAL] Remove Hardcoded Fallback for JWT SECRET_KEY
**Vulnerability:** The application used a hardcoded fallback secret key (`"dev-secret-key-do-not-use-in-production-change-me"`) for JWT token generation and validation if the `SECRET_KEY` environment variable was missing.
**Learning:** Hardcoded cryptographic secrets are a severe vulnerability (CWE-798). If the environment variable isn't set properly, the application falls back to an insecure, easily guessable default in production, compromising all tokens.
**Prevention:** Never use default fallback values for cryptographic secrets. When removing hardcoded cryptographic secrets to use environment variables, ensure the application fails securely (e.g., raises a `ValueError`) if the variable is unset, rather than falling back to an insecure default. For testing, set dummy values in `conftest.py` before application imports.
## 2025-03-01 - Mitigate user enumeration timing attack in verify_password
**Vulnerability:** User enumeration timing attack and unhandled NoneType in `verify_password`.
**Learning:** Returning early or throwing an exception when a user is not found (and hash is `None`) creates a noticeable timing difference compared to a valid login attempt. Attackers can use this to enumerate registered users.
**Prevention:** Normalize response times by executing a dummy bcrypt hash validation (`bcrypt.checkpw`) against a hardcoded dummy hash when the retrieved user hash is `None`.
Comment on lines +1 to +4
Copy link

Copilot AI Apr 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change replaces the entire Sentinel knowledge base with a single entry, deleting prior security learnings. If the intent is to record an additional finding, append a new section instead of overwriting the existing content (or move older items to an archive file if size is a concern).

Copilot uses AI. Check for mistakes.
9 changes: 9 additions & 0 deletions backend/src/security/auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,10 @@ def hash_password(password: str) -> str:
return bcrypt.hashpw(password.encode("utf-8"), bcrypt.gensalt(rounds=BCRYPT_COST)).decode("utf-8")


# Dummy hash for timing attack prevention (bcrypt hash of 'dummy_password')
# Calculated using bcrypt.hashpw(b'dummy_password', bcrypt.gensalt(rounds=12))
DUMMY_HASH = b'$2b$12$R.S.YqB461B2O.y0U3L6aOhx0zTzP9gq2i.50vR.H7V22uV0U5u9y'
Comment on lines +44 to +46
Copy link

Copilot AI Apr 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DUMMY_HASH is hardcoded with a cost factor of 12; if BCRYPT_COST changes, response times may diverge again. Add a guard to ensure the dummy hash cost matches BCRYPT_COST, or generate/refresh the dummy hash based on BCRYPT_COST so the timing normalization stays correct over time.

Suggested change
# Dummy hash for timing attack prevention (bcrypt hash of 'dummy_password')
# Calculated using bcrypt.hashpw(b'dummy_password', bcrypt.gensalt(rounds=12))
DUMMY_HASH = b'$2b$12$R.S.YqB461B2O.y0U3L6aOhx0zTzP9gq2i.50vR.H7V22uV0U5u9y'
# Dummy hash for timing attack prevention.
# Generate it from BCRYPT_COST so the dummy verification path stays aligned
# with the configured bcrypt work factor over time.
DUMMY_PASSWORD = b"dummy_password"
DUMMY_HASH = bcrypt.hashpw(DUMMY_PASSWORD, bcrypt.gensalt(rounds=BCRYPT_COST))

Copilot uses AI. Check for mistakes.

def verify_password(plain_password: str, hashed_password: str) -> bool:
"""
Verify a password against its hash.
Expand All @@ -53,6 +57,11 @@ def verify_password(plain_password: str, hashed_password: str) -> bool:
True if password matches, False otherwise
"""
try:
if hashed_password is None:
# Perform dummy check to mitigate timing attacks for user enumeration
bcrypt.checkpw(plain_password.encode("utf-8"), DUMMY_HASH)
Comment on lines +60 to +62
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚨 suggestion (security): Consider normalizing more invalid-hash cases through the dummy check to better equalize timing.

Right now only hashed_password is None uses the dummy path. Other invalid values (e.g., empty string, malformed hash) fall through to bcrypt.checkpw(..., hashed_password.encode(...)), rely on an exception, and return False without a bcrypt call, which can be measurably faster. To better equalize timing, you could extend the dummy branch to cover these invalid formats, or invoke the dummy bcrypt.checkpw in the except block before returning False.

return False
Comment on lines +60 to +63
Copy link

Copilot AI Apr 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The dummy-hash mitigation here won’t run for non-existent users if callers short-circuit before calling verify_password (e.g., backend/src/api/auth.py returns 401 when user is missing). To actually normalize timing for user enumeration, consider always calling verify_password (passing None when the user isn’t found) or moving the dummy checkpw into the login flow where the user lookup happens.

Copilot uses AI. Check for mistakes.
Comment on lines +60 to +63
Copy link

Copilot AI Apr 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

New behavior was added for hashed_password is None, but the existing unit tests for security.auth.verify_password don’t cover this branch. Please add a test asserting it returns False and still calls bcrypt.checkpw with the dummy hash when hashed_password is None.

Copilot uses AI. Check for mistakes.

return bcrypt.checkpw(plain_password.encode("utf-8"), hashed_password.encode("utf-8"))
except (ValueError, TypeError):
# Invalid hash format
Expand Down
Loading