Skip to content

fix(security): approval binding, chip authorship and task lifecycle follow-ups (#97 #106 #111) - #135

Merged
Birfy merged 2 commits into
ThinkFlowLab:mainfrom
Birfy:cursor/followups-97-106-111-dae8
Oct 16, 2026
Merged

Birfy merged 2 commits into
ThinkFlowLab:mainfrom
Birfy:cursor/followups-97-106-111-dae8

Conversation

@Birfy

@Birfy Birfy commented Oct 16, 2026

Copy link
Copy Markdown
Collaborator

Follow-ups for #97, #106 and #111.

#97 approval binding

  • Pinning test: a hold from a Writer to the Chief survives replacing the Chief and the owner can still decide it.
  • Docs: "demoted Chief" is eager setMain invalidation, not a staleReason cause.
  • invalidateFor throws on an unknown field.
  • Release TOCTOU: in-flight approved records are included in invalidation and release() re-checks right before dispatch().

#106 chip resolution

  • Author check on answer-question and schedule-create; schedule-create authz keys off body.target.
  • postToRoom strips dsh-session: chips from Bot-authored group posts.
  • Deferred: stripping chips on bot-to-bot own-chat followups (stays in that Bot's own chat).

#111 task lifecycle

  • Holds are per task (meta.taskId, or the Bot's only working task); rejecting one card no longer cancels all.
  • result-ready tasks are protected from pruning.
  • Authz uses an explicit task action set, not the task-* prefix.
  • Deferred: clarification state and workBotId (need [Team] Recover task handoffs without duplicate work #55 exchange wiring).

Verified: ARCH_DOCS_BASE=upstream/main npm run verify passes.

Held-send invalidation, session-chip authorship, and task records still
had the gaps ThinkFlowLab#97/ThinkFlowLab#106/ThinkFlowLab#111 named after ThinkFlowLab#52/ThinkFlowLab#53/ThinkFlowLab#54. Pin sender-side
Chief replacement, refuse unknown invalidateFor fields, and re-check
in-flight approves before dispatch. Gate chips on answer-question and
schedule-create, key schedule-create authz off target, and strip chips
from Bot group posts. Link holds per task, protect result-ready from
prune, and allow-list task endpoints. Clarification and workBotId wait
on ThinkFlowLab#55.

Co-authored-by: Danyang Chen <birfychen@gmail.com>
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 16, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-16T11:52:03.795065Z f7195c4 PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector chatgpt-codex-connector Bot 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f7195c414a

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread core/src/host/groups.js Outdated
if (typeof text !== 'string' || !text.includes('dsh-session:')) return text
return text
.replace(/@\[((?:\\.|[^\]\\])*)\]\(dsh-session:[A-Za-z0-9_-]+\)/g, (_, label) => `@${String(label).replace(/\\([\\\]])/g, '$1')}`)
.replace(/dsh-session:[A-Za-z0-9_-]+/g, '')

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Remove chip markers without allowing reconstitution

When a Bot posts a nested marker such as @[secret](dsh-sessiondsh-session:x:ImQi), the canonical-chip replacement does not match, and this one-shot replacement removes only the inner dsh-session:x, yielding the valid canonical chip @[secret](dsh-session:ImQi). postToRoom then puts that result into the group's user-shaped message, so DSH can attach session d's snapshot and bypass the shared-room privacy gate; neutralize the marker until stable (or escape it) and add a nested-marker regression.

AGENTS.md reference: AGENTS.md:L18-L18

Useful? React with 👍 / 👎.

Comment thread core/src/host/channel.js
return ok(await rt.scheduleEndpoint(endpoint, body))
case 'schedule-create': {
await rt.load()
const leaked = unshareableReference(actor, [body.title, body.prompt].filter(part => typeof part === 'string').join('\n'))

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Decode canonical chip payloads before checking ownership

When either newly guarded path receives a real menu-generated reference, session a is represented by sessionMention() as dsh-session:ImEi, but unshareableReference passes ImEi directly to sessionOwnedBy rather than base64url-decoding it to a. Consequently an owner referencing their own chat is rejected as bot/forbidden by schedule-create and answer-question; reuse the canonical decoder already present in hooks.js and test encoded chips rather than raw IDs.

AGENTS.md reference: AGENTS.md:L18-L18

Useful? React with 👍 / 👎.

…oads

- stripSessionChips loops until stable so nested dsh-session markers cannot
  reconstitute a valid chip in group posts (ThinkFlowLab#106 Codex P1).
- unshareableReference base64url-decodes menu chip payloads before
  sessionOwnedBy, so schedule-create / answer-question accept own chips
  (ThinkFlowLab#106 Codex P2).

Co-authored-by: Danyang Chen <birfychen@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants