docs(#6689): two setup routes and per-repository trust in the OpenAI WIF guide - #6777
docs(#6689): two setup routes and per-repository trust in the OpenAI WIF guide#6777waynesun09 wants to merge 1 commit into
Conversation
PR Summary by QodoDocument two OpenAI WIF setup routes and per-repository trust
AI Description
Diagram
High-Level Assessment
Files changed (2)
|
|
/fs-fix-stop |
|
Fix agent disabled for this PR. Remove the |
|
🤖 Review · Commit: |
Site previewPreview: https://d67afca9-site.fullsend-ai.workers.dev Commit: |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
…WIF guide The guide read as if managing the organization's identity provider were a prerequisite. In most companies an IT administrator owns Organization Settings and the GitHub Actions provider, and a team owns (or requests) a project and a service account. The setup is now an explicit fork: shared claims check first, then route A (you manage providers: add or reuse one, add the mapping) or route B (send the administrator a request — template included — and record the identifiers you get back); step 4 onwards is common to both. The audience is described as whatever the provider has (any string, used verbatim) instead of a fullsend:// value. The per-repository trust rule is stated for both routes: one mapping (or allowlist entry) per company-owned repository, never a pattern over the organization, since an organization can contain repositories the company does not own. Also drops the org-wide installation note (that mode is deprecated), re-points the troubleshooting rows, and fixes the pi runtime doc's "three repository variables" wording. Assisted-by: Claude Signed-off-by: Wayne Sun <gsun@redhat.com>
d89179d to
3d094bc
Compare
Code Review by Qodo
1. Prerequisites heading was removed
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
| > **GitHub Actions only.** The exchange needs the job's OIDC endpoint. For GitLab CI and for runs | ||
| > on your own machine, use an API key in the runner environment — see [Run it locally](#run-it-locally). | ||
|
|
||
| ## Before you start |
There was a problem hiding this comment.
1. Prerequisites heading was removed 📜 Skill insight ✧ Quality
The guide replaces its Before you start section with What you end up with, leaving prerequisite information unlabeled before step 1. Readers can begin the procedure without a clearly identified prerequisites section.
Agent Prompt
## Issue description
The modified guide no longer has a clearly labeled prerequisites section before its procedure.
## Issue Context
The OpenAI project, repository enrollment, and route-specific access requirements should be grouped under `## Prerequisites` before step 1.
## Fix Focus Areas
- docs/guides/infrastructure/openai-workload-identity.md[15-30]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| If your organization already has a provider for GitHub Actions, reuse it: open it, note its | ||
| **audience** and copy its **identity provider ID**, and go to A3. Otherwise, in **Organization | ||
| Settings → Security → Workload Identity Provider**, add one: |
There was a problem hiding this comment.
2. Route procedures use prose 📜 Skill insight ✧ Quality
The new route-specific procedures describe sequences of actions in prose paragraphs rather than ordered lists. This makes the operational sequence less explicit and violates the required guide procedure format.
Agent Prompt
## Issue description
The new A2/A3 and B2/B3 procedures express multiple actions as prose instead of numbered steps.
## Issue Context
Preserve the route headings, but format each action sequence as an ordered list, including provider reuse/creation, mapping creation, administrator request, and verification.
## Fix Focus Areas
- docs/guides/infrastructure/openai-workload-identity.md[104-138]
- docs/guides/infrastructure/openai-workload-identity.md[150-191]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| OpenAI decides whether to trust a run by comparing claims in the GitHub token with the mapping's | ||
| assertions. A wrong audience and a wrong claim fail the same way, so look at the real values before | ||
| writing the mapping (route A) or before asking for it (route B). Add this temporary workflow to | ||
| the repository, run it, and copy the printed claims. Put the provider's audience in `AUD` if you |
There was a problem hiding this comment.
3. Step 1 explains before acting 📜 Skill insight ✧ Quality
Step 1 begins with diagnostic explanation before telling the reader to add and run the temporary workflow. The required action should lead the step, with the explanation following it.
Agent Prompt
## Issue description
Step 1 explains claim matching before stating the action the reader must perform.
## Issue Context
Start the step with the action to add and run the temporary workflow, then explain why inspecting the claims is necessary and how to set `AUD`.
## Fix Focus Areas
- docs/guides/infrastructure/openai-workload-identity.md[55-62]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| fullsend workflow on a feature branch will not match `refs/heads/main`; that is expected. In the | ||
| org-wide installation mode the runs happen in the organization's central `.fullsend` repository, | ||
| so assert that repository instead. |
There was a problem hiding this comment.
4. Adr 0044 reference omitted 📘 Rule violation § Compliance
The PR removes documentation for deprecated org-wide installation mode, but the PR description does not reference ADR 0044. The checklist explicitly requires that reference even for removals of per-org mode content.
| **Is it safe to commit these?** Yes. They are identifiers, not secrets: on their own they grant | ||
| nothing. OpenAI issues a token only to a caller presenting a GitHub OIDC token whose claims match | ||
| the mapping from step 3, and only your repository's `main` workflow can obtain one. fullsend reads | ||
| a mapping, and only your repository's `main` workflow can obtain one. fullsend reads |
There was a problem hiding this comment.
5. Mapping trusts every main workflow 🐞 Bug ⛨ Security
The recommended mapping constrains only repository and ref, so any job in that repository running from main with id-token: write can obtain a matching GitHub token—not only the implied fullsend workflow. This overstates the mapping's security boundary and can cause administrators to overlook other OIDC-enabled workflows in the repository.
Agent Prompt
## Issue description
The guide says only the repository's `main` workflow can obtain a token, but the documented mapping does not assert a workflow identity. Any workflow job in that repository on `main` with `id-token: write` can request matching OIDC claims.
## Issue Context
Clarify that the mapping establishes repository-and-ref isolation, while workflow-level restriction requires an additional claim or another control. Keep the explanation consistent with the earlier discussion of `workflow_ref` and dispatch authorization.
## Fix Focus Areas
- docs/guides/infrastructure/openai-workload-identity.md[220-225]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| > repositories the company does not own, and a pattern would let every one of them obtain your | ||
| > token. Adding a repository later means adding its assertion; that is the gate, by design. | ||
|
|
||
| ## 1. See what your repository actually claims (both routes) |
There was a problem hiding this comment.
6. Troubleshooting uses obsolete steps 🐞 Bug ⚙ Maintainability
The route split renames the claims check from step 2 to step 1 and the mapping step from step 3 to A3/B2, but troubleshooting still tells readers to compare claims from “step 2” and narrow permissions in “step 3.” Those instructions now point to nonexistent sections and can send users to the wrong setup action.
Agent Prompt
## Issue description
The new route-specific numbering leaves two troubleshooting entries referring to the removed step 2 and step 3 labels.
## Issue Context
Point the claims comparison to step 1. Point permission narrowing to A3 for route A or to the administrator request in B2 for route B; links are preferable to bare step numbers.
## Fix Focus Areas
- docs/guides/infrastructure/openai-workload-identity.md[319-329]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
|
🤖 Finished Review · ✅ Success · Started 3:44 PM UTC · Completed 4:05 PM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $6.03 |
|
Risk Assessment: low (1/5) DetailsDocs-only follow-up PR by a known contributor updating an infrastructure guide and a runtime page; no protected paths, no CI/dependency/security-sensitive changes, and the linked implementation issue is already closed and merged. |
ReviewFindingsLow
Labels: PR modifies only documentation files (operator guide and runtime docs) Next steps:
|
![fullsend-ai-review[bot]](https://avatars.githubusercontent.com/in/3478501?s=60&v=4){kind=small}
There was a problem hiding this comment.
Note: The following review comments could not be posted on the diff (GitHub returned 422) and are included here instead:
docs/guides/infrastructure/openai-workload-identity.md(file-level): Line 254 · [low] removed troubleshooting guidance
The troubleshooting row for 'OpenAI WIF exchange failed: token endpoint returned 4xx' removed the diagnostic hint about PR-vs-main ref differences. The remaining body text in the 'Which runs this mapping trusts' section and step 1 provide the same information more accurately, and the old hint was partially misleading since PRs should match refs/heads/main (the job runs in the target repo).
| | Use uploaded JWKS for token verification | **Off** | | ||
|
|
||
| Copy the **identity provider ID**. One provider serves every repository; OpenAI allows 50 providers | ||
| per organization and 50 mappings per provider. |
There was a problem hiding this comment.
[low] removed guidance
Section A3 (route A) no longer explicitly warns against asserting workflow_ref in the 'Two things not to do' list. The old section 3 had a clear prohibition explaining that fullsend runs agents from seven workflow files, so a single workflow_ref value would exclude the others. In the new structure, this reasoning appears in step 1's explanation and in the B2 request template, but route A users creating mappings themselves lose the direct warning at the point where they fill in assertions.
Suggested fix: Add a brief 'Do not assert workflow_ref' note to the A3 section's 'Two things not to do' list, expanding it to three items.
Supersedes #6777, which rewrote the operator guide's setup half while this branch was being written. The guide read as if managing the organization's identity provider were a prerequisite; in most companies an IT administrator owns Organization Settings and the GitHub Actions provider, and a team owns (or requests) a project. It is now an explicit fork: shared claims check first, then route A (you manage providers) or route B (send the administrator a request), with common steps from "tell fullsend the three identifiers" on. Trust is stated as per repository for both routes: one mapping per company-owned repository, never a wildcard or a pattern over the organization, since an organization can contain repositories the company does not own. Mappings match exact values only and have no list form. The new commands are wired into that structure rather than bolted onto the old sections: route A points at `request` for the console fields, route B generates its ticket with `request --format md` (the hand-written template stays for readers who prefer it), B3 records the reply with `import` and checks it with `status`, and step 4 keeps a shortcut note. Also fixes the pi runtime doc's "three repository variables" wording. Assisted-by: Claude Signed-off-by: Wayne Sun <gsun@redhat.com>
|
🤖 Finished Retro · ✅ Success · Started 4:19 PM UTC · Completed 4:31 PM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $3.90 |
Retro: PR #6777 — docs(#6689): two setup routes and per-repository trust in the OpenAI WIF guideTimelineWayne Sun opened this docs-only PR at 15:35 UTC, immediately disabled the fix agent ( Workflow qualityReview quality was good. The fullsend review agent produced 2 sensible low-severity findings with no false positives: (1) a removed Cost was inflated by the known sonnet model failure. The review orchestrator dispatched 4 sub-agents with Evidence for existing issues
No new proposalsAll improvement opportunities identified are covered by existing open issues. The workflow operated correctly given its constraints — the review agent outperformed the third-party bot on signal quality, and the PR's short lifespan was a human workflow decision, not a system failure. |
1 participant
Summary
The operator guide
docs/guides/infrastructure/openai-workload-identity.mdpresented "you can manage Workload Identity Providers in your OpenAI organization" as a prerequisite. In most companies an IT administrator owns Organization Settings and the GitHub Actions identity provider, and a team owns (or requests) a project and a service account. The guide now has an explicit fork:fullsend://…value.repository_owner, a prefix or a derived attribute — a GitHub organization can contain repositories the company does not own, and a pattern would let them obtain the token. Adding a repository means adding its assertion; that is the gate.Docs only; no behaviour change. Follows up on #6695 / #6689.
Testing
make lint: markdown links, prettier, gitlint) on the commit.