Skip to content

ci(NOJIRA-1234): extend bot PR automerge to smartling and aikido - #171

Merged
Jlougedo-TF merged 7 commits into
mainfrom
NOJIRA-1234/bot-pr-automerge
Oct 7, 2026
Merged

Jlougedo-TF merged 7 commits into
mainfrom
NOJIRA-1234/bot-pr-automerge

Conversation

@Jlougedo-TF

Copy link
Copy Markdown
Contributor

Gate on the PR author login instead of a single actor, so translation PRs from smartling-github-connector[bot] and security fixes from aikido-autofix[bot] are auto-approved and auto-merged alongside dependabot.

Titles are not a reliable signal for these bots, so the allowlist keys off github.actor only.

Gate on the PR author login instead of a single actor, so translation PRs
from smartling-github-connector[bot] and security fixes from
aikido-autofix[bot] are auto-approved and auto-merged alongside dependabot.

Titles are not a reliable signal for these bots, so the allowlist keys off
github.actor only.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@Jlougedo-TF Jlougedo-TF self-assigned this Sep 9, 2026
@Jlougedo-TF
Jlougedo-TF requested a review from a team as a code owner September 9, 2026 13:29
@pr-auditor

pr-auditor Bot commented Sep 9, 2026 •

Copy link
Copy Markdown

⚠️ Security Analysis Results

🟡 1 medium · 1 files reviewed

🟡 Medium severity

1. business_logic · .github/workflows/dependabot-automerge.yml:50 · confidence 4/5

The newly added aikido-specific risk check (rule 6) only flags a PR as risky when it touches a file whose basename falls outside the dependency-manifest/lockfile allowlist (package.json, lockfiles, go.mod/sum, requirements files). Rule 2, which inspects the content of package.json changes, still only looks for 'resolutions'/'overrides'/'**/' glob entries — it does not detect additions or modifications to the 'scripts' field (e.g. preinstall/postinstall/prepare hooks). Because package.json is explicitly on the 'safe' basename list used by rule 6, a PR that only edits package.json to add a malicious script hook passes every heuristic (not a major-version title, no override/glob pattern, not a bulk sweep, no lockfile churn, and the only touched file is an allowed manifest) and is marked risky=false.

Exploit: If the aikido-autofix[bot] token/app is compromised or manipulated (e.g. via a crafted upstream vulnerability report it 'auto-fixes') into opening a PR that only modifies package.json to insert a 'postinstall': 'curl ... | sh' script, none of the six heuristics fire, and the workflow auto-approves the PR and enables auto-merge with a write-permission GITHUB_TOKEN, landing a supply-chain backdoor in main with zero human review.

Fix: Extend the content-based check in rule 2 (or add a new rule) to flag any addition/modification of the package.json 'scripts', 'bin', or similar executable-hook fields as risky for any actor other than dependabot[bot], regardless of whether the touched file is otherwise an 'allowed' manifest/lockfile.

Resolved Issues (1)
Status Category File Details
Fixed broken_trust_boundary .github/workflows/dependabot-automerge.yml Rule 6 now flags any aikido-autofix[bot] PR touching files outside the known dependency-manifest/lockfile allowlist as risky, dismissing any prior approval, disabling auto-merge, labeling needs-human, and requiring manual review instead of auto-merging. This directly addresses the previously flagged scenario of workflow/source-code modifications bypassing review.

@pr-auditor rescan to re-run · Powered by Claude Sonnet 5 · Docs · #security-engineering-team

Jlougedo-TF and others added 5 commits September 9, 2026 15:36
This repo has no Smartling-managed content: the smartling-github-connector[bot]
has never opened a PR here and there is no Smartling config. Narrow the
allowlist to the bots that actually raise PRs in this repo.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A green CI run does not prove a transitive dependency bump is safe: the
repo's own tests never exercise how the intermediate package uses the changed
API. Approve as before, but only arm auto-merge when the diff looks routine.

Held back for a human when any of these match:
  - the bot's title declares a major version upgrade
  - a JS manifest touches resolutions/overrides (a forced transitive pin)
  - go.mod gains a +incompatible major bump
  - more than 6 manifest dependency lines change at once
  - the lockfile rewrite exceeds 600 lines

Validated against 13 real bot PRs: correctly holds xfiles#543 (docker v24->v25
+incompatible), blocks#3039 (major axios), renderer#1481 and mail-composer#400
(forced resolutions), and correctly passes the single direct minor bumps such
as embed#760, pages#620 and purgatory#314.

Also drops the checkout and 'apt-get install gh' steps: nothing read the working
tree (gh is API-only) and gh ships on ubuntu-latest. All repos now hold a
byte-identical file apart from the allowlist line.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Assess risk first, then approve only when the diff looks routine. A risky PR
now gets no approval at all, so it cannot satisfy the required-review count and
a human has to sign it off - the label alone was advisory, since a bot approval
already met the review requirement.

Also close the stale-arming gap: a PR can open looking routine (approved,
auto-merge armed) and then be force-pushed into something risky. On the risky
path the workflow now calls 'gh pr merge --disable-auto' and dismisses its own
earlier approval before labelling and commenting.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@Jlougedo-TF

Copy link
Copy Markdown
Contributor Author

Addressed the aikido finding: for aikido-autofix[bot] the workflow now holds (no approval, no auto-merge, needs-human label) any PR touching a file that is not a dependency manifest or lockfile (package.json, yarn.lock, package-lock.json, pnpm-lock.yaml, go.mod, go.sum, requirements*.txt/.in). Backtested on 72 historical Aikido PRs: 0 false positives.

@pr-auditor rescan

@Jlougedo-TF
Jlougedo-TF merged commit 38ffecd into main Oct 7, 2026
4 of 5 checks passed
@Jlougedo-TF
Jlougedo-TF deleted the NOJIRA-1234/bot-pr-automerge branch October 7, 2026 14:21
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.

2 participants