docs(adr): propose ADR 0044 gate workflows:write on repo secret allowlist

[ifireball]

Summary

Proposes ADR 0044 (Proposed): a minimal-privilege path to grant the coder/fix GitHub App workflows: write only when target-repo workflow exposure is acceptable — without implementing the full repo-readiness checklist from #788.

Background / why this ADR exists

Link	Role in this design
#470	Coder agent cannot push .github/workflows/ changes today; GitHub requires workflows permission (see failed push in that issue).
#788	Granting workflows: write without repo hardening enables prompt-injection via new on: push workflows on agent branches; issue argues for a gate before elevation. PR #787 was closed pending this work.
ADR 0041	Implementation blocked until ADR 41 lands — assess + conditional mint in reusable-code.yml / reusable-fix.yml needs stable dispatch.

Design discussion refined the #788 “full assessment” into a narrower, deterministic gate:

• contents: write alone does not allow authoring workflow files; workflows: write is the distinct escalation.
• Assessment (code workflow only) inventories secret names visible to repo-context workflows on source_repo: repository secrets, org secrets visible to the repo, environment secrets (with fail-closed deployment-branch rules), plus default GITHUB_TOKEN workflow permissions.
• Minimal privilege: only assess when triage (or human) sets fullsend-needs-workflow-write on the issue; builtins (FULLSEND_GCP_*, FULLSEND_DISPATCH_TOKEN) are hard-allowlisted; admins may extend via explicit workflow_secret_allowlist in config.yaml (no FULLSEND_* prefix wildcard).
• Fix agent does not assess (PR job context ≠ push-time workflow secret visibility); it elevates only when PR has fullsend-workflow-write from coder (post-assess) or an authorized human (OWNER | MEMBER | COLLABORATOR).
• Policy stays in workflows, not in mint.

Test plan

• pre-commit ADR linters pass on the new file
• Reviewers agree the Proposed ADR captures intent before implementation (post–ADR 41)

Made with Cursor

[github-actions]

Site preview

Preview: https://65ad1ce2-site.fullsend-ai.workers.dev

Commit: 834cb247c90b0cf1ea64a5d559e2929a9b3d6344

[fullsend-ai-review]

Review

Findings

Medium

• [correctness] docs/ADRs/0044-gate-workflows-write-on-repo-secret-allowlist.md:2,22 — ADR status is set to Accepted in both the YAML frontmatter (line 2: status: Accepted) and the body (line 22: Accepted), but the PR description explicitly calls this a "Proposed ADR" and the test plan says "Reviewers agree the Proposed ADR captures intent before implementation." Per the ADR template, new ADRs enter as Proposed and move to Accepted when the decision is approved. Merging with Accepted status would freeze the content immediately (per the template comment: "Once this ADR is Accepted, its content is frozen"), which conflicts with the stated goal of gathering reviewer agreement first.
  Remediation: Change status: Accepted to status: Proposed in both the frontmatter and the ## Status section. Update to Accepted in a follow-up PR once reviewers have agreed on the design.

• [documentation-currency] docs/architecture.md:118 — The architecture.md living document states: "When an ADR is accepted (or superseded), this document is updated to match." If the ADR should be Proposed (not yet accepted), adding it to the "Decided" section is premature. This entry should be deferred until the ADR reaches Accepted status, or the ADR status should be corrected to Accepted intentionally (in which case the first finding is moot).
  Remediation: Either (a) remove the architecture.md bullet from this PR and add it when the ADR is accepted, or (b) if the intent is to merge as Accepted, keep the bullet and update the ADR status consistently.

Low

• [style] docs/architecture.md:118 — The new bullet is placed under Agent Identity Provider > Decided, but the ADR primarily concerns workflow permission gating, secret allowlist assessment, and label-based policy enforcement in the code/fix workflows. The Agent Infrastructure > Decided section (where ADR 0041 and other workflow-related decisions are listed) appears to be a more natural fit. The mint connection provides some justification for the current placement, but readers looking for workflow permission decisions would check Infrastructure first.
  Remediation: Consider moving the bullet to the Agent Infrastructure > Decided section.

[ralphbean]

I think this needs one change before we can merge. See inline.

[ralphbean]

[critical] I think we should be explicit here that check (4) is defense-in-depth, not a reliable gate. GitHub Actions lets individual workflow YAML files set their own permissions: block, escalating beyond the repo default. A rogue workflow pushed via workflows: write can include permissions: contents: write regardless of the repo-level default — the default only controls what workflows get when they omit the permissions: key entirely.

So this check catches repos where the defaults are already wide open (a signal that the org isn't thinking about least privilege), but it can't actually prevent a rogue workflow from obtaining a write-scoped GITHUB_TOKEN. The assessment as written treats it as a hard gate alongside (1)–(3), which I think overstates what it provides.

[ifireball]

@ralphbean

hmm, you are right, I was focused on secret exfiltration, but writing into main is also possible via a rogue workflow.

So perhaps I should refocus on trying to ensure the pushed bot commit does not create or modify any workflows that would trigger as a result of the merge itself, or perhaps add that in addition to the mechanism I have already described.

WDYT?

[ralphbean]

[moderate, non-blocking] The authorization floor for the PR label override is OWNER | MEMBER | COLLABORATOR — same as /fs-fix. In large orgs, COLLABORATOR can be pretty broad (external contributors with push access). Since this override bypasses assessment entirely, do we want a tighter floor here (OWNER | MEMBER only)? Or does matching the /fs-fix authorization rule feel right for consistency?

[rh-hemartin]

This only prevent our agents from submitting a PR that wants to extract secrets, but what people is doing without agents? They shoyld have some prevention mechanism so GH does not run workflows without permission, right? Can't we just use that and do not implement this?

[ifireball]

This only prevent our agents from submitting a PR that wants to extract secrets, but what people is doing without agents? They shoyld have some prevention mechanism so GH does not run workflows without permission, right? Can't we just use that and do not implement this?

This is not about running workflows, its about writing workflow files.

Unauthorized humans typically create branches in their forks and PRs out of those - so as long as the PR isn't merged, a contributor cannot get his workflow to run in the context of the target repo.

Bots, however, create PRs by pushing branches directly into the target repo, such a branch can contain a new workflow that would trigger on push. To mitigate that, GitHub blocks commits pushed with workflow file modifications unless the pusher has a token with a "workflow: write" permission.

This whole PR and set of issues is about finding a way to grant "workflow: write" to the code agent so that it can edit workflows without causing it to become an attack platform into the repo.

[rh-hemartin]

Ok, so the current protection is forcing contributors to open a fork.

[ifireball]

Ok, so the current protection is forcing contributors to open a fork.

Yes, if you don't trust them. don't let them push (write) into the repo...