diff --git a/apply_security_fixes.py b/apply_security_fixes.py new file mode 100644 index 0000000..2f4420c --- /dev/null +++ b/apply_security_fixes.py @@ -0,0 +1,183 @@ +import os + +def fix_l402_proxy(): + path = "src/timmy_serve/l402_proxy.py" + with open(path, "r") as f: + content = f.read() + + # 1. Add hmac_secret to Macaroon dataclass + old_dataclass = "@dataclass\nclass Macaroon:\n \"\"\"Simplified HMAC-based macaroon for L402 authentication.\"\"\"\n identifier: str # payment_hash\n signature: str # HMAC signature\n location: str = \"timmy-time\"\n version: int = 1" + new_dataclass = "@dataclass\nclass Macaroon:\n \"\"\"Simplified HMAC-based macaroon for L402 authentication.\"\"\"\n identifier: str # payment_hash\n signature: str # HMAC signature\n location: str = \"timmy-time\"\n version: int = 1\n hmac_secret: str = \"\" # Added for multi-key support" + content = content.replace(old_dataclass, new_dataclass) + + # 2. Update _MACAROON_SECRET logic + old_secret_logic = """_MACAROON_SECRET_DEFAULT = "timmy-macaroon-secret" +_MACAROON_SECRET_RAW = os.environ.get("L402_MACAROON_SECRET", _MACAROON_SECRET_DEFAULT) +_MACAROON_SECRET = _MACAROON_SECRET_RAW.encode() + +if _MACAROON_SECRET_RAW == _MACAROON_SECRET_DEFAULT: + logger.warning( + "SEC: L402_MACAROON_SECRET is using the default value — set a unique " + "secret in .env before deploying to production." + )""" + new_secret_logic = """_MACAROON_SECRET_DEFAULT = "timmy-macaroon-secret" +_MACAROON_SECRET_RAW = os.environ.get("L402_MACAROON_SECRET", _MACAROON_SECRET_DEFAULT) +_MACAROON_SECRET = _MACAROON_SECRET_RAW.encode() + +_HMAC_SECRET_DEFAULT = "timmy-hmac-secret" +_HMAC_SECRET_RAW = os.environ.get("L402_HMAC_SECRET", _HMAC_SECRET_DEFAULT) +_HMAC_SECRET = _HMAC_SECRET_RAW.encode() + +if _MACAROON_SECRET_RAW == _MACAROON_SECRET_DEFAULT or _HMAC_SECRET_RAW == _HMAC_SECRET_DEFAULT: + logger.warning( + "SEC: L402 secrets are using default values — set L402_MACAROON_SECRET " + "and L402_HMAC_SECRET in .env before deploying to production." + )""" + content = content.replace(old_secret_logic, new_secret_logic) + + # 3. Update _sign to use the two-key derivation + old_sign = """def _sign(identifier: str) -> str: + \"\"\"Create an HMAC signature for a macaroon identifier.\"\"\" + return hmac.new(_MACAROON_SECRET, identifier.encode(), hashlib.sha256).hexdigest()""" + new_sign = """def _sign(identifier: str, hmac_secret: Optional[str] = None) -> str: + \"\"\"Create an HMAC signature for a macaroon identifier using two-key derivation. + + The base macaroon secret is used to derive a key-specific secret from the + hmac_secret, which is then used to sign the identifier. This prevents + macaroon forgery if the hmac_secret is known but the base secret is not. + \"\"\" + key = hmac.new( + _MACAROON_SECRET, + (hmac_secret or _HMAC_SECRET_RAW).encode(), + hashlib.sha256 + ).digest() + return hmac.new(key, identifier.encode(), hashlib.sha256).hexdigest()""" + content = content.replace(old_sign, new_sign) + + # 4. Update create_l402_challenge + old_create = """ invoice = payment_handler.create_invoice(amount_sats, memo) + signature = _sign(invoice.payment_hash) + macaroon = Macaroon( + identifier=invoice.payment_hash, + signature=signature, + )""" + new_create = """ invoice = payment_handler.create_invoice(amount_sats, memo) + hmac_secret = _HMAC_SECRET_RAW + signature = _sign(invoice.payment_hash, hmac_secret) + macaroon = Macaroon( + identifier=invoice.payment_hash, + signature=signature, + hmac_secret=hmac_secret, + )""" + content = content.replace(old_create, new_create) + + # 5. Update Macaroon.serialize and deserialize + old_serialize = """ def serialize(self) -> str: + \"\"\"Encode the macaroon as a base64 string.\"\"\" + raw = f"{self.version}:{self.location}:{self.identifier}:{self.signature}" + return base64.urlsafe_b64encode(raw.encode()).decode()""" + new_serialize = """ def serialize(self) -> str: + \"\"\"Encode the macaroon as a base64 string.\"\"\" + raw = f"{self.version}:{self.location}:{self.identifier}:{self.signature}:{self.hmac_secret}" + return base64.urlsafe_b64encode(raw.encode()).decode()""" + content = content.replace(old_serialize, new_serialize) + + old_deserialize = """ @classmethod + def deserialize(cls, token: str) -> Optional["Macaroon"]: + \"\"\"Decode a base64 macaroon string.\"\"\" + try: + raw = base64.urlsafe_b64decode(token.encode()).decode() + parts = raw.split(":") + if len(parts) != 4: + return None + return cls( + version=int(parts[0]), + location=parts[1], + identifier=parts[2], + signature=parts[3], + ) + except Exception: + return None""" + new_deserialize = """ @classmethod + def deserialize(cls, token: str) -> Optional["Macaroon"]: + \"\"\"Decode a base64 macaroon string.\"\"\" + try: + raw = base64.urlsafe_b64decode(token.encode()).decode() + parts = raw.split(":") + if len(parts) < 4: + return None + return cls( + version=int(parts[0]), + location=parts[1], + identifier=parts[2], + signature=parts[3], + hmac_secret=parts[4] if len(parts) > 4 else "", + ) + except Exception: + return None""" + content = content.replace(old_deserialize, new_deserialize) + + # 6. Update verify_l402_token + old_verify_sig = """ # Check HMAC signature + expected_sig = _sign(macaroon.identifier) + if not hmac.compare_digest(macaroon.signature, expected_sig):""" + new_verify_sig = """ # Check HMAC signature + expected_sig = _sign(macaroon.identifier, macaroon.hmac_secret) + if not hmac.compare_digest(macaroon.signature, expected_sig):""" + content = content.replace(old_verify_sig, new_verify_sig) + + with open(path, "w") as f: + f.write(content) + +def fix_xss(): + # Fix chat_message.html + path = "src/dashboard/templates/partials/chat_message.html" + with open(path, "r") as f: + content = f.read() + content = content.replace("{{ user_message }}", "{{ user_message | e }}") + content = content.replace("{{ response }}", "{{ response | e }}") + content = content.replace("{{ error }}", "{{ error | e }}") + with open(path, "w") as f: + f.write(content) + + # Fix history.html + path = "src/dashboard/templates/partials/history.html" + with open(path, "r") as f: + content = f.read() + content = content.replace("{{ msg.content }}", "{{ msg.content | e }}") + with open(path, "w") as f: + f.write(content) + + # Fix briefing.html + path = "src/dashboard/templates/briefing.html" + with open(path, "r") as f: + content = f.read() + content = content.replace("{{ briefing.summary }}", "{{ briefing.summary | e }}") + with open(path, "w") as f: + f.write(content) + + # Fix approval_card_single.html + path = "src/dashboard/templates/partials/approval_card_single.html" + with open(path, "r") as f: + content = f.read() + content = content.replace("{{ item.title }}", "{{ item.title | e }}") + content = content.replace("{{ item.description }}", "{{ item.description | e }}") + content = content.replace("{{ item.proposed_action }}", "{{ item.proposed_action | e }}") + with open(path, "w") as f: + f.write(content) + + # Fix marketplace.html + path = "src/dashboard/templates/marketplace.html" + with open(path, "r") as f: + content = f.read() + content = content.replace("{{ agent.name }}", "{{ agent.name | e }}") + content = content.replace("{{ agent.role }}", "{{ agent.role | e }}") + content = content.replace("{{ agent.description or 'No description' }}", "{{ (agent.description or 'No description') | e }}") + content = content.replace("{{ cap.strip() }}", "{{ cap.strip() | e }}") + with open(path, "w") as f: + f.write(content) + +if __name__ == "__main__": + fix_l402_proxy() + fix_xss() + print("Security fixes applied successfully.") diff --git a/coverage.xml b/coverage.xml new file mode 100644 index 0000000..6a8d2dd --- /dev/null +++ b/coverage.xml @@ -0,0 +1,2222 @@ + + + + + + /home/ubuntu/Timmy-time-dashboard/src + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/src/dashboard/templates/briefing.html b/src/dashboard/templates/briefing.html index 711a2d9..5d98f02 100644 --- a/src/dashboard/templates/briefing.html +++ b/src/dashboard/templates/briefing.html @@ -22,7 +22,7 @@
// TIMMY’S REPORT
-
{{ briefing.summary }}
+
{{ briefing.summary | e }}
diff --git a/src/dashboard/templates/marketplace.html b/src/dashboard/templates/marketplace.html index 081dfcd..244aae6 100644 --- a/src/dashboard/templates/marketplace.html +++ b/src/dashboard/templates/marketplace.html @@ -20,13 +20,13 @@
{{ agent.name[0] }}
- {{ agent.name }} + {{ agent.name | e }} - {{ agent.role }} + {{ agent.role | e }}
-
{{ agent.description or 'No description' }}
+
{{ (agent.description or 'No description') | e }}
{{ cap.strip() }} + {{ cap.strip() | e }} {% endfor %} {% endif %}
diff --git a/src/dashboard/templates/partials/approval_card_single.html b/src/dashboard/templates/partials/approval_card_single.html index 2a30388..db0d7e3 100644 --- a/src/dashboard/templates/partials/approval_card_single.html +++ b/src/dashboard/templates/partials/approval_card_single.html @@ -1,10 +1,10 @@
-
{{ item.title }}
+
{{ item.title | e }}
{{ item.impact }}
-
{{ item.description }}
-
▶ {{ item.proposed_action }}
+
{{ item.description | e }}
+
▶ {{ item.proposed_action | e }}
{% if item.status == "pending" %}
diff --git a/src/dashboard/templates/partials/chat_message.html b/src/dashboard/templates/partials/chat_message.html index 9620da2..5d48134 100644 --- a/src/dashboard/templates/partials/chat_message.html +++ b/src/dashboard/templates/partials/chat_message.html @@ -1,15 +1,15 @@
YOU // {{ timestamp }}
-
{{ user_message }}
+
{{ user_message | e }}
{% if response %}
TIMMY // {{ timestamp }}
-
{{ response }}
+
{{ response | e }}
{% elif error %}
SYSTEM // {{ timestamp }}
-
{{ error }}
+
{{ error | e }}
{% endif %} diff --git a/src/dashboard/templates/partials/history.html b/src/dashboard/templates/partials/history.html index 2c4bbc2..26165d5 100644 --- a/src/dashboard/templates/partials/history.html +++ b/src/dashboard/templates/partials/history.html @@ -3,17 +3,17 @@ {% if msg.role == "user" %}
YOU // {{ msg.timestamp }}
-
{{ msg.content }}
+
{{ msg.content | e }}
{% elif msg.role == "agent" %}
TIMMY // {{ msg.timestamp }}
-
{{ msg.content }}
+
{{ msg.content | e }}
{% else %}
SYSTEM // {{ msg.timestamp }}
-
{{ msg.content }}
+
{{ msg.content | e }}
{% endif %} {% endfor %} diff --git a/src/timmy_serve/l402_proxy.py b/src/timmy_serve/l402_proxy.py index ba35c4e..461aa51 100644 --- a/src/timmy_serve/l402_proxy.py +++ b/src/timmy_serve/l402_proxy.py @@ -26,10 +26,14 @@ _MACAROON_SECRET_DEFAULT = "timmy-macaroon-secret" _MACAROON_SECRET_RAW = os.environ.get("L402_MACAROON_SECRET", _MACAROON_SECRET_DEFAULT) _MACAROON_SECRET = _MACAROON_SECRET_RAW.encode() -if _MACAROON_SECRET_RAW == _MACAROON_SECRET_DEFAULT: +_HMAC_SECRET_DEFAULT = "timmy-hmac-secret" +_HMAC_SECRET_RAW = os.environ.get("L402_HMAC_SECRET", _HMAC_SECRET_DEFAULT) +_HMAC_SECRET = _HMAC_SECRET_RAW.encode() + +if _MACAROON_SECRET_RAW == _MACAROON_SECRET_DEFAULT or _HMAC_SECRET_RAW == _HMAC_SECRET_DEFAULT: logger.warning( - "SEC: L402_MACAROON_SECRET is using the default value — set a unique " - "secret in .env before deploying to production." + "SEC: L402 secrets are using default values — set L402_MACAROON_SECRET " + "and L402_HMAC_SECRET in .env before deploying to production." ) @@ -40,10 +44,11 @@ class Macaroon: signature: str # HMAC signature location: str = "timmy-time" version: int = 1 + hmac_secret: str = "" # Added for multi-key support def serialize(self) -> str: """Encode the macaroon as a base64 string.""" - raw = f"{self.version}:{self.location}:{self.identifier}:{self.signature}" + raw = f"{self.version}:{self.location}:{self.identifier}:{self.signature}:{self.hmac_secret}" return base64.urlsafe_b64encode(raw.encode()).decode() @classmethod @@ -52,21 +57,32 @@ class Macaroon: try: raw = base64.urlsafe_b64decode(token.encode()).decode() parts = raw.split(":") - if len(parts) != 4: + if len(parts) < 4: return None return cls( version=int(parts[0]), location=parts[1], identifier=parts[2], signature=parts[3], + hmac_secret=parts[4] if len(parts) > 4 else "", ) except Exception: return None -def _sign(identifier: str) -> str: - """Create an HMAC signature for a macaroon identifier.""" - return hmac.new(_MACAROON_SECRET, identifier.encode(), hashlib.sha256).hexdigest() +def _sign(identifier: str, hmac_secret: Optional[str] = None) -> str: + """Create an HMAC signature for a macaroon identifier using two-key derivation. + + The base macaroon secret is used to derive a key-specific secret from the + hmac_secret, which is then used to sign the identifier. This prevents + macaroon forgery if the hmac_secret is known but the base secret is not. + """ + key = hmac.new( + _MACAROON_SECRET, + (hmac_secret or _HMAC_SECRET_RAW).encode(), + hashlib.sha256 + ).digest() + return hmac.new(key, identifier.encode(), hashlib.sha256).hexdigest() def create_l402_challenge(amount_sats: int, memo: str = "API access") -> dict: @@ -78,10 +94,12 @@ def create_l402_challenge(amount_sats: int, memo: str = "API access") -> dict: - payment_hash: for tracking payment status """ invoice = payment_handler.create_invoice(amount_sats, memo) - signature = _sign(invoice.payment_hash) + hmac_secret = _HMAC_SECRET_RAW + signature = _sign(invoice.payment_hash, hmac_secret) macaroon = Macaroon( identifier=invoice.payment_hash, signature=signature, + hmac_secret=hmac_secret, ) logger.info("L402 challenge created: %d sats — %s", amount_sats, memo) return { @@ -104,7 +122,7 @@ def verify_l402_token(token: str, preimage: Optional[str] = None) -> bool: return False # Check HMAC signature - expected_sig = _sign(macaroon.identifier) + expected_sig = _sign(macaroon.identifier, macaroon.hmac_secret) if not hmac.compare_digest(macaroon.signature, expected_sig): logger.warning("L402: signature mismatch") return False diff --git a/tests/test_security_regression.py b/tests/test_security_regression.py new file mode 100644 index 0000000..94df9b6 --- /dev/null +++ b/tests/test_security_regression.py @@ -0,0 +1,75 @@ +import hmac +import hashlib +import base64 +import pytest +from timmy_serve.l402_proxy import create_l402_challenge, verify_l402_token, Macaroon, _sign + +def test_l402_macaroon_forgery_prevention(): + """Test that knowing the hmac_secret is not enough to forge a macaroon. + + The forgery attempt uses the same hmac_secret found in a valid macaroon + but doesn't know the server's internal _MACAROON_SECRET. + """ + # 1. Create a valid challenge + challenge = create_l402_challenge(100, "valid") + valid_token = challenge["macaroon"] + + # 2. Extract components from the valid macaroon + valid_mac = Macaroon.deserialize(valid_token) + assert valid_mac is not None + + # 3. Attempt to forge a macaroon for a different (unpaid) identifier + # but using the same hmac_secret and the same signing logic a naive + # attacker might assume (if it was just hmac(hmac_secret, identifier)). + fake_identifier = "forged-payment-hash" + + # Naive forgery attempt: + fake_signature = hmac.new( + valid_mac.hmac_secret.encode(), + fake_identifier.encode(), + hashlib.sha256 + ).hexdigest() + + fake_mac = Macaroon( + identifier=fake_identifier, + signature=fake_signature, + hmac_secret=valid_mac.hmac_secret, + version=valid_mac.version, + location=valid_mac.location + ) + fake_token = fake_mac.serialize() + + # 4. Verification should fail because the server uses two-key derivation + assert verify_l402_token(fake_token) is False + +def test_xss_protection_in_templates(): + """Verify that templates now use the escape filter for user-controlled content.""" + templates_to_check = [ + ("src/dashboard/templates/partials/chat_message.html", "{{ user_message | e }}"), + ("src/dashboard/templates/partials/history.html", "{{ msg.content | e }}"), + ("src/dashboard/templates/briefing.html", "{{ briefing.summary | e }}"), + ("src/dashboard/templates/partials/approval_card_single.html", "{{ item.title | e }}"), + ("src/dashboard/templates/marketplace.html", "{{ agent.name | e }}"), + ] + + for path, expected_snippet in templates_to_check: + with open(path, "r") as f: + content = f.read() + assert expected_snippet in content, f"XSS fix missing in {path}" + +def test_macaroon_serialization_v2(): + """Test that the new serialization format includes the hmac_secret.""" + mac = Macaroon(identifier="id", signature="sig", hmac_secret="secret") + serialized = mac.serialize() + + # Decode manually to check parts + raw = base64.urlsafe_b64decode(serialized.encode()).decode() + parts = raw.split(":") + assert len(parts) == 5 + assert parts[2] == "id" + assert parts[3] == "sig" + assert parts[4] == "secret" + + # Test deserialization + restored = Macaroon.deserialize(serialized) + assert restored.hmac_secret == "secret"