Develop #92

m4dm4rtig4n · 2025-12-21T01:31:18Z

No description provided.

- Remove insecure default value for SECRET_KEY - Add Pydantic validator to enforce secure configuration: - Production (DEBUG=False): Require SECRET_KEY, reject insecure patterns - Development (DEBUG=True): Generate random key with warning - Provide clear error messages with key generation command 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>

Remove trailing slashes from VITE_API_BASE_URL before concatenating paths. This fixes issues where URLs like `http://localhost:8081/` would result in `http://localhost:8081//openapi.json`. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>

nginx.conf: - Add Content-Security-Policy header - Add HSTS header (commented, enable in production) - Add Referrer-Policy and Permissions-Policy headers accounts.py: - Remove debug logging that could expose credentials - Simplify error handling in token endpoint 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>

auth.py & accounts.py: - Use secrets.compare_digest() for constant-time client_secret comparison - Prevents timing attacks that could leak secret information requests.py: - Add password complexity requirements: - At least one uppercase letter - At least one lowercase letter - At least one digit - Minimum 8 characters (existing) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>

Copilot

Pull request overview

This pull request implements security enhancements and bug fixes across frontend and backend components, with a focus on API URL normalization and authentication security.

Key Changes

Normalized API base URL handling across frontend components to prevent double slashes
Enhanced backend security with password strength validation, constant-time API key comparison, and SECRET_KEY validation
Added security headers (CSP, Referrer-Policy, Permissions-Policy) to nginx configuration

Reviewed changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 7 comments.

Show a summary per file

File	Description
package.json	Version bump to 1.4.1-dev.3
apps/web/package.json	Version bump to 1.4.1-dev.3
apps/api/pyproject.toml	Version bump to 1.4.1-dev.3
apps/web/src/pages/VerifyEmail.tsx	Added runtime environment support and trailing slash removal for API URLs
apps/web/src/pages/OAuthCallback.tsx	Applied trailing slash removal to prevent double slashes in API calls
apps/web/src/pages/ConsentRedirect.tsx	Applied trailing slash removal to prevent double slashes in API calls
apps/web/src/pages/ApiDocs.tsx	Applied trailing slash removal to prevent double slashes in API calls
apps/web/nginx.conf	Added comprehensive security headers including CSP, Referrer-Policy, Permissions-Policy, and commented HSTS
apps/api/src/schemas/requests.py	Added password strength validation requiring uppercase, lowercase, and digit characters
apps/api/src/routers/accounts.py	Removed debug logging and implemented constant-time comparison for client secret validation
apps/api/src/middleware/auth.py	Implemented constant-time comparison for API key authentication to prevent timing attacks
apps/api/src/config/settings.py	Added SECRET_KEY validation with production requirements and development fallback
CHANGELOG.md	Added version 1.4.1-dev.3 and 1.4.1-dev.2 release notes

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-12-21T01:35:06Z

apps/api/src/schemas/requests.py

+    @field_validator("password")
+    @classmethod
+    def validate_password_strength(cls, v: str) -> str:
+        """Validate password meets security requirements.
+
+        Requirements:
+        - Minimum 8 characters (enforced by Field)
+        - At least one uppercase letter
+        - At least one lowercase letter
+        - At least one digit
+        """
+        if not re.search(r"[A-Z]", v):
+            raise ValueError("Password must contain at least one uppercase letter")
+        if not re.search(r"[a-z]", v):
+            raise ValueError("Password must contain at least one lowercase letter")
+        if not re.search(r"\d", v):
+            raise ValueError("Password must contain at least one digit")
+        return v


While adding password strength validation is a security improvement, existing tests (e.g., apps/api/tests/integration/test_accounts.py) use the password "test123456" which lacks an uppercase letter and will now fail validation. This will cause existing integration tests to break. Consider either updating those test fixtures or documenting this breaking change.

Copilot · 2025-12-21T01:35:06Z

apps/api/src/schemas/requests.py

