Skip to content

Commit ffc19c5

Browse files
author
Sqoia Dev Agent
committed
fix: data integrity, security, ledger locking, role enforcement
Data Integrity: - Atomic ledger writes with fcntl.flock + rename (no more corruption) - Rolling backups (3 generations) before every ledger overwrite - Invoice state machine validation (reject invalid transitions server-side) - Ledger corruption detection with user-facing error message - Fixed useBillingData abort race condition - Fixed allocation period scoping (filter to start_date/end_date) Security: - Moved financial integration config (API keys, webhooks) to separate financial_config.json with 0640 permission guidance - Default role changed from 'admin' to 'member' — no admin UI flash - Added loading spinner during role resolution - Documented security model in README (Cockpit auth, role enforcement, parameterized SQL, billing rules safety) Ledger Operations: - All ledger mutations now go through ledger_util.py (centralized locking) - State machine: draft→sent→viewed→paid, paid→refunded, any→cancelled - No more client-side read-modify-write race conditions
1 parent 2800d18 commit ffc19c5

6 files changed

Lines changed: 371 additions & 61 deletions

File tree

README.md

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -203,6 +203,36 @@ npx eslint src/ test/
203203

204204
See [CONTRIBUTING.md](CONTRIBUTING.md).
205205

206+
## Security Model
207+
208+
SlurmLedger delegates authentication entirely to Cockpit. Users log in through the Cockpit web interface; the plugin receives the authenticated session and never manages passwords or session tokens itself.
209+
210+
**Role enforcement is UI-level only.** The role system (admin, finance, PI, member) controls which tabs and actions are visible in the browser. The backend Python scripts (`slurmdb.py`, `invoice.py`, `financial_export.py`) run with the OS permissions of the authenticated Cockpit user — they do not enforce roles independently. On most deployments the plugin is used by HPC administrators, so this is acceptable. If you need hard backend enforcement, wrap the scripts with sudo rules.
211+
212+
**API keys and webhook secrets** should be stored in `/etc/slurmledger/financial_config.json` with restricted permissions:
213+
214+
```bash
215+
sudo chown root:cockpit-ws /etc/slurmledger/financial_config.json
216+
sudo chmod 0640 /etc/slurmledger/financial_config.json
217+
```
218+
219+
This file is separate from `institution.json` (which holds non-sensitive configuration) so that access controls can be applied without restricting the rest of the profile.
220+
221+
**SQL injection prevention.** All queries in `slurmdb.py` use parameterized statements (`%s` placeholders with PyMySQL). Identifier values (host, user, database, cluster) are validated against a strict allowlist regex (`^[A-Za-z0-9_]+$`) before use. Date parameters are validated as YYYY-MM-DD before conversion to UNIX timestamps.
222+
223+
**Billing rules engine.** The rules engine in `slurmdb.py` evaluates conditions using a closed set of comparison operators (`=`, `!=`, `<`, `>`, `contains`). User-supplied rule data is never passed to `eval()` or `exec()`. Adding a billing rule through the UI cannot execute arbitrary code.
224+
225+
**Invoice ledger integrity.** The ledger (`/etc/slurmledger/invoices.json`) is written atomically via `ledger_util.py`: a temp file is written with an exclusive `flock`, then renamed into place. Rolling backups (`.bak`, `.bak.1`, `.bak.2`) are maintained automatically. If the ledger is corrupted, the UI displays an error pointing to the backup files.
226+
227+
**Recommended file permissions summary:**
228+
229+
| File | Owner | Mode | Notes |
230+
|---|---|---|---|
231+
| `/etc/slurmledger/institution.json` | root | 0644 | Non-sensitive institution profile |
232+
| `/etc/slurmledger/financial_config.json` | root:cockpit-ws | 0640 | API keys, webhook URL |
233+
| `/etc/slurmledger/invoices.json` | root:cockpit-ws | 0640 | Invoice ledger |
234+
| `/etc/slurmledger/rates.json` | root | 0644 | Billing rates |
235+
206236
## License
207237

208238
LGPL-2.1 — See [LICENSE](LICENSE).

src/financial_config.json

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
{
2+
"enabled": false,
3+
"type": "none",
4+
"webhookUrl": "",
5+
"apiKey": "",
6+
"chartOfAccounts": {}
7+
}

