Skip to content

docs(adr): define actor-attributed data security boundary - #8158

Merged
waynesun09 merged 10 commits into
mainfrom
codex/adr-0133-actor-data-security
Oct 13, 2026
Merged

waynesun09 merged 10 commits into
mainfrom
codex/adr-0133-actor-data-security

Conversation

@ifireball

@ifireball ifireball commented Oct 7, 2026 •

Copy link
Copy Markdown
Member

Summary

  • Add ADR 0133 defining a data-security boundary for actor-attributed content from events, polling, snapshots, and proactive API reads.
  • Allow content from authorized actors to guide task execution within the configured harness scope without granting platform authority.
  • Require provenance checks, Unicode/control-character handling, sensitive-data redaction, prompt-injection scanning, size limits, and fail-closed behavior.
  • Define actor-permission cache lifetime by logical event/entity read and agent invocation boundaries; re-fetches and re-invocations require fresh resolution.
  • Re-resolve current actor authorization after event or retrieval admission and again immediately before sandbox initialization; cached evidence cannot cross either checkpoint.
  • Re-check immediately before first model exposure and immediately before releasing each mid-run controlled-retrieval result.
  • Admit only records with reliable source-system record/actor attribution and applicable current authorization; treat each authorized comment as one trusted text unit, including quoted or copied text.
  • Bind authorization to the exact current content revision; redact records whose source system cannot establish revision attribution.
  • Distinguish source-system record attribution from trusted authorization providers, including configured OWNERS-derived permissions and registered-bot roles.
  • Update the living architecture and security threat model.
  • Add the permitted ADR 0024 cross-reference; ADR 0098 retains its original decision wording and now adds a contextual cross-reference to ADR 0133.

Validation

  • make lint

Authorize filtered, actor-attributed content from events and proactive data paths to guide task execution within the configured harness scope, while preserving platform authority boundaries. Document provenance checks, Unicode and prompt-injection filtering, and fail-closed handling.

Signed-off-by: Barak Korren <bkorren@redhat.com>
Assisted-by: Codex (gpt-5.6-sol)
@qodo-code-review

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

Grey Divider

Great, no issues found!

Qodo reviewed your code and found no material issues that require review

Grey Divider

Tip of the day
💡 Did you know, you can route each severity your way: inline, summary, both, or drop

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Define the security boundary for actor-attributed agent input

📝 Documentation 🕐 20-40 Minutes

Grey Divider

AI Description

• Define when authorized actor content may guide agents without granting platform authority.
• Require provenance checks and fail-closed filtering across event, polling, snapshot, and API
 inputs.
• Align architecture and threat-model guidance with the new boundary.
Diagram

graph TD
  Sources["Event and API data"] --> Provenance{"Verified provenance?"} -->|verified| Filter["Versioned filter"] --> Admission{"Safe to admit?"} -->|admit| Prompt["Bounded prompt"] --> Model["Agent model"]
  Provenance -->|reject| Omission["Omission record"]
  Admission -->|block| Omission
  Harness["Harness limits"] --> Prompt
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Treat all source content as data-only
  • ➕ Reduces the opportunity for actor-authored text to influence agent behavior as instructions.
  • ➖ Prevents authorized contributors and recognized bots from giving agents legitimate task direction through comments and other records.

Recommendation: The ADR's bounded-guidance approach better supports legitimate agent workflows than a data-only rule. Its value depends on implementing the shared, versioned filter and provenance checks across every model-bound path; the PR defines that contract but does not implement it.

Files changed (3) +113 / -2

Documentation (3) +113 / -2
0133-actor-attributed-data-security-boundary.mdDefine actor-attributed content admission and authority boundaries +93/-0

Define actor-attributed content admission and authority boundaries

• Adds an accepted ADR distinguishing actor authorization, content admission, and platform authority. It requires per-record provenance, a versioned host-side filtering pipeline, fail-closed omissions, and explicit separation of admitted task guidance from platform instructions.

docs/ADRs/0133-actor-attributed-data-security-boundary.md

architecture.mdAdd the data security boundary to the living architecture +10/-0

Add the data security boundary to the living architecture

• Documents how provenance-labelled, filtered content from event and data-retrieval paths may reach the model. It states that failures omit content and source text cannot expand agent authority.

docs/architecture.md

security-threat-model.mdResolve threat-model questions addressed by ADR 0133 +10/-2

Resolve threat-model questions addressed by ADR 0133

• Marks questions about trusted contributor instructions and Unicode handling as resolved by the new ADR. Clarifies that admitted actors may provide scoped task guidance while raw source content stays outside model input.

docs/problems/security-threat-model.md