+    @field_validator("password")
+    @classmethod
+    def validate_password_strength(cls, v: str) -> str:
+        """Validate password meets security requirements.
+
+        Requirements:
+        - Minimum 8 characters (enforced by Field)
+        - At least one uppercase letter
+        - At least one lowercase letter
+        - At least one digit
+        """
+        if not re.search(r"[A-Z]", v):
+            raise ValueError("Password must contain at least one uppercase letter")
+        if not re.search(r"[a-z]", v):
+            raise ValueError("Password must contain at least one lowercase letter")
+        if not re.search(r"\d", v):
+            raise ValueError("Password must contain at least one digit")
+        return v


The password validation logic is missing one common requirement: special characters. While the current implementation requires uppercase, lowercase, and digits, many security best practices also recommend including at least one special character (e.g., !@#$%^&*). Consider adding this requirement or documenting why it was intentionally omitted.

Copilot · 2025-12-21T01:35:06Z

CHANGELOG.md

+### Bug Fixes
+
+* normalize API base URL to prevent double slashes ([1edeb1d](https://github.com/MyElectricalData/myelectricaldata_new/commit/1edeb1d0ddac287242348d542c16dc7c50f7e954))
+


The CHANGELOG for version 1.4.1-dev.3 only mentions "normalize API base URL to prevent double slashes" as a bug fix, but this PR includes several other significant changes that should be documented:

Password strength validation requirements added (uppercase, lowercase, digit)

Constant-time comparison for API key authentication

SECRET_KEY validation and security improvements

Additional security headers in nginx configuration (CSP, Referrer-Policy, Permissions-Policy)

These are important security enhancements that users and developers should be aware of, especially the password validation change which could affect existing users or integrations.

Suggested change

### Security

* **auth:** enforce stronger password requirements (must include at least one uppercase letter, one lowercase letter, and one digit)

* **auth:** use constant-time comparison for API key authentication to reduce timing-attack risk

* **core:** validate `SECRET_KEY` configuration and improve secret handling safeguards

* **nginx:** add security headers (Content-Security-Policy, Referrer-Policy, Permissions-Policy) to default configuration

Copilot · 2025-12-21T01:35:07Z

apps/api/src/config/settings.py

+        if not self.SECRET_KEY:
+            if self.DEBUG:
+                # Generate random key for development (will change on restart)
+                object.__setattr__(self, "SECRET_KEY", secrets.token_urlsafe(32))


The SECRET_KEY validation uses object.__setattr__(self, "SECRET_KEY", ...) to bypass Pydantic's immutability. While this works, it's a pattern that could be fragile if Pydantic's internals change. A more idiomatic approach would be to use model_validator with mode="before" to set the SECRET_KEY before the model is fully constructed, avoiding the need for this workaround.

Copilot · 2025-12-21T01:35:07Z

apps/api/src/middleware/auth.py

+    result = await db.execute(select(User).where(User.is_active == True))  # noqa: E712
+    users = result.scalars().all()
+    user = None
+    for u in users:
+        if secrets.compare_digest(u.client_secret, token):
+            user = u
+            break


Using constant-time comparison for API keys is a good security improvement, but the current implementation has a significant performance issue. The code now fetches ALL active users from the database and iterates through them in Python, which becomes an O(n) operation that scales poorly as the user count grows.

A better approach would be to add an index on the client_secret column (or keep the existing database query but compare using secrets.compare_digest), so you can still query by client_secret efficiently while using constant-time comparison only for the final verification. Alternatively, consider hashing client_secrets like passwords are hashed, then you can safely query by the hash without timing attack concerns.

Copilot · 2025-12-21T01:35:07Z

apps/api/src/middleware/auth.py

+    # Try API key (client_secret) - iterate to use constant-time comparison
+    result = await db.execute(select(User).where(User.is_active == True))  # noqa: E712
+    users = result.scalars().all()
+    for u in users:
+        if secrets.compare_digest(u.client_secret, token):
+            return u


Same performance issue as in the get_current_user function above. Fetching all active users and iterating through them to find a matching client_secret is inefficient and will cause performance degradation as the user base grows. Consider the same solutions: either add an index and query by client_secret with a final constant-time comparison, or hash client_secrets and query by hash.

Suggested change

# Try API key (client_secret) - iterate to use constant-time comparison

result = await db.execute(select(User).where(User.is_active == True)) # noqa: E712

users = result.scalars().all()

for u in users:

if secrets.compare_digest(u.client_secret, token):

return u

# Try API key (client_secret) - query by client_secret and use constant-time comparison

result = await db.execute(

select(User).where(

User.client_secret == token,

User.is_active == True, # noqa: E712

)

)

user = result.scalar_one_or_none()

if user and secrets.compare_digest(user.client_secret, token):

return user

Copilot · 2025-12-21T01:35:07Z

apps/web/nginx.conf

+    # Allows inline styles (Tailwind), scripts from self, and API connections
+    add_header Content-Security-Policy "default-src 'self'; script-src 'self'; style-src 'self' 'unsafe-inline'; img-src 'self' data: https://logo.clearbit.com; font-src 'self'; connect-src 'self'; frame-ancestors 'self'; base-uri 'self'; form-action 'self';" always;


The Content-Security-Policy (CSP) header only allows scripts from 'self', which may block Swagger UI from loading properly if it attempts to load scripts from CDNs or uses inline scripts. The swagger-ui-react package (version 5.29.3) may require additional CSP directives. Consider testing thoroughly or adding 'unsafe-eval' if the Swagger UI functionality breaks, though this reduces security. You may also need to allow connections to external API documentation resources if Swagger UI tries to fetch schemas from external URLs.

Suggested change

# Allows inline styles (Tailwind), scripts from self, and API connections

add_header Content-Security-Policy "default-src 'self'; script-src 'self'; style-src 'self' 'unsafe-inline'; img-src 'self' data: https://logo.clearbit.com; font-src 'self'; connect-src 'self'; frame-ancestors 'self'; base-uri 'self'; form-action 'self';" always;

# Allows inline styles (Tailwind), scripts from self (with eval for Swagger UI), and API connections (including external HTTPS APIs)

add_header Content-Security-Policy "default-src 'self'; script-src 'self' 'unsafe-eval'; style-src 'self' 'unsafe-inline'; img-src 'self' data: https://logo.clearbit.com; font-src 'self'; connect-src 'self' https:; frame-ancestors 'self'; base-uri 'self'; form-action 'self';" always;

Clément VALENTIN and others added 6 commits December 21, 2025 02:02

chore(release): 1.4.1-dev.2 [skip ci]

87016bc

chore(release): 1.4.1-dev.3 [skip ci]

bc0a569

Copilot AI review requested due to automatic review settings December 21, 2025 01:31

m4dm4rtig4n merged commit d3c4989 into main Dec 21, 2025
12 checks passed

Copilot started reviewing on behalf of m4dm4rtig4n December 21, 2025 01:31 View session

Copilot AI reviewed Dec 21, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Develop #92

Develop #92

Uh oh!

m4dm4rtig4n commented Dec 21, 2025

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Dec 21, 2025

Uh oh!

Copilot AI Dec 21, 2025

Uh oh!

Copilot AI Dec 21, 2025

Uh oh!

Copilot AI Dec 21, 2025

Uh oh!

Copilot AI Dec 21, 2025

Uh oh!

Copilot AI Dec 21, 2025

Uh oh!

Copilot AI Dec 21, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		### Bug Fixes

		* normalize API base URL to prevent double slashes ([1edeb1d](https://github.com/MyElectricalData/myelectricaldata_new/commit/1edeb1d0ddac287242348d542c16dc7c50f7e954))

+### Security
+* **auth:** enforce stronger password requirements (must include at least one uppercase letter, one lowercase letter, and one digit)
+* **auth:** use constant-time comparison for API key authentication to reduce timing-attack risk
+* **core:** validate `SECRET_KEY` configuration and improve secret handling safeguards
+* **nginx:** add security headers (Content-Security-Policy, Referrer-Policy, Permissions-Policy) to default configuration

		# Allows inline styles (Tailwind), scripts from self, and API connections
		add_header Content-Security-Policy "default-src 'self'; script-src 'self'; style-src 'self' 'unsafe-inline'; img-src 'self' data: https://logo.clearbit.com; font-src 'self'; connect-src 'self'; frame-ancestors 'self'; base-uri 'self'; form-action 'self';" always;

Develop #92

Develop #92

Uh oh!

Conversation

m4dm4rtig4n commented Dec 21, 2025

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Key Changes

Reviewed changes

Uh oh!

Copilot AI Dec 21, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 21, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 21, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 21, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 21, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 21, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 21, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants