Uh oh!
There was an error while loading. Please reload this page.
fix(ci): PAT-reachability fallback + pin-ref shape guard for the data-ingestors jobs - #326
Merged
Merged
Conversation
…-ingestors jobs Two Cursor Bugbot findings surfaced on the #323 promotion re-review of the data-ingestors drift jobs: 1. Invalid PAT breaks canary checkout (Medium). head-drift-canary.yml passed `secrets.CROSS_REPO_READ_TOKEN || github.token` straight to actions/checkout. `||` only falls back when the secret is EMPTY, so a set-but-expired / mis-scoped PAT reds the weekly canary even though data-ingestors is public and github.token would work. Ported goldens-drift.yml's reachability probe: a `git ls-remote` check picks the PAT only when it actually works and falls back to the default token otherwise (use_pat output, kept in lockstep with the checkout). goldens-drift.yml already had this probe. 2. Pin ref lacks shape guard (Low). goldens-drift.yml and head-drift-canary.yml read scripts/.data-ingestors-ref and (for goldens-drift) pass it straight to actions/checkout after only an emptiness check, unlike chart-drift.yml which validates ref shape. Added the same guard to both: SHA / branch / tag characters only and no "..", so a malformed pin fails with a clear error instead of an opaque checkout failure. Verified: the shape guard accepts the real pin, branch/tag/SHA refs, and rejects "..", shell metacharacters, and empty; the probe's no-token path selects github.token; actionlint + shellcheck clean; YAML valid. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
LukasWodka
commented
Jul 14, 2026
ContributorAuthor
@BugBot run |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 7e2c233. Configure here.
LukasWodka
commented
Jul 14, 2026
ContributorAuthor
👋 Heads-up — Code review queue is at 48 / 30 Above the WIP limit. The team convention is to review existing PRs before opening new work. Open PRs currently in Code review (oldest first):
Pull from review before opening new work. (This is a nudge from the kanban WIP check, not a block.) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Fixes two Cursor Bugbot findings surfaced on the #323 promotion re-review of the data-ingestors drift jobs. Both are pre-existing and advisory (neither is a required check), but worth closing.
1. Invalid PAT breaks canary checkout (Medium)
head-drift-canary.ymlpassedsecrets.CROSS_REPO_READ_TOKEN || github.tokenstraight toactions/checkout.||only falls back when the secret is empty, so a set-but-expired / mis-scoped PAT reds the weekly canary even thoughdata-ingestorsis public andgithub.tokenwould work.Fix: ported
goldens-drift.yml's reachability probe — agit ls-remotecheck selects the PAT only when it actually works and falls back to the default token otherwise (use_patoutput kept in lockstep with the checkout).goldens-drift.ymlalready had this; only the canary was missing it.2. Pin ref lacks shape guard (Low)
goldens-drift.ymlandhead-drift-canary.ymlreadscripts/.data-ingestors-refwith only an emptiness check — andgoldens-drift.ymlpasses it straight toactions/checkout— unlikechart-drift.yml, which validates ref shape. A malformed pin then fails opaquely at checkout.Fix: added
chart-drift.yml's guard to both pin-resolution steps — SHA / branch / tag characters only, no..— so a bad pin fails with a clear::errorinstead.Verification
develop, a 40-char SHA,release/v1.2.3; rejects../../etc/passwd,foo..bar,x;rm -rf /,$(whoami), empty.github.token.actionlint+shellcheckclean, YAML valid.Once merged to develop, #323 inherits it.
Note
Low Risk
CI-only workflow shell changes; no application auth, data paths, or runtime behavior.
Overview
Hardens the data-ingestors drift workflows so bad pins and broken cross-repo tokens fail predictably instead of deep in checkout.
Pin resolution in
goldens-drift.ymlandhead-drift-canary.ymlnow validatesscripts/.data-ingestors-refthe same way aschart-drift.yml: allowed ref characters only, no.., with a clear::errorbefore checkout uses the value.HEAD-drift canary no longer passes
CROSS_REPO_READ_TOKENto checkout whenever the secret is merely set. A newgit ls-remoteprobe picks the PAT only when it can read the repo; expired or mis-scoped PATs fall back togithub.token, matching the probe/checkout pairing already used ingoldens-drift.yml.Reviewed by Cursor Bugbot for commit 7e2c233. Bugbot is set up for automated code reviews on this repo. Configure here.