@ifireball ifireball self-assigned this Oct 7, 2026
@fullsend-ai-review

fullsend-ai-review Bot commented Oct 7, 2026 •

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 6:28 PM UTC · Completed 6:40 PM UTC

Commit: 4cc5c14 · View workflow run →

Runtime: pi · Model: sonnet → claude-sonnet-5-5 · Effort: high · Cost: $2.43

@github-actions

github-actions Bot commented Oct 7, 2026 •

Copy link
Copy Markdown

Site preview

Preview: https://2a734752-site.fullsend-ai.workers.dev

Commit: ca4abb996f9d4f8d94465c9236e20b21178b90ad

@codecov

codecov Bot commented Oct 7, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@fullsend-ai-review fullsend-ai-review Bot added the risk/moderate PR risk: moderate label Oct 7, 2026
@fullsend-ai-review

fullsend-ai-review Bot commented Oct 7, 2026 •

Copy link
Copy Markdown

Risk Assessment: moderate (2/5)

Details

Documentation-only PR across 7 files with low metadata risk, elevated to moderate risk by high historical commit churn, multi-author contention, and change coupling on touched architecture and threat model documents.

Previous run

Risk Assessment: moderate (2/5)

Details

Documentation-only PR across 7 files with low metadata risk, elevated to moderate risk by high historical commit churn, multi-author contention, and change coupling on touched architecture and threat model documents.

Previous run (2)

Risk Assessment: moderate (2/5)

Details

Documentation-only PR across 7 files with low metadata risk, elevated to moderate risk by high historical commit churn, multi-author contention, and change coupling on touched architecture and threat model documents.

Previous run (3)

Risk Assessment: moderate (2/5)

Details

Documentation-only PR across 7 files with low metadata risk, elevated to moderate risk by high historical commit churn, multi-author contention, and change coupling on touched architecture and threat model documents.

Previous run (4)

Risk Assessment: moderate (2/5)

Details

Documentation-only PR with low metadata risk across 5 files, elevated to moderate by high historical commit frequency, multi-author contention, and change coupling on touched architecture and threat model documents.

Previous run (5)

Risk Assessment: moderate (2/5)

Details

Documentation-only PR with low metadata risk across 5 files, elevated to moderate by high historical commit frequency, multi-author contention, and change coupling on touched architecture and threat model documents.

Previous run (6)

Risk Assessment: moderate (2/5)

Details

Documentation-only PR with low metadata risk across 5 files, elevated to moderate by high historical commit frequency, multi-author contention, and change coupling on touched architecture and threat model documents.

Previous run (7)

Risk Assessment: moderate (2/5)

Details

Documentation-only PR with low metadata risk across 6 files, elevated to moderate risk by high historical commit frequency, author contention, and change coupling on touched architecture documentation.

Previous run (8)

Risk Assessment: moderate (2/5)

Details

Documentation-only addition of ADR 0133 has low metadata risk, but score is elevated to moderate due to high churn, multi-author contention, and broad change coupling on core architecture and threat-model documents.

@fullsend-ai-review

fullsend-ai-review Bot commented Oct 7, 2026 •

Copy link
Copy Markdown

Looks good to me

Previous run

Looks good to me

Earlier findings

  • f_d69c23ff02f687a9 — resolved_by_change: ADR 0133 (lines 90-99) now states the bot-actor rule (actor.role stays none; exact provider-backed actor.bot_role recognition under ADR 0107, admitted by the harness policy, satisfies the gate; otherwise fail closed).
  • f_7333d1a6a275abda — resolved_by_change: ADR 0098 (lines 125-127) restores the original "Entity history remains untrusted input." wording and moves the ADR 0133 pointer into a separate cross-reference sentence.
  • f_1543cc3a91c264e4 — reclassified (info): the accepted whole-record residual risk (a trusted actor can relay hostile text) is documented in ADR 0133 Consequences (line 191); informational only.
Previous run (2)

Review

Findings

Medium

  • [api-contract] docs/ADRs/0133-actor-attributed-data-security-boundary.md:90 — The unconditional requirement that each record actor meet the harness's observation or mutation threshold leaves registered-bot content without a consistent admission rule. ADR 0107 keeps registered bots at actor.role = none, gives them non-ordered bot_role values such as review, and exempts recognized bots from the actor.role-keyed thresholds. This ADR accepts registered-bot roles as authorization evidence and promises that recognized bots can provide task guidance, but never says how those roles satisfy its mandatory per-record threshold. An implementation following the threshold literally would redact registered-bot comments and reviews even after a successful fresh role resolution.
    Remediation: Define the per-record admission rule separately for humans and registered bots. State whether fresh, target-scoped bot-role recognition satisfies this gate, or specify a new bot-role admission policy if model-bound authorization is intentionally stricter than ADR 0107.

Low

  • [misleading-label] docs/ADRs/0098-entity-first-harness-evaluation.md:125 — (id f_7333d1a6a275abda, still open) The Decision sentence of an Accepted ADR was edited from "Entity history remains untrusted input." to "Entity history remains untrusted input for this evaluation." That is a wording change to the decision text, not a standalone cross-reference, and the PR description says ADR 0098 "retains its original decision wording", which is inaccurate. docs/contributing/adrs.md also asks that edits to accepted ADRs be called out in the PR description.
    Remediation: Restore the exact original sentence and put the ADR 0133 pointer in a separate annotation, or correct the PR description to disclose the qualifier.

Earlier findings

  • f_fa44e4922fdd25cb — resolved_by_change: the incremental diff in docs/normative/authorization/v1/README.md rejoins the ADR 0133 link text and target onto one line, [ADR 0133](../../../ADRs/0133-actor-attributed-data-security-boundary.md).

Next steps:

  • /fs-fix — agent addresses review findings automatically
  • /fs-fix <your instruction> — agent fixes with your specific guidance
  • Push commits directly — review re-runs automatically on push
  • /fs-fix-stop — disable automatic fix runs for this PR
Previous run (3)

Review

Findings

Low

  • [pattern-inconsistency] docs/normative/authorization/v1/README.md:311 — The Markdown link to ADR 0133 is split across lines: [ADR 0133] ends line 311 and (../../../ADRs/0133-actor-attributed-data-security-boundary.md) starts line 312. CommonMark requires ( to follow ] directly, so this renders as plain text instead of a hyperlink. (Introduced by the latest push, which fixed the earlier stale-doc finding at this location.)
    Remediation: Keep [ADR 0133](../../../ADRs/0133-actor-attributed-data-security-boundary.md) together on one line.

  • [misleading-label] docs/ADRs/0098-entity-first-harness-evaluation.md:125 — The PR description says "ADR 0098 retains its original point-in-time wording", but the diff edits ADR 0098's Decision text ("Entity history remains untrusted input for this evaluation. ADR 0133 defines the separate model-bound admission contract..."). The edit reads as a minor cross-reference annotation, but docs/contributing/adrs.md asks that edits to accepted ADRs be called out in the PR description, and the description currently states the opposite.
    Remediation: Update the PR description to list the ADR 0098 cross-reference edit (or revert the edit to match the description).

Earlier findings

  • f_f9d20fee27cb3e20 — resolved_by_change: the truncated sentence in ADR 0133 is now complete ("...the record is redacted. The actor's applicable permission or recognized role MUST be co-fetched...").
  • f_503d7ad4b596c4b1 — resolved_by_change: the ADR 0133 reference in docs/problems/security-threat-model.md is now a proper link.
  • f_896c80bcea2768cd — resolved_by_change: the authorization README now scopes "untrusted" to dispatch/CEL evaluation and points to ADR 0133's model-bound admission gate (see the link-formatting finding above).
  • f_08b77899e1125103 — resolved_by_change: ADR 0098 now points to ADR 0133 for the model-bound admission contract.
  • f_1543cc3a91c264e4 — reclassified low → info: the content-relay residual risk is documented and accepted in ADR 0133's Consequences, with mitigations listed.

Next steps:

  • /fs-fix — agent addresses review findings automatically
  • /fs-fix <your instruction> — agent fixes with your specific guidance
  • Push commits directly — review re-runs automatically on push
  • /fs-fix-stop — disable automatic fix runs for this PR
Previous run (4)

Review

Findings

Medium

  • [incorrect-doc] docs/ADRs/0133-actor-attributed-data-security-boundary.md:78 — The new revision-attribution text (lines 72-77, "The checked actor is the principal authenticated...") was inserted in the middle of an existing sentence. Line 71 now ends with a dangling "The", and line 78 starts with the subjectless fragment "actor's applicable permission or recognized role MUST be co-fetched...". That garbles a normative MUST requirement about co-fetching or lazily resolving actor permission.
    Remediation: End the inserted block cleanly (drop the dangling "The" at the end of line 71) and start line 78 as a complete sentence, e.g. "The actor's applicable permission or recognized role MUST be co-fetched ...".

Low

  • [auth-bypass] docs/ADRs/0133-actor-attributed-data-security-boundary.md:155 (f_1543cc3a91c264e4, reclassified from medium) — The whole-record rule admits quoted, forwarded or copied text as task guidance under the forwarding actor's authorization, so an unauthorized contributor's request relayed by a registered bot can influence otherwise permitted mutations (a confused-deputy path within the configured scope). The Consequences section (line 184) explicitly accepts and mitigates this residual risk, and line 185 now bounds guidance to the harness scope, so it is a documented design trade-off rather than an oversight.
    Remediation: Consider requiring explicit endorsement before relayed content becomes task guidance, or keeping automated quotations data-only. If the accepted risk stays, add adversarial regression cases for unauthorized requests forwarded by registered bots.

  • [pattern-inconsistency] docs/problems/security-threat-model.md:108 (f_503d7ad4b596c4b1) — This bullet cites "ADR 0133" as plain text. The adjacent bullets (lines 104-105 and 113) link it as [ADR 0133](../ADRs/0133-actor-attributed-data-security-boundary.md).
    Remediation: Change (ADR 0133 decides to ([ADR 0133](../ADRs/0133-actor-attributed-data-security-boundary.md) decides.

  • [stale-doc] docs/normative/authorization/v1/README.md:311 (f_896c80bcea2768cd) — This file is not changed by the PR. It still says "Retained content remains untrusted throughout.", which conflicts with ADR 0133's bounded-trust model (filtered content from authorized actors may guide task execution within harness scope) unless the reader knows ADR 0133 is a target contract.
    Remediation: Add a short note or cross-reference at this line pointing to ADR 0133 and clarifying that authorized-actor content is admitted under bounded trust.

  • [stale-doc] docs/ADRs/0098-entity-first-harness-evaluation.md:125 — This file is not changed by the PR. It still says "Entity history remains untrusted input." ADR 0133 refines this, and the PR annotates ADR 0024 but not ADR 0098. The PR description says the omission is deliberate, and docs/contributing/adrs.md permits minor cross-reference annotations on Accepted ADRs, so this is advisory.
    Remediation: Optionally add a one-line cross-reference to ADR 0133 near line 125, leaving the original wording intact.

Earlier findings

  • f_aa9f6f43f5f3c72c — resolved_by_change: ADR 0133 now requires fresh authorization immediately before first model exposure and before releasing each mid-run retrieval result, with checkpoint-local cache and fail-closed semantics (lines 94-114).
  • f_a6851da36629b1a8 — resolved_by_change: lines 72-77 bind authorization to the principal authenticated as producing the exact current content revision (creator or latest authenticated editor) and require redaction when attribution cannot be established. The insertion point is a new defect, listed above.
  • f_e695c59e07572fe7 — resolved_by_change: the logical-lifetime cache still has no wall-clock expiry, but event-carried evidence cannot satisfy model-bound checks, and fresh resolution is now required before sandbox initialization, before first model exposure and before each mid-run retrieval release (lines 94-114).
  • f_1543cc3a91c264e4 — reclassified: medium to low (see above).

Next steps:

  • /fs-fix — agent addresses review findings automatically
  • /fs-fix <your instruction> — agent fixes with your specific guidance
  • Push commits directly — review re-runs automatically on push
  • /fs-fix-stop — disable automatic fix runs for this PR
Previous run (5)

Review

Findings

Medium

  • [api-contract] docs/ADRs/0133-actor-attributed-data-security-boundary.md:94 — The required checkpoint sequence cannot be satisfied by model-facing retrieval during an active run: non-event retrieval requires a sandbox-initialization check, but records fetched by a model tool are discovered after the sandbox has initialized, and the ADR requires controlled retrieval capabilities to give the same guarantees without defining an alternative checkpoint.
    Remediation: Distinguish preloaded records from records retrieved during execution; keep the sandbox-initialization recheck for the former and define an equivalent fresh authorization checkpoint immediately before releasing a mid-run retrieval result to the model, including cache and failure semantics.
  • [auth-bypass] docs/ADRs/0133-actor-attributed-data-security-boundary.md:139 — Whole-record admission lets quoted or forwarded text inside an authorized actor's record (e.g. a registered bot relaying an external contributor's request) be treated as authorized task guidance. The ADR documents and accepts this trade-off, and platform authority and forge mutations remain gated, but the mitigations do not enforce authorization of the relayed content itself.
    Remediation: Consider separating admission for analysis from authorization to supply task instructions (e.g. explicit endorsement before relayed content can direct changes), or at minimum state the accepted residual risk and its blast-radius limits as explicit harness-level requirements.

Low

  • [auth-bypass] docs/ADRs/0133-actor-attributed-data-security-boundary.md:69 — The record-actor rule does not say which actor's authorization applies to the current content of an edited record (creator vs. latest editor vs. event sender).
    Remediation: State which principal must meet the threshold for the admitted revision, and that content whose revision attribution cannot be established is redacted.
  • [auth-bypass] docs/ADRs/0133-actor-attributed-data-security-boundary.md:82 — The logical-lifetime cache has no wall-clock expiry and the final refresh occurs before sandbox initialization rather than immediately before first model exposure, leaving a residual check-to-use window.
    Remediation: Optionally define a maximum authorization age or a fresh check immediately before first model exposure, and document whether revocation after first exposure affects a running task.
  • [pattern-inconsistency] docs/problems/security-threat-model.md:109 — The note references 'ADR 0133' as plain text without a markdown link, unlike neighbouring open-question annotations.
    Remediation: Use [ADR 0133](../ADRs/0133-actor-attributed-data-security-boundary.md).
  • [stale-doc] docs/normative/authorization/v1/README.md:311 — The living authorization contract says "Retained content remains untrusted throughout" and does not reference ADR 0133 or its per-record model-bound authorization checkpoints. Not a direct contradiction (ADR 0133 is a target contract), but the wording and missing cross-reference may mislead readers.
    Remediation: Add a cross-reference to ADR 0133 and clarify the line-311 wording relative to ADR 0133's bounded-trust admission and redaction rules.

Next steps:

  • /fs-fix — agent addresses review findings automatically
  • /fs-fix <your instruction> — agent fixes with your specific guidance
  • Push commits directly — review re-runs automatically on push
  • /fs-fix-stop — disable automatic fix runs for this PR
Previous run (6)

Review

Findings

Medium

  • [design-document-alignment] docs/ADRs/0133-actor-attributed-data-security-boundary.md:83 — Requiring actor identity and current authorization to come from the source system's own authorization subsystem conflicts with the authorization providers the ADR incorporates. ADR 0107 authorizes registered bots through a trusted provider registry rather than forge permissions (their actor.role remains none), and ADR 0054 permits OWNERS-based authorization independently of collaborator permissions. Read literally, comments from those authorized actors must be redacted, contradicting the stated support for recognized bots and leaving implementers with incompatible admission rules.
    Remediation: Distinguish source-system verification of record identity and attribution from authorization through configured trusted providers. Explicitly explain how registered bot roles and OWNERS-derived permissions qualify for data admission, or document their exclusion as a change to the referenced authorization decisions.

  • [prompt-injection] docs/ADRs/0133-actor-attributed-data-security-boundary.md:123 — Assigning the record actor's authorization to all quoted, forwarded, or copied text creates an external-content laundering path: a registered bot forwarding an external report, or a maintainer quoting an attack for analysis, makes attacker text eligible task guidance even though the original author's record would be redacted. The ADR concedes filtering cannot prove the absence of prompt injection, and fixed capabilities/schema validation do not prevent an attacker-directed change within the permitted harness scope.
    Remediation: Treat quoted or forwarded material as analysis-only data unless an authorized actor explicitly adopts the requested action; for automated forwarding, retain original-source provenance and apply its admission policy instead of inheriting the forwarding bot's authorization. If the single-trust-unit rule is intentional, document the residual risk and compensating controls in Consequences.

Low

  • [authorization] docs/ADRs/0133-actor-attributed-data-security-boundary.md:68 — A verified event payload establishes authenticity, not freshness. Event-carried permission/role evidence can admit content for the event's logical lifetime with no wall-clock expiry, leaving a revocation window for delayed events (contrast ADR 0098's "current permission" language). Re-reads, retries, requeues and re-invocations already require fresh resolution, so exposure is limited to single-event delivery latency.
    Remediation: Require authoritative resolution at initial model admission, state a bounded freshness/revocation policy for event-carried authorization evidence, or explicitly accept the delivery-latency window as a documented trade-off.

