Skip to content

sec(ri-exchange): auto mode and nil-creator rows bypass 4-eyes; tighten #673 tests #679

Closed

Description

@cristim

Summary

Gaps left by PR #673 (direct-execute refusal under 4-eyes; closes #517), found in its independent review (deployment-investigation-kimi, review at 2aa8f1d). #517 covers the two direct-execute endpoints only.

Current behaviour

A. Auto mode runs unattended. With RIExchangeEnabled and RIExchangeMode == "auto", the scheduler runs exchange.RunAutoExchange (internal/server/handler_ri_exchange.go) and executes exchanges with no approval; RequireDifferentApprover is never consulted. Manual mode only creates pending rows, which go through the approve path.
B. Nil-creator rows are approvable under 4-eyes (pre-existing, #233). requireDifferentRIExchangeApprover (internal/api/handler_ri_exchange.go) returns nil when CreatedByUserID is nil. Automated pending rows therefore can be approved by any approve-any holder, or by an emailed token with no session. Purchases deny NULL creators in this mode (requireDifferentApprover, handler_purchases.go); exchanges deliberately do not.
C. Test gaps in #673. The Azure global-config lookup failure (fail-closed) is untested; the AWS tests do not spy on exchange.ExecuteExchange and rely on 403 vs the no-credentials 500.

Owner questions

  1. Should enabling auto mode require 4-eyes to be off (or refuse/downgrade auto to manual while RequireDifferentApprover is on), or is auto an explicit admin opt-in that stays exempt?
  2. Should nil-creator exchange rows be approvable under 4-eyes, or denied like purchases (admin disables the mode to approve)?

Expected behaviour

Per the answers: enforce or document each exemption; add the Azure fail-closed test and an explicit "no exchange call" assertion on the AWS path.

References

PR #673, issues #517, #233. Severity: medium (A, needs owner decision), low (B, C).

Activity

  1. changed the title [-]sec(ri-exchange): auto mode executes unattended while 4-eyes is on; tighten #673 tests[/-] [+]sec(ri-exchange): auto mode and nil-creator rows bypass 4-eyes; tighten #673 tests[/+] on Oct 8, 2026
  2. IAmJSD commented on Oct 19, 2026

    @IAmJSD
    Collaborator

    Claiming: working on this

  3. self-assigned this
    on Oct 19, 2026
  4. IAmJSD commented on Oct 25, 2026

    @IAmJSD
    Collaborator

    Owner decision (IAmJSD, acting owner while the maintainers are away, 2026-10-25), for question 1: while 4-eyes is on, unattended RI exchanges (the scheduled reshape and the ladder) run as manual. A human approver must approve them, and auto mode is not exempt. #1108 implements this and fails closed if the config is missing or unreadable. Part B (exchanges with no creator) stays as it is today; #989 covers it. Follow-up: settings should show the mode that actually runs while 4-eyes is on (it shows auto today).

  5. added a commit that references this issue on Oct 25, 2026
    bdfe866
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions