-
Notifications
You must be signed in to change notification settings - Fork 0
fix(#1835): require file reads before asserting contents in findings #70
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.
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
base: main
Are you sure you want to change the base?
Changes from all commits
92e2d17
436a7f8
0a0561b
f19f1e3
32aaf9d
b5baa69
8a9681e
0b50f96
1f678e7
1881e3b
88ecef4
0013c75
893d1af
52dc9d2
b7b04f5
7d71e38
d330766
99ddc9d
fed552c
1d3da39
73dea45
63c27e4
ba99ae3
9a35c91
d4a394e
e492ac7
b2055cb
c48a832
3a44b0c
6f79d87
080368c
11bae49
e57f10a
602f75b
d1baca8
47e61b6
368890e
2e040b5
7c40a70
162dce2
80a414d
d2d2428
22c6e28
f126581
bbbb0b5
5fe6487
22be06d
7ecf899
61f467d
5e3d932
ecf5175
3305c1a
4c360c8
ac64c91
ded059b
78302ba
3c9f0db
7249b34
df020f5
3ae6f72
966abbf
65b155c
d988d32
e66f2d9
a24ffd1
d6988a9
515e49b
387968a
133ed6e
6832b14
32f73a4
b4d1c97
a9bd135
2b93fff
3fb219c
9241475
22d710d
25a286f
7905dfc
6f7ddf6
f322448
560ace4
7aef782
f902ef8
f4e19d5
f71504f
b04ecb3
ed8c416
b405b36
7993274
854d2e0
5c5e14d
c7ad026
6ac8e8f
d8c20b3
543d3ce
37ffc36
a4d5818
58c0e94
1077242
8dc0b93
33084f7
70ed5c1
2aaead0
2181382
3d54bc9
71601af
24fd33f
9806973
18846a2
25d4659
12b47a9
f01e246
e972b2c
fe94a21
6f20434
adba556
1dabdc6
14d5335
8b62249
fcd4101
ad57f0b
a39963f
a84bddf
773df28
874e0bb
2c94eab
241c5da
b73e233
72f1848
095039e
9ea24e8
57e807c
de9e17a
cf544d0
c8ea622
24f969e
bb406a3
1b69b0f
b4f6454
81848a5
5ce3e65
dce83dd
f77a94b
6cf0bb0
c5c248d
62926fc
6ce108d
ac47bf5
6371614
270ab1d
758c27d
3ed6080
c78c7d1
c30a531
e7f68c3
fee13a5
7077be2
d2856eb
b906210
71dc194
8e9ba31
5ee4955
36186df
a4a5008
8271187
1e985c9
47c8fdc
4d83c42
f112cad
6dbd59a
650261c
67376d4
725329c
a777a5d
559eb92
59159d0
0f5066f
05987d1
6005fea
b11081c
5f6d481
840b252
903478e
7c557e3
de8fe36
616fe5f
abc0f39
e99fd5f
6bec164
7f22940
e815459
858bd8c
abd7a49
fc4106b
aa78c67
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,18 @@ | ||
| #!/usr/bin/env bash | ||
| # Install the pinned OpenShell version via upstream install.sh. | ||
| # | ||
| # Sources openshell-version.sh for the version and commit SHA, then | ||
| # runs the upstream installer. Requires sudo for RPM installation. | ||
| # | ||
| # Usage: | ||
| # .github/scripts/install-openshell.sh | ||
| set -euo pipefail | ||
|
|
||
| SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)" | ||
| source "${SCRIPT_DIR}/openshell-version.sh" | ||
|
|
||
| echo "Installing OpenShell ${OPENSHELL_VERSION} (${OPENSHELL_SHA})" | ||
| curl -LsSf "https://raw.githubusercontent.com/NVIDIA/OpenShell/${OPENSHELL_SHA}/install.sh" \ | ||
| | OPENSHELL_VERSION="v${OPENSHELL_VERSION}" sh | ||
|
|
||
| openshell --version | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,20 @@ | ||
| #!/usr/bin/env bash | ||
| # Single source of truth for the pinned OpenShell version. | ||
| # | ||
| # Source this script to set OPENSHELL_VERSION and OPENSHELL_SHA in the | ||
| # current shell. In GitHub Actions it also exports them to GITHUB_ENV | ||
| # for downstream steps. | ||
| # | ||
| # Usage: | ||
| # source .github/scripts/openshell-version.sh | ||
|
|
||
| # renovate: datasource=github-tags depName=NVIDIA/OpenShell | ||
| OPENSHELL_VERSION=0.0.63 | ||
| OPENSHELL_SHA=ec197a43ef349e36c3fff04e9aaea9599fb83b31 | ||
|
|
||
| export OPENSHELL_VERSION OPENSHELL_SHA | ||
|
|
||
| if [[ -n "${GITHUB_ENV:-}" ]]; then | ||
| echo "OPENSHELL_VERSION=${OPENSHELL_VERSION}" >> "${GITHUB_ENV}" | ||
| echo "OPENSHELL_SHA=${OPENSHELL_SHA}" >> "${GITHUB_ENV}" | ||
| fi |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -9,6 +9,7 @@ permissions: {} | |
| on: | ||
| push: | ||
| branches: [main] | ||
| # SYNC-WITH: grep regex in "Check for e2e-relevant changes" step in the e2e job | ||
| paths: | ||
| - '**/*.go' | ||
| - 'go.mod' | ||
|
|
@@ -24,19 +25,6 @@ on: | |
| - 'scripts/check-e2e-authorization.sh' | ||
| pull_request_target: | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [high] secrets exposure / workflow attack surface expansion The diff removes the paths: filter from the pull_request_target trigger, causing the e2e workflow to trigger on ALL pull_request_target events. The pull_request_target event is security-sensitive (accesses secrets, id-token: write). The runtime file-relevance check falls through as fail-open on gh api failure. Suggested fix: Consider keeping a broad paths: filter as a declarative first-pass and refining with the runtime check. If removal is intentional, document the rationale. |
||
| types: [opened, synchronize, reopened, labeled] | ||
| paths: | ||
| - '**/*.go' | ||
| - 'go.mod' | ||
| - 'go.sum' | ||
| - 'e2e/**' | ||
| - 'internal/scaffold/fullsend-repo/**' | ||
| - 'internal/security/hooks/**' | ||
| - 'internal/dispatch/gcf/mintsrc/**' | ||
| - 'internal/sentencetoken/english.json' | ||
| - 'Makefile' | ||
| - '.github/workflows/e2e.yml' | ||
| - '.github/actions/check-e2e-authorization/**' | ||
| - 'scripts/check-e2e-authorization.sh' | ||
| merge_group: | ||
| workflow_dispatch: | ||
|
|
||
|
|
@@ -93,19 +81,44 @@ jobs: | |
| contents: read | ||
| id-token: write | ||
| steps: | ||
| - name: Check for e2e-relevant changes | ||
| id: changes | ||
| if: github.event_name == 'pull_request_target' | ||
| env: | ||
| GH_TOKEN: ${{ github.token }} | ||
| PR_NUMBER: ${{ github.event.pull_request.number }} | ||
| REPO: ${{ github.repository }} | ||
| # SYNC-WITH: push.paths filter above | ||
| run: | | ||
| FILES=$(gh api "repos/${REPO}/pulls/${PR_NUMBER}/files" --paginate --jq '.[].filename') || { | ||
| echo "::warning::Failed to fetch PR files — running e2e tests as a precaution" | ||
| echo "relevant=true" >> "$GITHUB_OUTPUT" | ||
| exit 0 | ||
| } | ||
| if echo "$FILES" | grep -qE '\.go$|^go\.(mod|sum)$|^e2e/|^internal/scaffold/fullsend-repo/|^internal/security/hooks/|^internal/dispatch/gcf/mintsrc/|^internal/sentencetoken/english\.json$|^Makefile$|^\.github/workflows/e2e\.yml$|^\.github/actions/check-e2e-authorization/|^scripts/check-e2e-authorization\.sh$'; then | ||
| echo "relevant=true" >> "$GITHUB_OUTPUT" | ||
| else | ||
| echo "::notice::No e2e-relevant files changed — skipping tests" | ||
| echo "relevant=false" >> "$GITHUB_OUTPUT" | ||
| fi | ||
|
|
||
| - uses: actions/checkout@v4 | ||
| if: steps.changes.outputs.relevant != 'false' | ||
| with: | ||
| ref: ${{ github.event_name == 'pull_request_target' && github.event.pull_request.head.sha || github.sha }} | ||
| persist-credentials: false | ||
|
|
||
| - uses: actions/setup-go@v5 | ||
| if: steps.changes.outputs.relevant != 'false' | ||
| with: | ||
| go-version-file: go.mod | ||
|
|
||
| - name: Install Playwright system dependencies | ||
| if: steps.changes.outputs.relevant != 'false' | ||
| run: npx playwright install-deps chromium | ||
|
|
||
| - name: Check for secrets | ||
| if: steps.changes.outputs.relevant != 'false' | ||
| id: secrets-check | ||
| run: | | ||
| if [ -z "$E2E_GITHUB_SESSION_B64" ]; then | ||
|
|
@@ -118,7 +131,7 @@ jobs: | |
| E2E_GITHUB_SESSION_B64: ${{ secrets.E2E_GITHUB_SESSION }} | ||
|
|
||
| - name: Decode session | ||
| if: steps.secrets-check.outputs.available == 'true' | ||
| if: steps.changes.outputs.relevant != 'false' && steps.secrets-check.outputs.available == 'true' | ||
| run: | | ||
| SESSION_FILE="${RUNNER_TEMP}/github-session.json" | ||
| printf '%s' "$E2E_GITHUB_SESSION_B64" | base64 -d > "$SESSION_FILE" | ||
|
|
@@ -127,14 +140,14 @@ jobs: | |
| E2E_GITHUB_SESSION_B64: ${{ secrets.E2E_GITHUB_SESSION }} | ||
|
|
||
| - name: Authenticate to GCP | ||
| if: steps.secrets-check.outputs.available == 'true' | ||
| if: steps.changes.outputs.relevant != 'false' && steps.secrets-check.outputs.available == 'true' | ||
| uses: google-github-actions/auth@v2 | ||
| with: | ||
| workload_identity_provider: ${{ secrets.E2E_GCP_WIF_PROVIDER }} | ||
| service_account: ${{ secrets.E2E_GCP_SERVICE_ACCOUNT }} | ||
|
|
||
| - name: Run e2e tests | ||
| if: steps.secrets-check.outputs.available == 'true' | ||
| if: steps.changes.outputs.relevant != 'false' && steps.secrets-check.outputs.available == 'true' | ||
| run: make e2e-test | ||
| env: | ||
| E2E_SCREENSHOT_DIR: ${{ runner.temp }}/e2e-screenshots | ||
|
|
@@ -144,7 +157,7 @@ jobs: | |
| E2E_GCP_PROJECT_ID: ${{ secrets.E2E_GCP_PROJECT_ID }} | ||
|
|
||
| - name: Upload debug screenshots | ||
| if: always() && steps.secrets-check.outputs.available == 'true' | ||
| if: always() && steps.changes.outputs.relevant != 'false' && steps.secrets-check.outputs.available == 'true' | ||
| uses: actions/upload-artifact@v4 | ||
| with: | ||
| name: e2e-screenshots-${{ github.event_name == 'pull_request_target' && github.event.pull_request.number || github.run_id }} | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -54,6 +54,7 @@ jobs: | |
| uses: actions/checkout@v6 | ||
|
|
||
| - name: Checkout upstream defaults | ||
| if: hashFiles('.defaults/action.yml', '.fullsend/.defaults/action.yml') == '' | ||
| uses: actions/checkout@v6 | ||
| with: | ||
| repository: fullsend-ai/fullsend | ||
|
|
@@ -100,6 +101,7 @@ jobs: | |
| mkdir -p .github/scripts | ||
| cp "${SRC}/.github/scripts/setup-agent-env.sh" .github/scripts/setup-agent-env.sh | ||
|
|
||
|
|
||
| - name: Validate enrollment and extract repo metadata | ||
| id: repo-parts | ||
| uses: ./.defaults/.github/actions/validate-enrollment | ||
|
|
@@ -145,12 +147,10 @@ jobs: | |
| ORIGINATING_URL: ${{ fromJSON(inputs.event_payload).pull_request.html_url || fromJSON(inputs.event_payload).issue.html_url }} | ||
| RETRO_COMMENT: ${{ fromJSON(inputs.event_payload).comment.body || '' }} | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [low] secrets handling The diff removes GH_TOKEN from the env block. The post-retro.sh script requires GH_TOKEN. Verify the mint-url flow provides GH_TOKEN to post-scripts. Suggested fix: Confirm fullsend run with --mint-url correctly populates GH_TOKEN for post-scripts. |
||
| REPO_FULL_NAME: ${{ inputs.source_repo }} | ||
| RETRO_SANDBOX_TOKEN: ${{ steps.app-token.outputs.token }} | ||
| GH_TOKEN: ${{ steps.app-token.outputs.token }} | ||
| with: | ||
| agent: retro | ||
| version: ${{ inputs.fullsend_version }} | ||
| run-url: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }} | ||
| status-repo: ${{ inputs.source_repo }} | ||
| status-number: ${{ fromJSON(inputs.event_payload).pull_request.number || fromJSON(inputs.event_payload).issue.number }} | ||
| status-token: ${{ steps.app-token.outputs.token }} | ||
| mint-url: ${{ inputs.mint_url }} | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[medium] curl-pipe-sh remote code execution
The new script fetches and executes a remote install script via curl | sh. While the commit SHA is pinned, there is no integrity verification (checksum) of the downloaded script content before execution.
Suggested fix: Add SHA-256 checksum verification of the downloaded install script before piping to sh.