Skip to content

fix: elevation sweep revoke payload 序列化(W2-C4 实测缺陷) - #443

Merged
randypanding merged 2 commits into
mainfrom
w2c4-sweep-payload-fix
Aug 29, 2026
Merged

fix: elevation sweep revoke payload 序列化(W2-C4 实测缺陷)#443
randypanding merged 2 commits into
mainfrom
w2c4-sweep-payload-fix

Conversation

@randypanding

@randypanding randypanding commented Aug 29, 2026

Copy link
Copy Markdown
Contributor

Card: #415

摘要(W2-C4 实测缺陷修复 #2

AC-9d TTL 收回实测(run 33247268672):sweep 到期 grant 补 revoke 事件时 payload 以 JSON 对象形态构造 → schema v1 校验(payload 须字符串或 null)exit 3,revoke 落账失败(fail-closed 红,无静默)。修复:|tojson 序列化(与 adjudicate job 同款)。

ADR

  • ADR-0103(统一证据账本 schema v1:payload 字符串纪律)

Summary by CodeRabbit

  • Bug Fixes
    • 优化权限角色识别,将成员身份正确纳入所有者角色判定。
    • 修复无法识别请求方时的角色记录问题,确保台账数据使用有效值。
    • 统一台账事件中的执行者角色信息,提升记录准确性。
    • 修复撤销事件载荷格式,确保相关事件能够正常处理。

…测缺陷)

实测(run 33246847376/33246848965)两缺陷:
- org 仓 owner 的 author_association 呈 MEMBER 而非 OWNER(一人公司语境
  MEMBER 即 owner 本人)→ 正向请求被误判 role=none 而拒;
- 裁决 role=none 直写影子账本 actor.role → 枚举校验 exit 3(合法集
  owner/agent/bot/human),deny 记录落账失败。

修复:OWNER/MEMBER→owner;none→human(事实身份)入账。

Card: #415
实测(run 33247268672):sweep 到期 grant 补 revoke 时 payload 以对象形态
构造 → evidence_shadow 枚举校验 exit 3(schema v1 要求 payload=字符串或
null)。改 |tojson(与 adjudicate job 同款)。

Card: #415
Copilot AI lite review requested due to automatic review settings August 29, 2026 10:14

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Fix elevation role mapping and revoke payload serialization

🐞 Bug fix ⚙️ Configuration changes 🕐 10-20 Minutes

Grey Divider

AI Description

• Recognize organization members as owners for elevation policy decisions.
• Record unauthorized requesters as schema-valid human actors.
• Serialize TTL revoke payloads so expired grants reach the evidence ledger.
Diagram

graph TD
  C["Issue Comment"] --> R["Role Mapping"] --> A["Policy Adjudication"] --> E["Evidence Event"] --> L[("Evidence Ledger")]
  S["Hourly Sweep"] --> V["Revoke Serialization"] --> E
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Query repository permissions
  • ➕ Distinguishes administrators from ordinary organization members
  • ➕ Remains safe if organization membership expands
  • ➖ Adds an API dependency and token permission requirements
  • ➖ Introduces latency and another fail-closed failure mode
2. Maintain an owner allowlist
  • ➕ Makes privileged identities explicit
  • ➕ Avoids treating every organization member as an owner
  • ➖ Creates a second authorization source to maintain
  • ➖ Can drift from GitHub organization membership

Recommendation: Keep the event-based OWNER/MEMBER mapping for the documented single-member organization because it is deterministic and avoids new network dependencies. Preserve the separation between the policy role and schema actor role, and revisit permission lookup or an allowlist before additional organization members are granted access.

Files changed (1) +12 / -6

Bug fix (1) +12 / -6
elevation.ymlNormalize elevation identities and serialize revoke evidence payloads +12/-6

Normalize elevation identities and serialize revoke evidence payloads

• Treats GitHub organization MEMBER associations as owner for policy adjudication while recording otherwise unauthorized requesters as schema-valid human actors. Serializes TTL-expiration revoke details with 'tojson', allowing schema-v1 validation and append-only ledger persistence to succeed.