Next steps:

  • /fs-fix — agent addresses review findings automatically
  • /fs-fix <your instruction> — agent fixes with your specific guidance
  • Push commits directly — review re-runs automatically on push
  • /fs-fix-stop — disable automatic fix runs for this PR
Previous run (7)

Review

Findings

Medium

  • [authorization] docs/ADRs/0133-actor-attributed-data-security-boundary.md:65 — Authorization caching is bounded only by the lifetime of an event/entity structure, with no maximum age or revalidation point. The same structure can survive queueing, retries, or further retrieval after a human's permission or a bot's registration is revoked. Restricting reuse across unrelated records does not prevent stale authorization within the original structure. This leaves a gap relative to ADR 0098's current-permission requirement and ADR 0107's registration-removal requirement, and to Authorization Contract v1 ("Historical or cached actor relationships are insufficient").
    Remediation: Define an authorization freshness point at entity evaluation/model admission, impose a bounded cache age, and require revalidation after queueing, retries, or changes to the record, target, harness threshold, or applicable policy. Explicitly include bot-registration revocation.

  • [identity-spoofing] docs/ADRs/0133-actor-attributed-data-security-boundary.md:59 — The per-record rule distinguishes a proposal updater from a comment author, but does not define how the actor is established for the exact content revision. Commit messages are explicitly listed as actor-attributed data, yet Git author/committer fields are self-asserted and forge account associations are not proof that the associated account authored those bytes. An implementation checking only the record's author and that account's permissions could admit attacker-supplied text as authorized task guidance. Mutable records (edited bodies/comments) have a weaker version of the same gap. The generic attribution-verification requirement does not resolve these source-specific cases.
    Remediation: Require authoritative attribution bound to the exact admitted revision or digest. Define editor handling for mutable records and authenticated provenance for commits; explicitly reject self-asserted Git identity/account association as sufficient evidence. Omit records whose current content attribution cannot be established.

  • [prompt-injection] docs/ADRs/0133-actor-attributed-data-security-boundary.md:100 — Authorized records may provide task instructions, but the contract does not distinguish the actor's own instructions from quoted, forwarded, or copied third-party material inside that record. Independent authorization is specified for separate comments and linked records, not embedded content. A maintainer quoting an external report, or a registered bot copying attacker-controlled upstream text, can give that text the container actor's task-guidance eligibility. Prompt-injection scanning is required, but the ADR itself acknowledges that filtering cannot prove the absence of injection.
    Remediation: Specify that record authorization does not automatically authorize embedded third-party instructions. Preserve nested provenance where available, keep quoted/forwarded material non-instructional, and require explicit authorized adoption of a requested action before treating it as task guidance. Include maintainer-quotation and bot-republication cases in the boundary's regression requirements.

  • [adr-immutability] docs/ADRs/0098-entity-first-harness-evaluation.md:134 — ADR 0098 is an Accepted point-in-time record on main. The diff changes its Decision text from 'Further trust and injection filtering MAY run...' to 'Further trust and injection filtering MUST follow ADR 0133...'. docs/contributing/adrs.md allows only minor annotations (cross-references, short notes) on Accepted ADRs and says to flag substantial rewrites of Decision sections as a policy violation; changing a normative RFC 2119 keyword goes beyond a cross-reference. adrs.md also requires edits to accepted ADRs to be called out in the PR description, and the PR body does not mention the edits to ADR 0098 or ADR 0024.
    Remediation: Restore the original ADR 0098 Decision wording and convey the newer requirement through a non-normative note (e.g., a blockquote 'Note (ADR 0133): ...' cross-reference) or rely on ADR 0133's own normative text. Call out all edits to accepted ADRs (0024, 0098) in the PR description.

  • [stale-doc] docs/normative/authorization/v1/README.md:310 — The living normative Authorization Contract v1 still says 'Further trust or injection filtering MAY run after CEL routing and before the harness pre-script. Retained content remains untrusted throughout.' This conflicts with ADR 0133's mandatory host-side verification/filtering boundary and with its rule that authorized-actor content may guide task execution. The contract is not updated or linked to ADR 0133 in this PR.
    Remediation: Update the passage to state that further trust and injection filtering MUST follow ADR 0133 after CEL routing and before the harness pre-script, and reconcile the 'remains untrusted throughout' sentence with ADR 0133's bounded-trust model; link to ADR 0133.

