fix(harness): atomic claim for auto-approval and rejected proposals stay rejected
보안 리뷰 반영. 자동 승인은 approved_at·executed_at·rejected_at 이 비어 있고 만료 전인 행만 조건부 UPDATE 로 선점한 경우에만 실행하므로, 사람의 승인·실행·거절이 먼저 오면 이중 실행이 없다. proposals 에 rejected_at 열을 추가해 거절 경로가 기록하고, approve/authorize/자동 승인이 거절된 제안을 거부한다.
This commit is contained in:
1 parent
ca933a646e
commit
85d99324a1
5 files changed
+34
-8
No files matched your search
@@ -127,6 +127,8 @@ func (a *App) authorize(ctx context.Context, req ActionRequest) (string, error)
|
||||
return "", errors.New("invalid token")
|
||||
}
|
||||
switch {
|
||||
case p.RejectedAt != nil:
|
||||
return "", errors.New("proposal rejected")
|
||||
case p.ApprovedAt == nil:
|
||||
return "", errors.New("proposal not approved")
|
||||
case p.ExecutedAt != nil:
|
||||
|
||||
@@ -9,16 +9,20 @@ import (
|
||||
// after the configured delay the harness approves the layer-2 proposal (actor "auto") and executes it exactly as the
|
||||
// HITL bot does after a button press (actor "ai-approved", MarkProposalExecuted). MTTA therefore equals the delay;
|
||||
// the verdict logic is unchanged (layer 2 from the ai-approved action). A proposal already decided, expired or
|
||||
// recorded-only is handled the same way as the API path.
|
||||
// recorded-only is handled the same way as the API path. The claim (approve + executed_at) is a single conditional
|
||||
// UPDATE, so a human approval/execution or a rejection that arrives first takes precedence (no double execution).
|
||||
func (a *App) autoApprove(token string, req ProposalRequest, after time.Duration) {
|
||||
time.Sleep(after)
|
||||
ctx, cancel := context.WithTimeout(context.Background(), 10*time.Minute)
|
||||
defer cancel()
|
||||
p, err := a.store.GetProposal(ctx, token)
|
||||
if err != nil || p.ApprovedAt != nil || time.Now().After(p.ExpiresAt) {
|
||||
if err != nil {
|
||||
return
|
||||
}
|
||||
if err := a.store.ApproveProposal(ctx, token, "auto"); err != nil {
|
||||
// one conditional update claims approval + execution: a human/bot decision (approve, reject, execute) that lands
|
||||
// first wins and this goroutine does nothing
|
||||
claimed, err := a.store.ClaimProposal(ctx, token, "auto")
|
||||
if err != nil || !claimed {
|
||||
return
|
||||
}
|
||||
suffix := token[len(token)-6:]
|
||||
@@ -31,7 +35,6 @@ func (a *App) autoApprove(token string, req ProposalRequest, after time.Duration
|
||||
if !ce.Exec {
|
||||
a.store.Event(ctx, p.RunID, p.ScenarioID, "action", "ai-approved", map[string]any{"action_id": p.ActionID, "target": req.Target, "recorded_only": true, "note": note, "layer": ce.Layer})
|
||||
mActs.WithLabelValues(p.ActionID, "ai-approved", "recorded").Inc()
|
||||
_ = a.store.MarkProposalExecuted(ctx, token)
|
||||
return
|
||||
}
|
||||
res, err := a.executeAction(ctx, p.ActionID, req.Target, "ai-approved", p.RunID, p.ScenarioID)
|
||||
@@ -41,5 +44,4 @@ func (a *App) autoApprove(token string, req ProposalRequest, after time.Duration
|
||||
}
|
||||
mActs.WithLabelValues(p.ActionID, "ai-approved", result).Inc()
|
||||
a.store.Event(ctx, p.RunID, p.ScenarioID, "action", "ai-approved", map[string]any{"action_id": p.ActionID, "target": req.Target, "result": res, "error": errStr(err), "note": note})
|
||||
_ = a.store.MarkProposalExecuted(ctx, token)
|
||||
}
|
||||
Binary file not shown.
@@ -94,6 +94,9 @@ func (a *App) handleProposalApprove(w http.ResponseWriter, r *http.Request) {
|
||||
return
|
||||
}
|
||||
switch {
|
||||
case p.RejectedAt != nil:
|
||||
writeErr(w, 409, errors.New("proposal rejected"))
|
||||
return
|
||||
case p.ApprovedAt != nil:
|
||||
writeErr(w, 409, errors.New("proposal already approved"))
|
||||
return
|
||||
@@ -122,6 +125,7 @@ func (a *App) handleProposalReject(w http.ResponseWriter, r *http.Request) {
|
||||
writeErr(w, 404, errors.New("unknown proposal"))
|
||||
return
|
||||
}
|
||||
_ = a.store.RejectProposal(r.Context(), token) // blocks a later approval (button, API or auto-approval)
|
||||
a.store.Event(r.Context(), p.RunID, p.ScenarioID, "note", req.RejectedBy, map[string]any{"rejected": true, "action_id": p.ActionID, "reason": req.Reason, "token_suffix": token[len(token)-6:]})
|
||||
writeJSON(w, 200, map[string]any{"token": token, "rejected": true})
|
||||
}
|
||||
+21
-3
@@ -26,6 +26,7 @@ CREATE TABLE IF NOT EXISTS runs (
|
||||
);
|
||||
ALTER TABLE runs ADD COLUMN IF NOT EXISTS model text; -- LLM requested for the run's diagnoses (campaign-level model selection, 2026-10-14)
|
||||
ALTER TABLE runs ADD COLUMN IF NOT EXISTS auto_approve_after_s int; -- measurement mode: the harness approves + executes layer-2 proposals after this delay (2026-10-15)
|
||||
ALTER TABLE proposals ADD COLUMN IF NOT EXISTS rejected_at timestamptz; -- POST /proposals/{token}/reject (a rejected proposal can no longer be approved or executed)
|
||||
CREATE TABLE IF NOT EXISTS scenarios (
|
||||
scenario_id text PRIMARY KEY,
|
||||
path text NOT NULL,
|
||||
@@ -287,13 +288,14 @@ type ProposalRow struct {
|
||||
ApprovedAt *time.Time
|
||||
ApprovedBy *string
|
||||
ExecutedAt *time.Time
|
||||
RejectedAt *time.Time
|
||||
ExpiresAt time.Time
|
||||
}
|
||||
|
||||
func (s *Store) GetProposal(ctx context.Context, token string) (*ProposalRow, error) {
|
||||
var p ProposalRow
|
||||
err := s.pool.QueryRow(ctx, `SELECT token,run_id,scenario_id,action_id,approved_at,approved_by,executed_at,expires_at FROM proposals WHERE token=$1`, token).
|
||||
Scan(&p.Token, &p.RunID, &p.ScenarioID, &p.ActionID, &p.ApprovedAt, &p.ApprovedBy, &p.ExecutedAt, &p.ExpiresAt)
|
||||
err := s.pool.QueryRow(ctx, `SELECT token,run_id,scenario_id,action_id,approved_at,approved_by,executed_at,rejected_at,expires_at FROM proposals WHERE token=$1`, token).
|
||||
Scan(&p.Token, &p.RunID, &p.ScenarioID, &p.ActionID, &p.ApprovedAt, &p.ApprovedBy, &p.ExecutedAt, &p.RejectedAt, &p.ExpiresAt)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
@@ -301,7 +303,23 @@ func (s *Store) GetProposal(ctx context.Context, token string) (*ProposalRow, er
|
||||
}
|
||||
|
||||
func (s *Store) ApproveProposal(ctx context.Context, token, by string) error {
|
||||
_, err := s.pool.Exec(ctx, `UPDATE proposals SET approved_at=now(), approved_by=$2 WHERE token=$1 AND approved_at IS NULL`, token, by)
|
||||
_, err := s.pool.Exec(ctx, `UPDATE proposals SET approved_at=now(), approved_by=$2 WHERE token=$1 AND approved_at IS NULL AND rejected_at IS NULL`, token, by)
|
||||
return err
|
||||
}
|
||||
|
||||
// ClaimProposal approves and marks executed in one conditional update, so exactly one caller (auto-approval or a
|
||||
// human/bot execute racing with it) wins: it reports whether this call claimed the proposal.
|
||||
func (s *Store) ClaimProposal(ctx context.Context, token, by string) (bool, error) {
|
||||
tag, err := s.pool.Exec(ctx, `UPDATE proposals SET approved_at=now(), approved_by=$2, executed_at=now()
|
||||
WHERE token=$1 AND approved_at IS NULL AND executed_at IS NULL AND rejected_at IS NULL AND expires_at > now()`, token, by)
|
||||
if err != nil {
|
||||
return false, err
|
||||
}
|
||||
return tag.RowsAffected() == 1, nil
|
||||
}
|
||||
|
||||
func (s *Store) RejectProposal(ctx context.Context, token string) error {
|
||||
_, err := s.pool.Exec(ctx, `UPDATE proposals SET rejected_at=now() WHERE token=$1 AND approved_at IS NULL AND rejected_at IS NULL`, token)
|
||||
return err
|
||||
}
|
||||
|
||||
|
||||
Reference in new issue
Block a user