diff --git a/app/system/impl/raspiblitz.py b/app/system/impl/raspiblitz.py index 54f61f5..7e477bd 100644 --- a/app/system/impl/raspiblitz.py +++ b/app/system/impl/raspiblitz.py @@ -261,11 +261,11 @@ class RaspiBlitzSystem(SystemBase): async def change_password(self, type: str, old_password: str, new_password: str): # check just allowed type values - type = type.lower() - if type not in ["a", "b", "c"]: + if not type or type.lower() not in ["a", "b", "c"]: raise HTTPException( status.HTTP_400_BAD_REQUEST, detail=f"unknown password type: {type}" ) + type = type.lower() # check password formatting if not password_valid(old_password): @@ -279,7 +279,8 @@ class RaspiBlitzSystem(SystemBase): # first check if old password is correct result = await exec_bash_command( - f'/home/admin/config.scripts/blitz.passwords.sh check {type} "{old_password}"' # noqa: E501 + f'/home/admin/config.scripts/blitz.passwords.sh check {type} "{old_password}"', # noqa: E501 + sensitive=True, ) data = {} match result: @@ -300,7 +301,7 @@ class RaspiBlitzSystem(SystemBase): if type == "c": # will set password c of both lnd & core lightning if installed/activated script_call = f'/home/admin/config.scripts/blitz.passwords.sh set c "{old_password}" "{new_password}"' # noqa: E501 - result = await exec_bash_command(script_call) + result = await exec_bash_command(script_call, sensitive=True) data = {} match result: case Ok(in_data): diff --git a/app/system/models.py b/app/system/models.py index 2f4a9b1..dc812fc 100644 --- a/app/system/models.py +++ b/app/system/models.py @@ -16,6 +16,13 @@ class LoginInput(BaseModel): ] = None +class ChangePasswordInput(BaseModel): + old_password: constr(min_length=1) + new_password: constr(min_length=1) + # RaspiBlitz only: which password (a, b or c) to change + type: Optional[str] = None + + class APIPlatform(str, Enum): RASPIBLITZ = "raspiblitz" NATIVE_PYTHON = "native_python" diff --git a/app/system/router.py b/app/system/router.py index 6c0943c..fd75c75 100644 --- a/app/system/router.py +++ b/app/system/router.py @@ -1,5 +1,3 @@ -from typing import Optional - from fastapi import APIRouter, HTTPException, Request, Response, status from fastapi.params import Depends, Query @@ -14,6 +12,7 @@ from app.system.docs import ( get_hw_info_json, ) from app.system.models import ( + ChangePasswordInput, ConnectionInfo, LoginInput, RawDebugLogData, @@ -74,18 +73,10 @@ def refresh_token(): response_description="if 200 OK - password change worked", dependencies=[Depends(JWTBearer())], ) -async def change_password_impl( - old_password: str, - new_password: str, - type: Optional[str] = Query( - None, - description=( - "ℹ️ Used in **RaspiBlitz only**. Password A, B or C. " - 'Must be one of `["a", "b", "c"]`' - ), - ), -): - return await change_password(type, old_password, new_password) +async def change_password_impl(data: ChangePasswordInput): + return await change_password( + data.type, data.old_password, data.new_password + ) @router.get( diff --git a/tests/test_change_password.py b/tests/test_change_password.py new file mode 100644 index 0000000..00b57a6 --- /dev/null +++ b/tests/test_change_password.py @@ -0,0 +1,50 @@ +""" +Regression tests for the change-password endpoint. + +- Passwords were accepted as query parameters, so they leaked into access + logs, proxy logs and browser history. They must travel in the request body. +- The RaspiBlitz password scripts were called without sensitive=True, so the + plaintext passwords were written to the debug log. +""" + +from app.api.models import ProcessResult +from app.external.result_type.src.result.result import Ok + + +def test_change_password_uses_request_body_not_query(): + from app.main import app + + spec = app.openapi() + path = next(p for p in spec["paths"] if p.endswith("change-password")) + operation = spec["paths"][path]["post"] + + param_names = {p["name"] for p in operation.get("parameters", [])} + assert "old_password" not in param_names, "password must not be a query param" + assert "new_password" not in param_names, "password must not be a query param" + assert "requestBody" in operation, "passwords must be sent in the request body" + + +async def test_raspiblitz_change_password_calls_are_sensitive(monkeypatch): + from app.system.impl import raspiblitz as rb + + # the shell-script existence check would exit(1) in the test env + monkeypatch.setattr( + rb.RaspiBlitzSystem, "_check_shell_scripts_status", lambda self: None + ) + system = rb.RaspiBlitzSystem() + calls = [] + + async def fake_exec(command, **kwargs): + calls.append(kwargs) + # 'check' -> correct=1; 'set' -> no error key + return Ok(ProcessResult(0, "correct=1\n", "")) + + monkeypatch.setattr(rb, "exec_bash_command", fake_exec) + monkeypatch.setattr(rb, "password_valid", lambda p: True) + + await system.change_password("a", "oldpass12", "newpass12") + + assert len(calls) == 2, "expected a check call and a set call" + assert all(c.get("sensitive") is True for c in calls), ( + "password script calls must be marked sensitive so they aren't logged" + )