Low

  • [stale-doc] docs/ADRs/0054-require-authorization-on-all-agent-dispatch-paths.md:40 — ADR 0054's Status section already annotates the later ADRs that extend dispatch authorization (ADR 0107, ADR 0098). ADR 0133 builds explicitly on ADR 0054 (its Context opens with it), but ADR 0054 is not annotated with a cross-reference to ADR 0133, contrary to the writing-adrs guidance to link older ADRs to newer related decisions.
    Remediation: Add a short cross-reference note in ADR 0054's Status section pointing to ADR 0133 for actor-attributed source-record data admission.

  • [documentation-format] docs/architecture.md:393 — The new bullet uses a bold prefix (- **Target state — data security boundary:**), unlike sibling bullets in this list (e.g., Event-backed dispatch authorization:, Poll entity-discovery authorization:), which use plain unbolded prefixes. The 'Target state' qualifier itself is accurate (ADR 0133 states the boundary is not yet enforced on every path) and should be retained in some form.
    Remediation: Drop the bold markup so the prefix matches the sibling bullets while keeping the target-state qualifier (e.g., Data security boundary (target state):).


Next steps:

  • /fs-fix — agent addresses review findings automatically
  • /fs-fix <your instruction> — agent fixes with your specific guidance
  • Push commits directly — review re-runs automatically on push
  • /fs-fix-stop — disable automatic fix runs for this PR
Previous run (8)

Review

Findings

Medium

  • [authorization] docs/ADRs/0133-actor-attributed-data-security-boundary.md:46 — The admission rule groups ADR 0098's trusted poll-origin authorization with actor authorization under ADR 0054/0107, then lets content with "authorized provenance" provide task instructions. Authorization Contract v1 (docs/normative/authorization/v1/README.md) says poll-origin authorization does not authorize entity content, and it requires each action-indicating actor's current permission to meet the candidate harness's threshold. ADR 0133 doesn't restate this per-record, action-scoped check. Its mandatory host-side checks cover only source, attribution, bounds and filtering. An implementation could treat authentic content fetched by an authorized poller, or from an actor admitted through a label exception, as authorized task guidance.
    Remediation: Require host-side authorization of each instruction-bearing record's actor (current permission against the applicable stage threshold), independent of the initiating event, API credential or poll invocation. Keep ADR 0098's current-permission and stage-threshold checks. State that label grants cover only verified transitions. Withhold instruction use when authorization fails.

Low

  • [wording-defect] docs/architecture.md:393 — "authorization establishes which actor-attributed content for a run" has no predicate; it never says what authorization establishes.
    Remediation: Use "authorization establishes which actor-attributed content may be considered for admission to a run" or equivalent.
  • [adr-cross-reference] docs/ADRs/0133-actor-attributed-data-security-boundary.md:47 — ADR 0098 is a substantive dependency but appears only as plain text. It is not linked and is absent from Context. ADR 0054/0107 are also plain text in the Decision section (sibling ADRs use relative Markdown links).
    Remediation: Link 0098-entity-first-harness-evaluation.md in Context, noting that trusted invocation provenance authorizes discovery while action-indicating entity records remain subject to their actors' current permission thresholds. Link the 0054/0107 mentions too.
  • [formatting] docs/ADRs/0133-actor-attributed-data-security-boundary.md:45 — No blank line between "receives." (line 45) and "Authorization under ADR 0054..." (line 46), so they render as one paragraph.
  • [conventions] docs/problems/security-threat-model.md:99 — Resolved open questions in this repo keep the full original question inside ~~...~~ (e.g. agent-architecture.md:222). Line 99 drops "(Relates to insider threat.)", and the Unicode question near line 105 drops its follow-up sentences instead of striking them through.
  • [missing-backlink] docs/problems/agent-architecture.md:54 — ADR 0133 lists agent-architecture in relates_to, and the "Defensive behavior" bullet discusses sanitizing and validating feedback from authorized reviewers. This is a discoverability gap.
    Remediation: Add a contextual reference to ADR 0133 in that bullet.
  • [missing-cross-reference] docs/ADRs/0098-entity-first-harness-evaluation.md:134 — ADR 0098 says further trust and injection filtering "MAY" run after CEL routing, while ADR 0133 makes the filtering pipeline mandatory ("MUST").
    Remediation: Add a short annotation referencing ADR 0133 (minor annotations to accepted ADRs are permitted).
  • [missing-cross-reference] docs/ADRs/0024-harness-definitions.md:88 — The "Security scanning" item (host-side Unicode normalization, secret redaction, injection detection) overlaps directly with ADR 0133's mandatory pipeline.
    Remediation: Add an annotation under "Security scanning" referencing ADR 0133.

Next steps:

  • /fs-fix — agent addresses review findings automatically
  • /fs-fix <your instruction> — agent fixes with your specific guidance
  • Push commits directly — review re-runs automatically on push
  • /fs-fix-stop — disable automatic fix runs for this PR

fullsend-ai-review[bot]

This comment was marked as outdated.

Comment thread docs/ADRs/0133-actor-attributed-data-security-boundary.md Outdated
Comment thread docs/ADRs/0133-actor-attributed-data-security-boundary.md
Comment thread docs/ADRs/0133-actor-attributed-data-security-boundary.md Outdated
@ralphbean

Copy link
Copy Markdown
Member

Referential context: #7907 is related to this topic (but more specific).

@waynesun09 waynesun09 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

ADR convention review (manual, against main ec8f577b8). Three medium findings inline; low items already raised by the review bot (architecture.md predicate, 0098/0054/0107 links) are not repeated.

