From a67f57c37eb9774a922c3f0cf1854866d12a2442 Mon Sep 17 00:00:00 2001 From: Markus Graf Date: Mon, 22 Jun 2026 10:04:48 +0200 Subject: [PATCH] refactor: remove command field from GraphQL Alarm type command is generated internally and has no informational value to API consumers, so stop exposing it on getAlarms/getAlarm/setAlarm. --- README.md | 7 +++---- api/schema.py | 17 +++++++++++++---- tests/test_api.py | 20 ++++++++++++-------- 3 files changed, 28 insertions(+), 16 deletions(-) diff --git a/README.md b/README.md index 06cc2ff..42eecde 100644 --- a/README.md +++ b/README.md @@ -82,7 +82,6 @@ query { getAlarms { id cronExpression - command isEnabled } } @@ -92,16 +91,16 @@ query { ```graphql mutation { setAlarm( - cronExpression: "45 6 * * 1-5", + cronExpression: "45 6 * * 1-5", isEnabled: true ) { id cronExpression - command + isEnabled } } ``` -*(Note: The `command` is managed automatically by the API and cannot be overridden, to prevent command injection into the system crontab.)* +*(Note: `command` is generated automatically by the API and is **not** accepted as an input parameter on `setAlarm`, to prevent command injection into the system crontab.)* **Delete an alarm:** ```graphql diff --git a/api/schema.py b/api/schema.py index b5b5948..08c7925 100644 --- a/api/schema.py +++ b/api/schema.py @@ -65,7 +65,6 @@ def _start_wecker_process() -> subprocess.Popen: class Alarm: id: str cron_expression: str - command: str is_enabled: bool @@ -78,7 +77,14 @@ class Query: @strawberry.field def get_alarms(self) -> List[Alarm]: manager = get_manager() - return [Alarm(**a) for a in manager.get_alarms()] + return [ + Alarm( + id=a["id"], + cron_expression=a["cron_expression"], + is_enabled=a["is_enabled"], + ) + for a in manager.get_alarms() + ] @strawberry.field def get_alarm(self, id: str) -> Optional[Alarm]: @@ -86,7 +92,11 @@ class Query: alarms = manager.get_alarms() for a in alarms: if a["id"] == id: - return Alarm(**a) + return Alarm( + id=a["id"], + cron_expression=a["cron_expression"], + is_enabled=a["is_enabled"], + ) return None @@ -111,7 +121,6 @@ class Mutation: return Alarm( id=new_id, cron_expression=cron_expression, - command=command, is_enabled=is_enabled, ) diff --git a/tests/test_api.py b/tests/test_api.py index 87985b1..8499ebf 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -102,13 +102,16 @@ def test_bugfix_default_command_uses_append_for_logs(): instead of overwriting (>) it. """ from api.schema import Mutation + from api.crontab_manager import CrontabManager mutation = Mutation() # Call the resolver directly without command to trigger default command generation alarm = mutation.set_alarm(cron_expression="0 9 * * *") - # Verify the generated command string - command = alarm.command + manager = CrontabManager(tabfile=os.environ["TABFILE"]) + alarms = manager.get_alarms() + assert len(alarms) == 1 + command = alarms[0]["command"] assert ">> wecker.log 2>&1" in command, ( f"Command must use append '>>' syntax. Got: {command}" ) @@ -126,21 +129,24 @@ def test_set_alarm_default_command_append(): mutation { setAlarm(cronExpression: "0 9 * * *") { id - command } } """ res = client.post("/graphql", json={"query": mutation}, headers=headers) assert res.status_code == 200 - data = res.json()["data"]["setAlarm"] + alarm_id = res.json()["data"]["setAlarm"]["id"] + + from api.crontab_manager import CrontabManager + manager = CrontabManager(tabfile=os.environ["TABFILE"]) + alarms = manager.get_alarms() + assert len(alarms) == 1 + command = alarms[0]["command"] # Assert the command contains the correct append syntax (>>) and not just overwrite (>) - command = data["command"] assert ">> wecker.log 2>&1" in command assert "> wecker.log 2>&1" not in command.replace(">> wecker.log", "REPLACED") # Cleanup so we don't break subsequent tests - alarm_id = data["id"] mutation_delete = f""" mutation {{ deleteAlarm(id: "{alarm_id}") @@ -202,7 +208,6 @@ def test_graphql_workflow(): setAlarm(cronExpression: "30 7 * * *", isEnabled: true) { id cronExpression - command isEnabled } } @@ -211,7 +216,6 @@ def test_graphql_workflow(): assert res.status_code == 200 alarm = res.json()["data"]["setAlarm"] assert alarm["cronExpression"] == "30 7 * * *" - assert "wecker.py" in alarm["command"] assert alarm["isEnabled"] is True alarm_id = alarm["id"]