src/institution.json

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -24,13 +24,6 @@
2424
"departmentName": "",
2525
"costCenter": "",
2626
"notes": "",
27-
"logo": "",
28-
"financialIntegration": {
29-
"enabled": false,
30-
"type": "none",
31-
"webhookUrl": "",
32-
"apiKey": "",
33-
"chartOfAccounts": {}
34-
}
27+
"logo": ""
3528
}
3629

src/ledger_util.py

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,97 @@
1+
#!/usr/bin/env python3
2+
"""Atomic read/modify/write for the invoice ledger with file locking."""
3+
import fcntl
4+
import json
5+
import os
6+
import shutil
7+
import sys
8+
9+
LEDGER_PATH = '/etc/slurmledger/invoices.json'
10+
BACKUP_COUNT = 3
11+
12+
def read_ledger():
13+
try:
14+
with open(LEDGER_PATH, 'r') as f:
15+
return json.load(f)
16+
except (FileNotFoundError, json.JSONDecodeError):
17+
return {"invoices": []}
18+
19+
def write_ledger(data):
20+
"""Atomic write with backup and file locking."""
21+
# Create backup
22+
if os.path.exists(LEDGER_PATH):
23+
for i in range(BACKUP_COUNT - 1, 0, -1):
24+
src = f"{LEDGER_PATH}.bak.{i-1}" if i > 1 else f"{LEDGER_PATH}.bak"
25+
dst = f"{LEDGER_PATH}.bak.{i}"
26+
if os.path.exists(src):
27+
shutil.copy2(src, dst)
28+
shutil.copy2(LEDGER_PATH, f"{LEDGER_PATH}.bak")
29+
30+
# Atomic write with lock
31+
tmp_path = LEDGER_PATH + '.tmp'
32+
with open(tmp_path, 'w') as f:
33+
fcntl.flock(f.fileno(), fcntl.LOCK_EX)
34+
json.dump(data, f, indent=2)
35+
f.flush()
36+
os.fsync(f.fileno())
37+
fcntl.flock(f.fileno(), fcntl.LOCK_UN)
38+
os.rename(tmp_path, LEDGER_PATH) # atomic on same filesystem
39+
40+
def update_invoice(invoice_id, patch):
41+
"""Update a single invoice by ID with locking."""
42+
data = read_ledger()
43+
for inv in data.get('invoices', []):
44+
if inv.get('id') == invoice_id:
45+
# State machine validation
46+
VALID_TRANSITIONS = {
47+
'draft': ['sent', 'cancelled'],
48+
'sent': ['viewed', 'paid', 'cancelled'],
49+
'viewed': ['paid', 'cancelled'],
50+
'paid': ['refunded'],
51+
'cancelled': [],
52+
'refunded': [],
53+
}
54+
if 'status' in patch:
55+
current = inv.get('status', 'draft')
56+
new_status = patch['status']
57+
if new_status not in VALID_TRANSITIONS.get(current, []):
58+
print(json.dumps({"error": "InvalidTransition",
59+
"message": f"Cannot change status from '{current}' to '{new_status}'"}))
60+
sys.exit(1)
61+
inv.update(patch)
62+
break
63+
else:
64+
print(json.dumps({"error": "NotFound", "message": f"Invoice {invoice_id} not found"}))
65+
sys.exit(1)
66+
write_ledger(data)
67+
print(json.dumps({"ok": True}))
68+
69+
if __name__ == '__main__':
70+
import argparse
71+
parser = argparse.ArgumentParser()
72+
parser.add_argument('--action', choices=['read', 'add', 'update', 'add-refund'])
73+
parser.add_argument('--invoice-id', default='')
74+
args = parser.parse_args()
75+
76+
if args.action == 'read':
77+
print(json.dumps(read_ledger()))
78+
elif args.action == 'update':
79+
patch = json.load(sys.stdin)
80+
update_invoice(args.invoice_id, patch)
81+
elif args.action == 'add':
82+
invoice = json.load(sys.stdin)
83+
data = read_ledger()
84+
data['invoices'].append(invoice)
85+
write_ledger(data)
86+
print(json.dumps({"ok": True}))
87+
elif args.action == 'add-refund':
88+
refund = json.load(sys.stdin)
89+
data = read_ledger()
90+
for inv in data.get('invoices', []):
91+
if inv.get('id') == args.invoice_id:
92+
inv.setdefault('refunds', []).append(refund)
93+
if refund.get('full'):
94+
inv['status'] = 'refunded'
95+
break
96+
write_ledger(data)
97+
print(json.dumps({"ok": True}))

0 commit comments

Comments
 (0)