mirror of
https://github.com/Sea-Haven-Industries/afterhours-shift-manager.git
synced 2026-09-30 19:33:12 +00:00
bdff6bee30
* Fix auth and race-condition flaws in shift commands Four confirmed findings from the 2026-06-17 security sweep: - register_user let any Slack user overwrite an extension already bound to a different user (account takeover). Add a DynamoDB ConditionExpression so a write only succeeds when the extension is unclaimed or already this user's; raise ExtensionAlreadyRegistered otherwise and surface a clear Slack message. - The `rate` subcommand was routed without the is_admin flag, so any user could set $0 pay rates. Gate _handle_rate on is_admin, matching the admin-command guard. - `/oncall pick` used a plain put_item (TOCTOU): two concurrent picks both won. Use the atomic claim_open_shift conditional claim so the loser gets an "already picked up" message. - swap-accept overwrote a shift independently claimed after the swap was initiated. Add reassign_if_held_by, a conditional write that only applies the swap while the override is still the requester's (or on the weekly fallback), and notify the accepter otherwise. Add tests for the register-ownership guard and the rate admin guard. Refs: INFRA * Scope shift-manager Lambda IAM to least privilege The nightly sweep flagged four over-broad permissions. Scope each to only what the function actually reads (verified against source): - WeeklyPost: secrets to slack-bot-token-* only (was the whole afterhours-shift-manager/* namespace); SES SendEmail to the single noreply@seahaven.com identity (was identity/*). - RosterSync and RingScheduler: secrets to 3cx-* only (was the whole namespace); both read only the 3cx domain/client-id/client-secret. SlackBotFunction and HolidayRouter wildcards are left unchanged — out of scope for this sweep. Refs: INFRA * fix: re-validate shift holder on swap-accept (sh-security-review RIHB-1) reassign_if_held_by trusted 'no override row' as 'still the requester's', but a weekly-held shift also has no override row. An admin clear or weekly edit between swap-init and accept could move the shift to a third party with no override, letting the accept steal it (CWE-367, confirmed HIGH). Re-resolve the current holder at accept and abort if it is no longer the requester. Adds regression test + seeds the holder in existing accept tests. * fix: complete IAM least-privilege sweep (sh-security-review) HolidayRouter secrets scope afterhours-shift-manager/* -> /3cx-* (reads only 3cx secrets); RingScheduler DynamoDBCrudPolicy -> DynamoDBReadPolicy (read-only at runtime). SlackBot wildcard left as-is (reads across all sub-prefixes; verified defensible). |
||
|---|---|---|
| .. | ||
| conftest.py | ||
| test_app_home.py | ||
| test_handle_admin.py | ||
| test_handle_drop.py | ||
| test_handle_holiday.py | ||
| test_handle_holiday_actions.py | ||
| test_handle_pick.py | ||
| test_handle_register_rate.py | ||
| test_handle_swap.py | ||
| test_helpers.py | ||
| test_late_pickup.py | ||
| test_parse_date.py | ||
| test_pickup_button.py | ||
| test_swap_accept_decline.py | ||