Skip to content

ENG-7621: Pin instruction-file audit to App token - #141

Merged
nsportsman merged 1 commit into
mainfrom
eng-7621-app-token-pin
Sep 4, 2026
Merged

nsportsman merged 1 commit into
mainfrom
eng-7621-app-token-pin

Conversation

@nsportsman

Copy link
Copy Markdown
Collaborator

Summary

  • pin the instruction-file audit caller to public-workflows 522f02d44dc72b199da3d6b8db755fe3aeabff1d (v2.19.2)
  • forward PALATINE_SKILLS_APP_ID and PALATINE_SKILLS_PRIVATE_KEY instead of RUBRIC_TOKEN

Linear: ENG-7621

@nsportsman
nsportsman requested a review from a team as a code owner September 4, 2026 19:47
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Gemini Review

No critical issues — LGTM pending human review.


Reviewed by Gemini (gemini-3.1-pro-preview)

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Codex Review

No critical issues — LGTM pending human review.


Reviewed by Codex (gpt-5.5)

@coderabbitai

coderabbitai Bot commented Sep 4, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Team

Run ID: 409d8005-0909-4f17-b832-3bda430c8a15

📥 Commits

Reviewing files that changed from the base of the PR and between 63bab59 and 7ec0c11.

📒 Files selected for processing (1)
  • .github/workflows/agents-md-audit.yml

Included review availability: Your plan provides up to 10 included reviews per hour; 1 remains after this review.


Walkthrough

The audit workflow now references public-workflows v2.19.2. It replaces the RUBRIC_TOKEN secret with PALATINE_SKILLS_APP_ID and PALATINE_SKILLS_PRIVATE_KEY.

Merge Risk: ⚪ Minimal · up to 7ec0c

The audit workflow is updated to the intended public-workflows version and Palatine Skills application credentials with no current merge-blocking risk identified.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch eng-7621-app-token-pin

Comment @coderabbitai help to get the list of available commands.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Claude Review

Critical issues

  • The swap from RUBRIC_TOKEN to PALATINE_SKILLS_APP_ID / PALATINE_SKILLS_PRIVATE_KEY only works if the reusable workflow at the pinned SHA declares those exact secret names as inputs. Verify the secrets block in public-workflows agents-md-audit.yml at v2.19.2 matches these names, or the job fails at startup with an unmapped-secret error.
  • Confirm the two new secrets (PALATINE_SKILLS_APP_ID, PALATINE_SKILLS_PRIVATE_KEY) actually exist in this repo secret store; a missing secret resolves to empty and downstream App-token generation fails.

Security

  • Good: the caller stays pinned to a full commit SHA rather than a mutable tag. Worth double-checking that the new SHA genuinely corresponds to the v2.19.2 tag it claims in the trailing comment.
  • Moving to a GitHub App token (App ID + private key) is a reasonable improvement over a static RUBRIC_TOKEN, assuming the App is scoped least-privilege for the audit pull-requests:write need.

Test coverage
No production code changes; CI config only, no tests applicable.

@nsportsman

Copy link
Copy Markdown
Collaborator Author

REFUTED both Critical items.

  1. Callee public-workflows@522f02d (v2.19.2) declares on.workflow_call.secrets.PALATINE_SKILLS_APP_ID and PALATINE_SKILLS_PRIVATE_KEY as optional, and RUBRIC_TOKEN as optional deprecated. Passing these two names is valid; that SHA does not still require RUBRIC_TOKEN.
  2. Both org secrets are granted to this repository (visibility: selected). Unset-secret empty-string resolution is not this PR's runtime state.

No code change.

@nsportsman
nsportsman merged commit 73e09e3 into main Sep 4, 2026
18 checks passed
@nsportsman
nsportsman deleted the eng-7621-app-token-pin branch September 4, 2026 21:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant