Skip to content

fix(#6452): propagate org-level allowed_remote_resources to URL resolution - #6454

Open
fullsend-ai-coder[bot] wants to merge 5 commits into
mainfrom
agent/6452-org-allowlist-policy-resolve
Open

fix(#6452): propagate org-level allowed_remote_resources to URL resolution#6454
fullsend-ai-coder[bot] wants to merge 5 commits into
mainfrom
agent/6452-org-allowlist-policy-resolve

Conversation

@fullsend-ai-coder

Copy link
Copy Markdown
Contributor

Summary

Propagate the org-level allowed_remote_resources from config.yaml to all URL resolution paths, not just base: composition. Previously, policy:, agent:, skills:, plugins:, profiles:, and providers: URLs were only checked against the harness-level AllowedRemoteResources, causing confusing failures when users expected org-level trust to apply uniformly.

Related Issue

Fixes #6452

Changes

  • Add OrgAllowlist []string field to resolve.ResolveOpts
  • Update resolveFileURL and resolveSkillDirURL to fall back to the org allowlist when the harness-level check fails
  • Pass orgAllowlist in run.go and lock.go callers when calling resolve.ResolveHarness
  • Update resolveFromLock to accept and check the org allowlist during lock-file validation
  • Add tests covering: policy/agent/skill resolution via org allowlist, rejection when URL is in neither allowlist, and mixed harness+org allowlist scenarios

Testing

  • All new org-allowlist tests pass
  • All existing TestResolveHarness_* tests pass (no regressions)
  • All existing TestResolveFromLock_* tests pass (no regressions)
  • go vet passes
  • go build ./... passes

Closes #6452

Post-script verification

  • Branch is not main/master (agent/6452-org-allowlist-policy-resolve)
  • Secret scan passed (gitleaks — 18792930cd964497a98a4ba99d4543be13ece8a3..HEAD)
  • PR body secret scan passed (gitleaks — no-git)

@fullsend-ai-coder
fullsend-ai-coder Bot requested a review from a team as a code owner August 21, 2026 15:28
@fullsend-ai-coder fullsend-ai-coder Bot added the ready-for-review Triggers review agent dispatch label Aug 21, 2026
@fullsend-ai-review

fullsend-ai-review Bot commented Aug 21, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 3:30 PM UTC · Completed 3:47 PM UTC

Commit: d6db2a5 · View workflow run →

@codecov

codecov Bot commented Aug 21, 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 commented Aug 21, 2026

Copy link
Copy Markdown

Review

Findings

Medium

  • [authorization-scope-change] internal/resolve/resolve.go:581 — This PR widens the effective trust surface for URL resolution. Previously, the org-level allowed_remote_resources from config.yaml was used as a fallback only for base: composition. This change extends that org-level fallback to all URL resolution types (agents, policies, skills, plugins, profiles, providers). The control model is correctly fail-closed (empty/nil lists deny all), SHA-256 integrity pins are enforced, and HTTPS-only is preserved. The documentation updates explicitly confirm this is the intended authorization model. No code change needed.

Low

  • [implicit trust expansion via defaults] internal/cli/run.go:538 — When no config.yaml exists, orgAllowlist falls back to config.DefaultAllowedRemoteResources() (two first-party fullsend-ai GitHub raw URLs). Before this PR, these defaults only applied to base: composition. They now serve as a fallback for all URL resolution types. Risk is low: defaults are first-party URLs, SHA-256 integrity hashes are still required, and absence of config.yaml means the organization has not locked down its resource policies.
Previous run

Review

Findings

Medium

  • [authorization-scope-change] internal/resolve/resolve.go:581 — This PR widens the effective trust surface for URL resolution. Previously, the org-level allowed_remote_resources from config.yaml was used as a fallback only for base: composition. This change extends that org-level fallback to all URL resolution types (agents, policies, skills, plugins, profiles, providers). The control model is correctly fail-closed (empty/nil lists deny all), SHA-256 integrity pins are enforced, and HTTPS-only is preserved. The documentation updates explicitly confirm this is the intended authorization model. No code change needed.

  • [code-organization] docs/ADRs/0038-universal-harness-access.md:261 — Inline > **Note (PR #6454):** annotation inside ADR narrative text is inconsistent with this ADR's own convention. ADR 0038 already has an ## Amendments section with dated, titled entries. The new note should follow that established pattern rather than being inserted mid-paragraph as a blockquote.
    Remediation: Move the amendment text into a new entry under the existing ## Amendments section at the end of this ADR, following the ### YYYY-MM-DD: Title (#issue) format used by the two existing amendments.

Low

  • [implicit trust expansion via defaults] internal/cli/run.go:538 — When no config.yaml exists, orgAllowlist falls back to config.DefaultAllowedRemoteResources() (two first-party fullsend-ai GitHub raw URLs). Before this PR, these defaults only applied to base: composition. They now serve as a fallback for all URL resolution types. Risk is low: defaults are first-party URLs, SHA-256 integrity hashes are still required, and absence of config.yaml means the organization has not locked down its resource policies.

  • [code-organization] docs/ADRs/0070-portable-provider-profile-resolution.md:210 — Same inline > **Note (PR #6454):** annotation pattern as ADR 0038 — inconsistent with ADR amendment conventions established elsewhere in the repo.
    Remediation: Add a ## Amendments section at the end of this ADR with a dated entry describing the org-level fallback change, or reference the amendment in ADR 0038.

  • [naming-conventions] internal/cli/lock.go:745 — Comment shortened from "Skip the harness-level allowlist check for these entries" to "Skip the allowlist check for these entries". With two allowlists now present (harness-level and org-level), the shortened comment is ambiguous about which check(s) are being skipped.
    Remediation: Clarify the comment: // Skip both harness-level and org-level allowlist checks for agent_source entries — they were validated at lock creation time.


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 (2)

Review

Findings

Medium

  • [authorization-scope-change] internal/resolve/resolve.go:581 — This PR widens the effective trust surface for URL resolution. Previously, the org-level allowed_remote_resources from config.yaml was used as a fallback only for base: composition. This change extends that org-level fallback to all URL resolution types (agents, policies, skills, plugins, profiles, providers). The control model is correctly fail-closed (empty/nil lists deny all), SHA-256 integrity pins are enforced, and HTTPS-only is preserved. The documentation updates explicitly confirm this is the intended authorization model. No code change needed.

Low

  • [implicit trust expansion via defaults] internal/cli/run.go:538 — When no config.yaml exists, orgAllowlist falls back to config.DefaultAllowedRemoteResources() (two first-party fullsend-ai GitHub raw URLs). Before this PR, these defaults only applied to base: composition. They now serve as a fallback for all URL resolution types. Risk is low: defaults are first-party URLs, SHA-256 integrity hashes are still required, and absence of config.yaml means the organization has not locked down its resource policies.

  • [stale-doc] docs/ADRs/0070-portable-provider-profile-resolution.md:209 — States "Profile/provider URLs must match AllowedRemoteResources prefixes" without mentioning the org-level allowlist fallback. After this PR, profile/provider URLs pass validation if they match either the harness-level AllowedRemoteResources or the org-level allowed_remote_resources from config.yaml.
    Remediation: Amend the bullet to note the org-level fallback.


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

Medium

  • [authorization scope change] internal/resolve/resolve.go:581 — This PR widens the effective trust surface for URL resolution. Previously, the org-level allowed_remote_resources from config.yaml was used as a fallback only for base: composition. This change extends that org-level fallback to all URL resolution types (agents, policies, skills, plugins, profiles, providers). The control model is correctly fail-closed (empty/nil lists deny all), SHA-256 integrity pins are enforced, and HTTPS-only is preserved. No code change needed — confirm this is the intended authorization model.

  • [stale-doc] docs/guides/user/customizing-agents.md:109 — The note says allowed_remote_resources "are NOT inherited from base harnesses — the child must declare its own." The PR updated the parallel note in bring-your-own-agent.md to add the org-level fallback caveat, but customizing-agents.md was not updated. The statement now overstates the restriction: URLs trusted at the org level pass even when the child harness omits the field.

  • [stale-doc] docs/ADRs/0038-universal-harness-access.md:260 — States the effective allowlist is the "intersection" of org-level and harness-level declarations — "both must allow a domain for it to be trusted." After this PR, the effective allowlist is a union (harness OR org), not an intersection. A URL in the org allowlist passes resolution even when absent from the harness-level list, directly contradicting "both must allow."

Low

  • [implicit trust expansion via defaults] internal/cli/run.go:538 — When no config.yaml exists, orgAllowlist falls back to config.DefaultAllowedRemoteResources() (two first-party fullsend-ai GitHub raw URLs). Before this PR, these defaults only applied to base: composition. They now serve as a fallback for all URL resolution types.

  • [doc-intent-mismatch] docs/guides/user/bring-your-own-agent.md:294 — The appended "However" clause about org-level fallback is grafted onto a note whose purpose is explaining what is NOT inherited from base harnesses. A reader could conflate base-harness inheritance with org-level fallback — two distinct trust layers.

  • [stale-doc] docs/reference/harness-reference.md:218 — The merge-rules table says allowed_remote_resources is "NOT inherited (child must declare its own)," which is now an overstatement since the org-level allowlist acts as a fallback.


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

  • [stale-doc] docs/guides/infrastructure/layered-config-reference.md:235 — The description of allowed_remote_resources says it "controls which URL prefixes are allowed for remote agent sources and base composition." After this PR, the org-level allowed_remote_resources from config.yaml also serves as a fallback for all URL resolution (policy, skills, plugins, profiles, providers), not just agent sources and base composition. The description is now narrower than the actual behavior.

Low

  • [test-inadequate] internal/cli/lock_test.go — All 38 resolveFromLock test call sites pass nil for the new orgAllowlist parameter, so the org-level allowlist fallback in resolveFromLock is untested in the lock-file path. The equivalent logic is well-tested via resolve_test.go and MatchingAllowedPrefixInList has unit tests, mitigating the risk.

  • [incomplete-doc] docs/guides/user/bring-your-own-agent.md:397 — The note says allowed_remote_resources "are NOT inherited from base harnesses — the child must declare its own." While still correct for base-to-child inheritance, it may mislead users: the org-level config.yaml allowed_remote_resources now provides a fallback even when the child harness omits the field.

  • [stale-doc] docs/ADRs/0045-forge-portable-harness-schema.md:613 — Describes the default URL allowlist as specifically "for base composition." After this PR, the org-level allowlist also provides fallback for all URL resolution. (ADR is a point-in-time record; no update required.)

  • [stale-doc] docs/ADRs/0024-harness-definitions.md:444 — The allowed_remote_resources comment lists "skills, agents, plugins, and policies" but omits profiles and providers. (Pre-existing omission not caused by this PR.)

  • [field-ordering] internal/resolve/resolve.go:220OrgAllowlist is placed after GitToken in ResolveOpts, but before TreeFetcher in the sibling ComposeOpts struct. Minor stylistic inconsistency across packages.

@fullsend-ai-review fullsend-ai-review Bot added the requires-manual-review Review requires human judgment label Aug 21, 2026
…ution

The org-level allowed_remote_resources from config.yaml was only used
during base: composition (via ComposeOpts.OrgAllowlist) but not during
URL resolution of policy:, agent:, skills:, plugins:, profiles:, and
providers: fields. This caused confusing behavior where base: URLs
resolved successfully but policy: URLs with the same prefix failed.

Add OrgAllowlist field to ResolveOpts and check it as a fallback in
resolveFileURL and resolveSkillDirURL when the harness-level
AllowedRemoteResources check fails. Pass the org allowlist from both
run.go and lock.go callers. Also update resolveFromLock to accept and
check the org allowlist for lock-file validation.

Closes #6452
@ggallen
ggallen force-pushed the agent/6452-org-allowlist-policy-resolve branch from d6db2a5 to bc8e4ba Compare September 7, 2026 21:11
@ggallen ggallen added the fullsend-fix Enables automatic bot-triggered fix runs on human-authored PRs label Sep 7, 2026
@ggallen

ggallen commented Sep 7, 2026

Copy link
Copy Markdown
Member

/fs-fix

@fullsend-ai-review

fullsend-ai-review Bot commented Sep 7, 2026

Copy link
Copy Markdown

🤖 Review · ⚠️ Cancelled · Started 9:13 PM UTC · Ended 9:24 PM UTC

Commit: bc8e4ba · View workflow run →

@fullsend-ai-coder

fullsend-ai-coder Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor Author

🤖 Finished Fix · ✅ Success · Started 9:14 PM UTC · Completed 9:23 PM UTC

Commit: bc8e4ba · View workflow run →

Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $1.95

- Update layered-config-reference.md to describe allowed_remote_resources
  as covering all remote resource types, not just agent sources and base
  composition
- Add TestResolveFromLock_OrgAllowlistFallback to exercise the org-level
  allowlist in the lock-file resolution path
- Update bring-your-own-agent.md note to mention org-level config.yaml
  fallback for URL resolution
- Reorder OrgAllowlist field in ResolveOpts to match ComposeOpts ordering
- Fix 4 additional resolveFromLock call sites missing the orgAllowlist
  parameter

Note: pre-commit hooks were not run. pre-commit could not complete
(infrastructure failure), and hooks were run directly instead (gofmt,
go vet).

Addresses #6454
@fullsend-ai-coder

Copy link
Copy Markdown
Contributor Author

🔧 Fix agent — iteration 1 (human-triggered)

Addressed all 6 review findings: fixed 4 (doc updates for layered-config-reference and bring-your-own-agent, added resolveFromLock org-allowlist test, reordered ResolveOpts fields), disagreed with 2 (both ADR findings were acknowledged by the reviewer as not requiring changes).

Fixed (4):

  1. Description of allowed_remote_resources is narrower than actual behavior (docs/guides/infrastructure/layered-config-reference.md): Updated description from 'remote agent sources and base composition' to 'remote resources (agents, policies, skills, plugins, profiles, providers, and base composition)' to reflect the broadened scope after this PR
  2. All 38 resolveFromLock test call sites pass nil for orgAllowlist (internal/cli/lock_test.go): Added TestResolveFromLock_OrgAllowlistFallback test that exercises the org-level allowlist in the lock-file resolution path, verifying that a URL not in the harness-level allowlist is accepted when covered by the org-level allowlist. Also fixed 4 additional call sites that were missing the orgAllowlist parameter.
  3. Note about allowed_remote_resources inheritance may mislead users about org-level fallback (docs/guides/user/bring-your-own-agent.md): Added clarification that the org-level allowed_remote_resources from config.yaml acts as a fallback for all URL resolution, so URLs trusted at the org level are accepted even when the child harness omits the field
  4. OrgAllowlist field ordering inconsistency between ResolveOpts and ComposeOpts (internal/resolve/resolve.go): Moved OrgAllowlist before GitToken in ResolveOpts to match the field ordering in ComposeOpts (where OrgAllowlist appears before TreeFetcher and GitToken)

Disagreed (2):

  1. ADR 0045 describes default URL allowlist as specifically for base composition: The reviewer explicitly noted 'ADR is a point-in-time record; no update required.' ADRs document decisions as made; updating them to reflect subsequent changes would undermine their purpose as historical records.
  2. ADR 0024 allowed_remote_resources comment omits profiles and providers: The reviewer noted this is a 'pre-existing omission not caused by this PR.' Fixing unrelated pre-existing documentation gaps is out of scope for this review fix.

Tests: passed

Decision points
  • Whether to update ADR 0045 to reflect broadened allowlist scope (alternatives: Update the ADR text, Leave ADR as point-in-time record; rationale: ADRs are immutable historical records. The reviewer explicitly agreed no update was required.)
  • Whether to fix pre-existing ADR 0024 omission of profiles and providers (alternatives: Fix the pre-existing omission, Leave as out-of-scope; rationale: The omission predates this PR and is unrelated to the changes. Fixing it would expand scope beyond the review feedback.)

Next steps:

  • /fs-review — request a re-review of the changes
  • /fs-fix <your instruction> — run another fix pass with specific guidance
  • Push commits directly — review re-runs automatically on push
    Updated by fullsend fix agent

@fullsend-ai-review

fullsend-ai-review Bot commented Sep 7, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 9:26 PM UTC · Completed 9:43 PM UTC

Commit: 7b43102 · View workflow run →

Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $7.28

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

fullsend-ai-review Bot commented Sep 7, 2026

Copy link
Copy Markdown

Risk Assessment: moderate (2/5)

Details

Bot-authored targeted bug fix with no protected paths, CI, security, or dependency changes; high fix/revert churn across core files sustains moderate caution — score unchanged at 2, consistent with prior assessment.

Previous run

Risk Assessment: moderate (2/5)

Details

Targeted bot-authored bug fix with no protected paths, CI, security, or dependency changes; test ratio is mid-band; high-churn files with 17 distinct recent authors sustain moderate caution, consistent with prior assessment — score unchanged at 2.

Previous run (2)

Risk Assessment: moderate (2/5)

Details

Targeted bug fix propagating an org-level allowlist to all URL resolution paths; change is well-scoped and test-accompanied, but internal/resolve/resolve.go and internal/cli/lock.go remain high-churn files with six-to-seven distinct recent authors, warranting moderate caution. Signals are unchanged from prior assessment — score preserved at 2.

Previous run (3)

Risk Assessment: moderate (2/5)

Details

Targeted bug fix propagating an org-level allowlist to all URL resolution paths; change is well-scoped and test-accompanied, but internal/resolve/resolve.go is a high-churn file with six distinct recent authors, warranting moderate caution.

fullsend-ai-review[bot]

This comment was marked as outdated.

fullsend-ai-review[bot]

This comment was marked as outdated.

@fullsend-ai-review fullsend-ai-review Bot removed the requires-manual-review Review requires human judgment label Sep 7, 2026
@fullsend-ai-coder

fullsend-ai-coder Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor Author

🤖 Finished Fix · ✅ Success · Started 9:45 PM UTC · Completed 9:55 PM UTC

Commit: 7b43102 · View workflow run →

Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $1.46

Update four docs to reflect that the org-level allowed_remote_resources
from config.yaml now acts as a fallback for all URL resolution types,
not just base composition:

- customizing-agents.md: add org-level fallback caveat to inheritance note
- bring-your-own-agent.md: separate base-inheritance note from org-level
  fallback into distinct blockquotes for clarity
- harness-reference.md: add fallback qualifier to merge-rules table
- ADR 0038: add annotation noting the union resolution model

Addresses #6454
@fullsend-ai-coder

Copy link
Copy Markdown
Contributor Author

🔧 Fix agent — iteration 2 (bot-triggered)

Updated four documentation files to reflect that the org-level allowed_remote_resources now acts as a fallback for all URL resolution. Fixed stale notes in customizing-agents.md and harness-reference.md, added an ADR annotation to 0038, and improved clarity in bring-your-own-agent.md by separating base-inheritance from org-level fallback concepts. Two informational findings (authorization scope confirmation and default trust expansion) were acknowledged as intentional design.

Fixed (4):

  1. [stale-doc] customizing-agents.md:109 — missing org-level fallback caveat (docs/guides/user/customizing-agents.md): Added the org-level allowed_remote_resources fallback caveat to the inheritance note, matching the update already made in bring-your-own-agent.md
  2. [stale-doc] ADRs/0038-universal-harness-access.md:260 — says intersection but behavior is union (docs/ADRs/0038-universal-harness-access.md): Added an annotation note (per ADR immutability rules) clarifying that URL resolution now uses union semantics (org-level OR harness-level), while the subset constraint on harness-level entries is preserved
  3. [doc-intent-mismatch] bring-your-own-agent.md:294 — However clause conflates base-inheritance with org-level fallback (docs/guides/user/bring-your-own-agent.md): Split the combined note into two separate blockquotes: one for base-inheritance restriction, one for org-level fallback, clearly distinguishing the two trust layers
  4. [stale-doc] harness-reference.md:218 — merge-rules table overstates restriction (docs/reference/harness-reference.md): Added org-level fallback qualifier to the allowed_remote_resources merge-rules table entry

Disagreed (2):

  1. [authorization scope change] resolve.go:581 — confirm intended authorization model: Reviewer explicitly noted 'No code change needed — confirm this is the intended authorization model.' The PR description, issue Inconsistent allowlist behavior: base: uses config.yaml, policy: requires harness-level allowed_remote_resources #6452, and the code all confirm this is the intended design: org-level trust should apply uniformly to all URL resolution types.
  2. [implicit trust expansion via defaults] run.go:538 — DefaultAllowedRemoteResources now applies to all URL resolution: This is the intended behavior per the PR's purpose. The defaults (two first-party fullsend-ai GitHub raw URLs) were already trusted for base composition; extending to all URL resolution types is the explicit goal of this change. The defaults are well-scoped (only fullsend-ai URLs) and the code already has a comment explaining the fallback.

Tests: passed

Decision points
  • ADR 0038 says 'intersection' but actual behavior is now 'union' for resolution (alternatives: Rewrite the ADR section to say union, Add an annotation note preserving the original text; rationale: ADR immutability rules (docs/contributing/adrs.md) prohibit substantially rewriting accepted ADRs. A short annotation is the correct approach.)
  • bring-your-own-agent.md had a single note conflating two concepts (alternatives: Keep single note with 'However' clause, Split into two distinct blockquotes; rationale: Splitting into two blockquotes makes the two trust layers (base-inheritance vs org-level fallback) clearly distinct, addressing the reviewer's concern about reader confusion.)

Next steps:

  • /fs-review — request a re-review of the changes
  • /fs-fix <your instruction> — run another fix pass with specific guidance
  • Push commits directly — review re-runs automatically on push
    Updated by fullsend fix agent

@fullsend-ai-review

fullsend-ai-review Bot commented Sep 7, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 9:56 PM UTC · Completed 10:13 PM UTC

Commit: b5b9601 · View workflow run →

Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $9.98

fullsend-ai-review[bot]

This comment was marked as outdated.

fullsend-ai-review[bot]

This comment was marked as outdated.

@fullsend-ai-coder

fullsend-ai-coder Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor Author

🤖 Finished Fix · ✅ Success · Started 10:14 PM UTC · Completed 10:21 PM UTC

Commit: b5b9601 · View workflow run →

Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $1.15

Add a minor annotation to the Security section of ADR 0070 noting that
profile/provider URLs now also pass validation against the org-level
allowed_remote_resources from config.yaml, consistent with the changes
in this PR.

Note: pre-commit hooks were not run. pre-commit could not complete
(infrastructure failure), and the direct-execution fallback ran the
relevant local hooks (lint-adr-status, lint-adr-numbers, lint-docs-links,
lychee, trailing-whitespace, end-of-file-fixer) manually.

Addresses #6454
@fullsend-ai-coder

Copy link
Copy Markdown
Contributor Author

🔧 Fix agent — iteration 3 (bot-triggered)

Added org-level allowlist fallback annotation to ADR 0070 Security section. Two informational findings (authorization scope confirmation and default trust expansion) were acknowledged as intentional design with no code change needed.

Fixed (1):

  1. [stale-doc] docs/ADRs/0070-portable-provider-profile-resolution.md:209 — missing org-level allowlist fallback (docs/ADRs/0070-portable-provider-profile-resolution.md): Added a minor annotation (Note PR fix(#6452): propagate org-level allowed_remote_resources to URL resolution #6454) to the Security section bullet noting that profile/provider URLs now also pass validation against the org-level allowed_remote_resources from config.yaml, consistent with ADR immutability rules

Disagreed (2):

  1. [authorization-scope-change] internal/resolve/resolve.go:581 — confirm intended authorization model: Reviewer explicitly stated 'No code change needed.' This is an informational finding confirming the authorization model is correct. The PR description, issue Inconsistent allowlist behavior: base: uses config.yaml, policy: requires harness-level allowed_remote_resources #6452, and code all confirm the intended design: org-level trust applies uniformly to all URL resolution types.
  2. [implicit trust expansion via defaults] internal/cli/run.go:538 — DefaultAllowedRemoteResources now applies to all URL resolution: This is the intended behavior per the PR's purpose. The defaults (two first-party fullsend-ai GitHub raw URLs) were already trusted for base composition; extending to all URL resolution types is the explicit goal of this change. Risk is low as noted by the reviewer: defaults are first-party URLs, SHA-256 integrity hashes are still required, and absence of config.yaml means the organization has not locked down its resource policies.

Tests: passed

Decision points
  • Used ADR annotation pattern (Note PR fix(#6452): propagate org-level allowed_remote_resources to URL resolution #6454) rather than rewriting the ADR bullet (alternatives: Rewrite the bullet text directly, Add an annotation note preserving original text; rationale: ADR immutability rules (docs/contributing/adrs.md) prohibit substantially rewriting accepted ADRs. A short annotation is the correct approach, consistent with the pattern used for ADR 0038 earlier in this PR.)

Next steps:

  • /fs-review — request a re-review of the changes
  • /fs-fix <your instruction> — run another fix pass with specific guidance
  • Push commits directly — review re-runs automatically on push
    Updated by fullsend fix agent

@fullsend-ai-review

fullsend-ai-review Bot commented Sep 7, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 10:23 PM UTC · Completed 10:40 PM UTC

Commit: 1e33740 · View workflow run →

Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $5.85

fullsend-ai-review[bot]

This comment was marked as outdated.

fullsend-ai-review[bot]

This comment was marked as outdated.

@fullsend-ai-coder

fullsend-ai-coder Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor Author

🤖 Finished Fix · ✅ Success · Started 10:41 PM UTC · Completed 10:50 PM UTC

Commit: 1e33740 · View workflow run →

Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $1.92

….go comment

Move inline Note (PR #6454) blockquotes in ADR 0038 and ADR 0070
to dated entries under Amendments sections, following the established
YYYY-MM-DD Title (#issue) convention used by existing amendments.
Clarify the ambiguous allowlist-skip comment in lock.go to specify that
both harness-level and org-level checks are skipped for agent_source entries.

Addresses #6454
@fullsend-ai-coder

Copy link
Copy Markdown
Contributor Author

🔧 Fix agent — iteration 4 (bot-triggered)

Moved ADR inline annotations to Amendments sections (strategy change from prior iterations), clarified ambiguous lock.go comment. Two informational findings acknowledged as intentional design with no code change needed.

Fixed (3):

  1. [code-organization] ADR 0038 inline Note (PR fix(#6452): propagate org-level allowed_remote_resources to URL resolution #6454) annotation inconsistent with Amendments convention (docs/ADRs/0038-universal-harness-access.md): Moved the inline blockquote annotation from the Trust boundary paragraph to a new dated entry under the existing ## Amendments section, following the ### YYYY-MM-DD: Title (#issue) format used by the two existing amendments
  2. [code-organization] ADR 0070 inline Note (PR fix(#6452): propagate org-level allowed_remote_resources to URL resolution #6454) annotation inconsistent with ADR amendment conventions (docs/ADRs/0070-portable-provider-profile-resolution.md): Removed the inline blockquote annotation from the Security section and added a new ## Amendments section at the end of the ADR with a dated entry describing the org-level fallback change, cross-referencing the ADR 0038 amendment
  3. [naming-conventions] lock.go comment ambiguous about which allowlist checks are skipped (internal/cli/lock.go): Clarified the comment to explicitly state that both harness-level and org-level allowlist checks are skipped for agent_source entries, since they were validated at lock creation time

Disagreed (2):

  1. [authorization-scope-change] resolve.go:581 — confirm intended authorization model: Reviewer explicitly stated 'No code change needed.' This is an informational finding confirming the authorization model is correct. The PR description, issue Inconsistent allowlist behavior: base: uses config.yaml, policy: requires harness-level allowed_remote_resources #6452, and code all confirm the intended design.
  2. [implicit trust expansion via defaults] run.go:538 — DefaultAllowedRemoteResources now applies to all URL resolution: This is the intended behavior per the PR's purpose. The reviewer rated the risk as low: defaults are first-party URLs, SHA-256 integrity hashes are still required, and absence of config.yaml means the organization has not locked down its resource policies. No code change needed.

Tests: passed

Strategy change: Previous iterations (1-3) used inline blockquote annotations in ADR text. The reviewer repeatedly flagged this pattern as inconsistent with ADR conventions. This iteration moves the annotations to ## Amendments sections with dated entries, following the established ### YYYY-MM-DD: Title (#issue) format.

Decision points
  • Strategy change at iteration 4: use Amendments sections instead of inline blockquote annotations (alternatives: Keep inline blockquote annotations (rejected by reviewer in iterations 2-4), Move annotations to dated entries in ## Amendments sections; rationale: Previous iterations used inline > Note (PR fix(#6452): propagate org-level allowed_remote_resources to URL resolution #6454): blockquotes which the reviewer repeatedly flagged as inconsistent with ADR conventions. ADR 0038 already has an ## Amendments section with two dated entries. Using the same pattern addresses the root cause across both ADR findings.)

Next steps:

  • /fs-review — request a re-review of the changes
  • /fs-fix <your instruction> — run another fix pass with specific guidance
  • Push commits directly — review re-runs automatically on push
    Updated by fullsend fix agent

@fullsend-ai-coder fullsend-ai-coder Bot added the needs-human Agent loop needs human intervention label Sep 7, 2026
@fullsend-ai-review

fullsend-ai-review Bot commented Sep 7, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 10:52 PM UTC · Completed 11:09 PM UTC

Commit: 638d0f4 · View workflow run →

Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $5.56

@fullsend-ai-review
fullsend-ai-review Bot dismissed stale reviews from themself September 7, 2026 23:09

Superseded by updated review

@fullsend-ai-review fullsend-ai-review Bot added the requires-manual-review Review requires human judgment label Sep 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fullsend-fix Enables automatic bot-triggered fix runs on human-authored PRs needs-human Agent loop needs human intervention ready-for-review Triggers review agent dispatch requires-manual-review Review requires human judgment risk/moderate PR risk: moderate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Inconsistent allowlist behavior: base: uses config.yaml, policy: requires harness-level allowed_remote_resources

1 participant