.github/workflows/elevation.yml

@coderabbitai

coderabbitai Bot commented Aug 29, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 12f6e2c9-69c7-43c8-9007-cf366127676f

📥 Commits

Reviewing files that changed from the base of the PR and between 1a5ea09 and a5623e2.

📒 Files selected for processing (1)
  • .github/workflows/elevation.yml

Included review availability: Your plan provides up to 10 included reviews per hour; 3 remain after this review.


📝 Walkthrough

Walkthrough

变更概述

更新 elevation.yml 的角色判定和台账记录逻辑。MEMBER 映射为 owner,未知角色映射为 human。到期 revoke 事件使用 JSON 字符串作为 payload。

Changes

Elevation 工作流

Layer / File(s) Summary
角色映射与事件 payload 更新
.github/workflows/elevation.yml
角色判定同时接受 OWNERMEMBERMEMBER 映射为 owner。未知的 none 角色在台账中记录为 human。到期 revoke 事件的 payload 改为 JSON 字符串。

Suggested labels: security, bug

Merge Risk: ⚪ Minimal · up to a5623

This localized workflow change has no actionable merge-blocking risk remaining and is merge-ready after normal checks and review.

🚥 Pre-merge checks | ✅ 1 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Title check ⚠️ Warning 标题使用了合法的 fix: 前缀,且准确描述了 revoke payload 序列化修复。但标题长度为 51 个字符,超过 50 个字符的限制。 将标题缩短至不超过 50 个字符,同时保留 fix: 前缀和主要变更信息。
✅ Passed checks (1 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch w2c4-sweep-payload-fix

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot added bug Something isn't working security labels Aug 29, 2026
@randypanding
randypanding merged commit adaa816 into main Aug 29, 2026
15 of 16 checks passed
@randypanding
randypanding deleted the w2c4-sweep-payload-fix branch August 29, 2026 10:15
@qodo-code-review

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (2) 📘 Rule violations (0) 📜 Skill insights (0)

Grey Divider


Action required

1. Members gain owner authorization 🐞 Bug ⛨ Security
Description
Mapping every MEMBER comment author to ROLE=owner gives all organization members the owner-only
elevation permissions, regardless of their repository or organization privileges. GitHub defines
MEMBER as any member of the owning organization, and the adjudicator directly grants owner
capabilities from this value.
Code

.github/workflows/elevation.yml[69]

+          if [[ "$AUTHOR_ASSOC" == "OWNER" || "$AUTHOR_ASSOC" == "MEMBER" ]]; then ROLE=owner
Relevance

●●● Strong

Recent accepted security precedents support correcting authorization broadening and enforcing
owner-only privilege boundaries.

PR-#19
PR-#50

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The workflow sends its derived role directly to adjudication, and every declared capability permits
owner; the adjudicator grants whenever that role is in allowed_roles. GitHub's enum
documentation confirms that MEMBER means a member of the organization owning the repository, not
specifically its owner.

.github/workflows/elevation.yml[66-75]
.github/workflows/elevation.yml[103-105]
governance/policy/elevation.yaml[20-34]
governance/elevation.py[137-144]
🌐 GitHub defines CommentAuthorAssociation.MEMBER as an author who is a member of the organization that owns the repository, while OWNER is the repository owner.

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`author_association=MEMBER` identifies any organization member, not specifically an organization owner, so treating it as `owner` widens elevation authorization.

## Issue Context
Organization-owned repositories do not provide sufficient owner privilege information through this association alone. Resolve the requester against an explicit trusted-owner allowlist or a GitHub API check that verifies organization-owner status before passing `role=owner`; otherwise use `none`.

## Fix Focus Areas
- .github/workflows/elevation.yml[66-74]
- governance/policy/elevation.yaml[20-34]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

2. Bot requesters recorded human 🐞 Bug ◔ Observability
Description
The new none → human conversion records every unrecognized automation account as a human, because
bots other than cloudbrid-agent[bot] also receive ROLE=none. This corrupts the actor-role field
of deny events in the evidence ledger and makes machine-originated requests appear human-originated.
Code

.github/workflows/elevation.yml[74]

+          if [[ "$ROLE" == "none" ]]; then AROLE=human; else AROLE=$ROLE; fi
Relevance

●●● Strong

Recent accepted precedents favor fixing evidence-schema mismatches and preserving accurate
machine-versus-human attribution.

PR-#47
PR-#195

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
Only owners/members and the single named agent avoid ROLE=none, after which the new branch
unconditionally assigns AROLE=human. The evidence schema explicitly distinguishes bot from
human and describes identity as either a GitHub login or App slug, so other App/bot requesters are
incorrectly represented.

.github/workflows/elevation.yml[69-75]
.github/workflows/elevation.yml[111-116]
standards/evidence/record.schema.yaml[59-69]
governance/evidence_shadow.py[72-81]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Audit-role normalization currently converts all unauthorized requesters to `human`, including GitHub App and bot accounts.

## Issue Context
Keep authorization role (`owner/agent/none`) separate from evidence actor type. Derive the audit role from `github.event.comment.user.type` or an equivalent reliable bot indicator, using `bot` for automation and `human` only for users.

## Fix Focus Areas
- .github/workflows/elevation.yml[69-75]
- .github/workflows/elevation.yml[111-116]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Context sources
✅ Compliance rules (platform): 22 rules
✅ Web pages:
  +7 more
Review mode: ⚖️ Balanced: 该 PR 修改 GitHub Actions 的授权角色判定与证据账本写入逻辑,涉及权限语义和审计数据契约;虽变更集中且可理解,但并非低风险局部修复。

Grey Divider

Tip of the day
💡 Did you know, you can group findings by type and pick your Finding display, from Minimal to Full

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

# role 判定锚点机械(INV-01):OWNER/MEMBER→owner(org 仓 owner 的
# author_association 呈 MEMBER——一人公司语境 MEMBER 即 owner 本人,
# 2026-08-29 实测);cloudbrid-agent App→agent;其余→none
if [[ "$AUTHOR_ASSOC" == "OWNER" || "$AUTHOR_ASSOC" == "MEMBER" ]]; then ROLE=owner

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Action required

1. Members gain owner authorization 🐞 Bug ⛨ Security

Mapping every MEMBER comment author to ROLE=owner gives all organization members the owner-only
elevation permissions, regardless of their repository or organization privileges. GitHub defines
MEMBER as any member of the owning organization, and the adjudicator directly grants owner
capabilities from this value.
Agent Prompt
## Issue description
`author_association=MEMBER` identifies any organization member, not specifically an organization owner, so treating it as `owner` widens elevation authorization.

## Issue Context
Organization-owned repositories do not provide sufficient owner privilege information through this association alone. Resolve the requester against an explicit trusted-owner allowlist or a GitHub API check that verifies organization-owner status before passing `role=owner`; otherwise use `none`.

## Fix Focus Areas
- .github/workflows/elevation.yml[66-74]
- governance/policy/elevation.yaml[20-34]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

else ROLE=none; fi
# 影子账本 actor.role 枚举(owner/agent/bot/human):裁决 role=none 的
# 请求方按事实身份记 human(none 非法枚举——实测 exit 3)
if [[ "$ROLE" == "none" ]]; then AROLE=human; else AROLE=$ROLE; fi

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remediation recommended

2. Bot requesters recorded human 🐞 Bug ◔ Observability

The new none → human conversion records every unrecognized automation account as a human, because
bots other than cloudbrid-agent[bot] also receive ROLE=none. This corrupts the actor-role field
of deny events in the evidence ledger and makes machine-originated requests appear human-originated.
Agent Prompt
## Issue description
Audit-role normalization currently converts all unauthorized requesters to `human`, including GitHub App and bot accounts.

## Issue Context
Keep authorization role (`owner/agent/none`) separate from evidence actor type. Derive the audit role from `github.event.comment.user.type` or an equivalent reliable bot indicator, using `bot` for automation and `human` only for users.

## Fix Focus Areas
- .github/workflows/elevation.yml[69-75]
- .github/workflows/elevation.yml[111-116]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working security

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants