Commit graph

2 commits

Author SHA1 Message Date
Adam Moussa
bdff6bee30
INFRA-106: nightly-sweep security remediation (auth/race/IAM) (#125)
Some checks failed
Deploy / deploy (push) Has been cancelled
Deploy / release (push) Has been cancelled
* 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).
2026-06-18 12:05:25 -04:00
Adam Moussa
060bd0bf3e
Add swap-acceptance (verified-swap) flow (#87)
Some checks are pending
Deploy / deploy (push) Waiting to run
/oncall swap no longer reassigns immediately. It now writes a pending SWAP
record and DMs the target Accept/Decline buttons; the shift only moves once
they accept.

- schedule.py: create_pending_swap / get_swap / mark_swap_verified /
  clear_swap (PK=SWAP, date/shift SK mirroring OVERRIDE, status + timestamps
  + expires_at for TTL). A new request supersedes a prior pending one.
- app.py: _handle_swap creates the pending swap + DMs the target (requires the
  target be Slack-linked; rejects self-swap). New module-level
  handle_swap_accept / handle_swap_decline + two @app.action registrations.
  Accept writes the override, repoints 3CX when it's the active shift, marks
  the swap verified, notifies the channel + requester. Decline clears it and
  DMs the requester. Lazy expiry: accept is rejected once the shift has started
  (_shift_start/_shift_started).
- blocks.py: build_swap_request_blocks (Accept/Decline) + build_swap_resolved_blocks.
- template.yaml: enable DynamoDB TTL on expires_at so abandoned pending swaps
  self-clean.
- tests: swap schedule methods, swap blocks, rewritten test_handle_swap
  (pending + DM, no immediate override), new test_swap_accept_decline. 174 passed.
- README: swap behavior + SWAP item type + TTL.

The verified SWAP status is what #84 (24h drop guard) will query.

Closes #83
2026-06-01 19:22:55 -04:00