Skip to content

RTECO-1411 - Implement integration tests for Agent Plugins #3543

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
udaykb2 wants to merge 4 commits into from
Closed

RTECO-1411 - Implement integration tests for Agent Plugins #3543

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
5a9c96e
RTECO-1411 - Implement integration tests for Agent Plugins
udaykb2 Jun 14, 2026
3ab6df8
temp: add pull_request trigger for testing
udaykb2 Jun 14, 2026
907b4d9
Add more test scenarios
udaykb2 Jun 16, 2026
e8e099d
Merge branch 'master' into RTECO-1411-Agentplugins-tests
udaykb2 Jun 17, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 56 additions & 0 deletions .github/workflows/agentPluginsTests.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
name: Agent Plugins Tests
on:
workflow_dispatch:
push:
branches:
- "master"
# Triggers the workflow on PRs to master branch only.
pull_request:
branches:
- "master"
pull_request_target:

@Udaysonu Udaysonu Jun 17, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[CRITICAL] Security: pull_request_target + forked HEAD checkout + secrets

This is the classic "pwn-request" attack vector. pull_request_target runs with write permissions and exposes RTLIC and FASTCI_TOKEN to the workflow. Checking out the forked PR's HEAD SHA (line 38) means a malicious PR author can execute arbitrary code with access to those secrets.

The safe to test label guard reduces but does not eliminate the risk — a maintainer applying the label without scrutinising every changed file still grants secret access.

Safe pattern: for pull_request_target events, always check out github.sha (the base branch), never the fork head SHA. Or use a repository-dispatch approach where a separate, secret-free job validates the label then triggers a dispatch event.

types: [labeled]
branches:
- "master"

# Ensures that only the latest commit is running for each PR at a time.
concurrency:
group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.sha }}-${{ github.ref }}
cancel-in-progress: true

jobs:
Agent-Plugins-Tests:
name: agent-plugins ${{ matrix.os.name }}
if: github.event_name == 'workflow_dispatch' || github.event_name == 'push' || github.event_name == 'pull_request' || contains(github.event.pull_request.labels.*.name, 'safe to test')
strategy:
fail-fast: false
matrix:
os:
- name: ubuntu
version: 24.04
- name: windows
version: 2022
runs-on: ${{ matrix.os.name }}-${{ matrix.os.version }}
steps:
- name: Checkout code
uses: actions/checkout@v5
with:
ref: ${{ github.event.pull_request.head.sha || github.ref }}

@Udaysonu Udaysonu Jun 17, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[CRITICAL] Checking out forked PR HEAD under pull_request_target

When triggered by pull_request_target with the labeled event type, github.event.pull_request.head.sha resolves to the fork's commit. Combined with access to repository secrets (RTLIC), this is a confirmed secret-exfiltration vector (see GitHub Security Advisory GHSA-p2g7-xwvr-rrw3 and related). See also the comment on line 11.

@Udaysonu Udaysonu Jun 17, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Critical — Security: "pwn request" vulnerability

pull_request_target runs with repository write permissions and access to secrets (e.g. RTLIC). This step checks out the PR contributor's HEAD code (github.event.pull_request.head.sha), meaning any PR author can embed a payload that exfiltrates secrets when a maintainer applies the safe to test label.

Fix options:

  1. Only checkout github.ref (the base branch) under pull_request_target — never the PR's HEAD.
  2. Replace pull_request_target with pull_request, which has no write permissions and no access to secrets, and add the safe to test label guard only if needed for external fork isolation.


- name: Setup FastCI
uses: jfrog-fastci/fastci@v1
with:
github_token: ${{ secrets.GITHUB_TOKEN }}
fastci_otel_token: ${{ secrets.FASTCI_TOKEN }}

- name: Setup Go with cache
uses: jfrog/.github/actions/install-go-with-cache@main

@Udaysonu Udaysonu Jun 17, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Minor — Unpinned @main action references

jfrog/.github/actions/install-go-with-cache@main (and install-local-artifactory@main two lines below) use mutable refs. A breaking push to the main branch of that repo will silently break this workflow without any PR or diff. Pin to a versioned tag or a commit SHA for reproducibility.


- name: Install local Artifactory
uses: jfrog/.github/actions/install-local-artifactory@main
with:
RTLIC: ${{ secrets.RTLIC }}
RT_CONNECTION_TIMEOUT_SECONDS: ${{ env.RT_CONNECTION_TIMEOUT_SECONDS || '1200' }}

- name: Run agent plugins tests
run: go test -v github.com/jfrog/jfrog-cli --timeout 0 --test.agentPlugins

@Udaysonu Udaysonu Jun 17, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[CRITICAL] --timeout 0 disables Go test timeout entirely

If any test hangs (network stall, deadlock, missing prerequisite), the runner job blocks indefinitely and starves other PRs waiting for a runner slot.

Suggested: Replace --timeout 0 with a bounded value, e.g. --timeout 45m. Other CLI integration test jobs in this repo can be used as a reference for appropriate values.

@Udaysonu Udaysonu Jun 17, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Nit — --timeout 0 disables test timeout entirely

If a test deadlocks or hangs on a network call, this job runs forever and requires manual cancellation. A generous but finite value (e.g. --timeout 2h) ensures the runner recovers automatically.

Loading
Loading