Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
52 changes: 52 additions & 0 deletions utils/tests/verify_action_build/test_security.py
Original file line number Diff line number Diff line change
Expand Up @@ -863,3 +863,55 @@ def test_split_alone_exempts(self):
"""
assert _file_is_pure_data_fetch(split_fetch) is True
assert _find_binary_downloads_js(split_fetch) == []

# The exact pattern that triggered the false positive on PR #789
# (rubygems/configure-rubygems-credentials, transitively pulled in by
# rubygems/release-gem): @actions/http-client postJson against an OIDC
# token-exchange endpoint. The response is a credential, not a binary.
RUBYGEMS_OIDC_EXCHANGE = """\
import * as core from '@actions/core'
import {HttpClient} from '@actions/http-client'
import {IdToken, IdTokenSchema} from './responses'

export async function exchangeToken(audience, server) {
const webIdentityToken = await core.getIDToken(audience)
const http = new HttpClient('rubygems-oidc-action')
const url = `${server}/api/v1/oidc/trusted_publisher/exchange_token`
const res = await http.postJson(url, {jwt: webIdentityToken}, {})
return res.result
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is the plan to handle each of the OIDC / Trusted publishing actions as they upgrade and fail?

Also, it would be good to think about tagging these as it is important to identify all of the Trusted publishing actions that PMCs are using.

"""

def test_postJson_token_exchange_exempted(self):
# @actions/http-client postJson is JSON-only — response is parsed as
# structured data, never persisted or executed.
assert _file_is_pure_data_fetch(self.RUBYGEMS_OIDC_EXCHANGE) is True
assert _find_binary_downloads_js(self.RUBYGEMS_OIDC_EXCHANGE) == []

def test_getJson_alone_exempts(self):
# Same family as postJson — getJson auto-parses the response body.
get_json = """\
import {HttpClient} from '@actions/http-client'
const http = new HttpClient('x')
const res = await http.getJson('https://api.example.com/v1/info')
return res.result
"""
assert _file_is_pure_data_fetch(get_json) is True
assert _find_binary_downloads_js(get_json) == []

def test_postJson_with_extract_in_same_file_not_exempt(self):
# If a JSON RPC call lives next to a real binary extraction in the
# same file, the binary-handle gate must still disable the exemption.
mixed = """\
import {HttpClient} from '@actions/http-client'
import * as tc from '@actions/tool-cache'

const http = new HttpClient('x')
const meta = await http.getJson('https://api.example.com/manifest')
const archive = await tc.downloadTool(meta.result.url)
await tc.extractTar(archive, dest)
"""
assert _file_is_pure_data_fetch(mixed) is False
findings = _find_binary_downloads_js(mixed)
# Both the getJson and the downloadTool stay flagged.
assert len(findings) == 2
5 changes: 5 additions & 0 deletions utils/verify_action_build/security.py
Original file line number Diff line number Diff line change
Expand Up @@ -1076,6 +1076,11 @@ def analyze_action_metadata(
re.compile(r"\.endsWith\s*\("),
re.compile(r"\.toLowerCase\s*\(\)"),
re.compile(r"\.toUpperCase\s*\(\)"),
# @actions/http-client JSON helpers (postJson/getJson/putJson/patchJson/
# delJson/requestJson) auto-parse the response body as JSON. Reaching for
# these is an explicit "treat the response as structured data" signal
# — typical of OIDC/RPC token-exchange calls, not binary downloads.
re.compile(r"\.(?:get|post|put|patch|del|request)Json\s*\("),
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This handles other generic OIDC token exchanges?

]

# Markers indicating the response is treated as a binary or executable —
Expand Down
Loading