Comment thread docs/ADRs/0133-actor-attributed-data-security-boundary.md Outdated
Comment thread docs/ADRs/0133-actor-attributed-data-security-boundary.md Outdated
Comment thread docs/problems/security-threat-model.md Outdated
Require each source-system record to carry independently resolved actor authorization, with co-fetched, event-bound, or lifetime-cached permission evidence. Redact unrecognized and unauthorized records before model admission, and clarify the target boundary for controlled retrieval capabilities.

Signed-off-by: Barak Korren <bkorren@redhat.com>
Assisted-by: Codex (gpt-5.6-sol)
@fullsend-ai-review

fullsend-ai-review Bot commented Oct 7, 2026 •

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 7:43 PM UTC · Completed 7:56 PM UTC

Commit: 4408297 · View workflow run →

Runtime: pi · Model: sonnet → claude-sonnet-5-5 · Effort: high · Cost: $2.87

fullsend-ai-review[bot]

This comment was marked as outdated.

Define cache lifetime as a logical read and agent-invocation boundary. Re-fetches, retries, requeues, and termination followed by re-invocation require fresh authorization resolution even when an implementation reuses the same in-memory structure. Restore ADR 0098 wording and normalize the architecture entry.

Signed-off-by: Barak Korren <bkorren@redhat.com>
Assisted-by: Codex (gpt-5.6-sol)
@fullsend-ai-review

fullsend-ai-review Bot commented Oct 7, 2026 •

Copy link
Copy Markdown

🤖 Review · ⚠️ Cancelled · Started 8:50 PM UTC · Ended 8:54 PM UTC

Commit: 1ad49e2 · View workflow run →

@fullsend-ai-review

fullsend-ai-review Bot commented Oct 8, 2026 •

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 11:48 AM UTC · Completed 11:57 AM UTC

Commit: ca4abb9 · View workflow run →

Runtime: pi · Model: sonnet → claude-sonnet-5-5 · Effort: high · Cost: $1.92

@fullsend-ai-review
fullsend-ai-review Bot dismissed stale reviews from themself October 8, 2026 11:57

Superseded by updated review

fullsend-ai-review[bot]

This comment was marked as outdated.

@fullsend-ai-review fullsend-ai-review Bot added the ready-for-merge All reviewers approved — ready to merge label Oct 8, 2026

@waynesun09 waynesun09 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

ADR convention re-review on ca4abb9 (manual, against main ec8f577b8). All 31 earlier threads are resolved and nothing settled is re-raised; these six are new against the current text, which grew from 93 to 181 lines over the fix rounds. One high, five medium, inline.

Comment thread docs/ADRs/0133-actor-attributed-data-security-boundary.md
Comment thread docs/ADRs/0133-actor-attributed-data-security-boundary.md Outdated
Comment thread docs/ADRs/0133-actor-attributed-data-security-boundary.md
Comment thread docs/ADRs/0133-actor-attributed-data-security-boundary.md Outdated
Comment thread docs/ADRs/0133-actor-attributed-data-security-boundary.md Outdated
Comment thread docs/architecture.md Outdated
Comment thread docs/ADRs/0133-actor-attributed-data-security-boundary.md Outdated
Keep the selected authorization threshold strict for actor-attributed records, document the follow-up needed for external contributor review, and leave model-initiated source-system calls outside this ADR. Move cache lifetime and exposure checkpoint mechanics into the normative authorization contract.

Signed-off-by: Barak Korren <bkorren@redhat.com>
Assisted-by: Codex (gpt-5.6-sol)
@fullsend-ai-review

fullsend-ai-review Bot commented Oct 11, 2026 •

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 7:00 AM UTC · Completed 7:11 AM UTC

Commit: 0e6faf9 · View workflow run →

Runtime: pi · Model: sonnet → claude-sonnet-5-5 · Effort: high · Cost: $2.97

@fullsend-ai-review
fullsend-ai-review Bot dismissed their stale review October 11, 2026 07:11

Superseded by updated review

@waynesun09 waynesun09 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM. My round-2 findings are addressed in 0e6faf9.

@waynesun09
waynesun09 added this pull request to the merge queue Oct 13, 2026
Merged via the queue into main with commit a2f3e74 Oct 13, 2026
62 checks passed
@waynesun09
waynesun09 deleted the codex/adr-0133-actor-data-security branch October 13, 2026 22:16

This branch was successfully deployed

2 active (1 outdated) deployments
dev — 0e6faf9d Deployed Oct 11, 2026 by ifireball via behaviour #16275
site-preview — ca4abb99 Deployed Oct 8, 2026 by github-actions[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-for-merge All reviewers approved — ready to merge risk/moderate PR risk: moderate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants