Skip to content

Commit b7d15ee

Browse files
anandgupta42claude
andauthored
fix: harden dbt review action version lookup and credential write (#911)
Follow-up hardening for the `github/review` composite action from the #900 review. Stacks on #900 (refines its new semver version step). - Authenticate the release-version lookup with `${{ github.token }}` (lifts the unauthenticated 60 req/hr IP limit to 1,000 req/hr) so busy runners aren't throttled into the `latest` fallback. - Skip the binary cache when the version resolves to `latest` (`if: steps.version.outputs.version != 'latest'`), so one rate-limited/offline lookup can't pin a stale binary across all later runs. - Read the hosted API key from the environment inside the `jq` program (`$ENV.IN_ALT_KEY`) instead of passing it via `--arg`, keeping it out of `argv` (visible to other processes; printed under `ACTIONS_STEP_DEBUG`). - Add 4 adversarial tests: auth header present with a token, omitted+safe without one (bash-3.2 empty-array idiom), cache gated on `!= 'latest'`, and the API key absent from the `jq` argv. Closes #909 Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 3572f1c commit b7d15ee

3 files changed

Lines changed: 136 additions & 20 deletions

File tree

CHANGELOG.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,10 +12,16 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1212
- **The `github/review` composite action can be downloaded by GitHub Actions again.** The release tree contained three VS Code image symlinks whose removed targets caused GitHub's action downloader to reject the entire archive before the review step started. The images are now self-contained files and a release-critical test prevents dangling links from returning.
1313
- **A valid dbt manifest is no longer mislabeled as a lint-only run.** Manifest availability is now checked independently from changed-model lookup, so new models and other valid manifests receive the correct full-run status.
1414

15+
- **The release-version lookup is rate-limit resilient and never caches a floating `latest`.** The composite action now authenticates its GitHub release-API call with the workflow token (lifting the 60→1,000 req/hr unauthenticated limit) and skips the binary cache entirely when the version resolves to `latest`, so a single rate-limited or offline lookup can no longer pin a stale binary across all subsequent runs.
16+
1517
### Added
1618

1719
- **Direct GitHub onboarding and a live dbt review demo.** The GitHub App installer now opens GitHub's repository-selection screen directly, and the README/docs link to the public `dbt-pr-review-demo` pull requests.
1820

21+
### Security
22+
23+
- **The hosted Altimate API key is no longer placed on the `jq` process arg list.** The credential write reads the key from the environment inside the `jq` program, keeping it out of `argv` (which is visible to other processes and printed verbatim when `ACTIONS_STEP_DEBUG` enables `set -x`).
24+
1925
## [0.8.4] - 2026-06-05
2026

2127
A trace-durability patch. Open `/traces` mid-session and you'd see a rich waterfall — then the moment the agent finished its turn the view collapsed to a single "system-prompt" span, the Summary tab's *"What was asked"* showed *"No prompt recorded"*, and the Chat tab dropped every user turn but the last. The data was genuinely gone from disk, not just hidden in the viewer. This release stops the on-disk trace from being overwritten after each turn and makes the file authoritative across worker restarts. A five-persona pre-release review drove a follow-up wording fix so a reconstructed trace isn't misread as a failed run.

github/review/action.yml

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -59,13 +59,22 @@ runs:
5959
shell: bash
6060
env:
6161
ACTION_REF: ${{ github.action_ref }}
62+
# Authenticated API calls lift the unauthenticated 60 req/hr IP limit to
63+
# 1,000 req/hr, so a busy org's runners don't get rate-limited into the
64+
# "latest" fallback below.
65+
GITHUB_TOKEN: ${{ github.token }}
6266
run: |
6367
set -euo pipefail
6468
if [[ "${ACTION_REF:-}" =~ ^v?([0-9]+\.[0-9]+\.[0-9]+)$ ]]; then
6569
VERSION="${BASH_REMATCH[1]}"
6670
else
71+
# Only attach the auth header when a token is present; the
72+
# ${AUTH[@]+"${AUTH[@]}"} idiom expands to nothing on an empty array
73+
# even under `set -u` (portable back to bash 3.2).
74+
AUTH=()
75+
[[ -n "${GITHUB_TOKEN:-}" ]] && AUTH=(-H "Authorization: Bearer ${GITHUB_TOKEN}")
6776
VERSION=$(
68-
curl -sf --connect-timeout 5 --max-time 15 \
77+
curl -sf --connect-timeout 5 --max-time 15 ${AUTH[@]+"${AUTH[@]}"} \
6978
https://api.github.com/repos/AltimateAI/altimate-code/releases/latest \
7079
| grep -o '"tag_name": *"[^"]*"' | cut -d'"' -f4 || true
7180
)
@@ -75,6 +84,11 @@ runs:
7584
7685
- name: Cache altimate-code
7786
id: cache
87+
# Never cache against the floating "latest" key: a single rate-limited or
88+
# offline lookup would otherwise pin whatever binary it grabbed and reuse
89+
# it forever, silently ignoring later releases. A resolved semver caches
90+
# normally; "latest" falls through to a fresh install each run.
91+
if: steps.version.outputs.version != 'latest'
7892
uses: actions/cache@v4
7993
with:
8094
path: ~/.altimate/bin
@@ -120,11 +134,13 @@ runs:
120134
fi
121135
umask 077
122136
mkdir -p "$HOME/.altimate"
137+
# Read the API key from the environment inside the jq program rather
138+
# than passing it via --arg: argv is visible to other processes and
139+
# is printed verbatim if a user enables ACTIONS_STEP_DEBUG (set -x).
123140
jq -nc \
124141
--arg url "${IN_ALT_URL:-https://api.myaltimate.com}" \
125142
--arg inst "$IN_ALT_INSTANCE" \
126-
--arg key "$IN_ALT_KEY" \
127-
'{altimateUrl:$url, altimateInstanceName:$inst, altimateApiKey:$key}' \
143+
'{altimateUrl:$url, altimateInstanceName:$inst, altimateApiKey:$ENV.IN_ALT_KEY}' \
128144
> "$HOME/.altimate/altimate.json"
129145
echo "Advisory lane: hosted altimate model (tenant: $IN_ALT_INSTANCE)."
130146
elif [[ -n "${IN_MODEL:-}" ]]; then

packages/opencode/test/skill/release-v0.8.5-adversarial.test.ts

Lines changed: 111 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -61,10 +61,7 @@ describe("v0.8.5 adversarial - composite action", () => {
6161
const output = path.join(tmp.path, "github-output")
6262
const curlMarker = path.join(tmp.path, "curl-called")
6363
await fs.mkdir(bin)
64-
await Bun.write(
65-
path.join(bin, "curl"),
66-
`#!/usr/bin/env bash\ntouch "$CURL_MARKER"\nexit 99\n`,
67-
)
64+
await Bun.write(path.join(bin, "curl"), `#!/usr/bin/env bash\ntouch "$CURL_MARKER"\nexit 99\n`)
6865
await fs.chmod(path.join(bin, "curl"), 0o755)
6966

7067
const result = await runBash(await actionScript("Get altimate-code version"), {
@@ -76,7 +73,12 @@ describe("v0.8.5 adversarial - composite action", () => {
7673

7774
expect(result.exitCode).toBe(0)
7875
expect(await fs.readFile(output, "utf8")).toBe("version=0.8.5\n")
79-
expect(await fs.stat(curlMarker).then(() => true).catch(() => false)).toBe(false)
76+
expect(
77+
await fs
78+
.stat(curlMarker)
79+
.then(() => true)
80+
.catch(() => false),
81+
).toBe(false)
8082
})
8183

8284
test("the non-semver action ref fallback bounds the release lookup", async () => {
@@ -113,10 +115,7 @@ describe("v0.8.5 adversarial - composite action", () => {
113115
const capture = path.join(tmp.path, "args")
114116
const sentinel = path.join(tmp.path, "injected")
115117
await fs.mkdir(bin)
116-
await Bun.write(
117-
path.join(bin, "altimate"),
118-
`#!/usr/bin/env bash\nprintf '%s\\0' "$@" > "$CAPTURE"\n`,
119-
)
118+
await Bun.write(path.join(bin, "altimate"), `#!/usr/bin/env bash\nprintf '%s\\0' "$@" > "$CAPTURE"\n`)
120119
await fs.chmod(path.join(bin, "altimate"), 0o755)
121120

122121
const manifest = `target/manifest $(touch ${sentinel}) .json`
@@ -138,7 +137,12 @@ describe("v0.8.5 adversarial - composite action", () => {
138137
})
139138

140139
expect(result.exitCode).toBe(0)
141-
expect(await fs.stat(sentinel).then(() => true).catch(() => false)).toBe(false)
140+
expect(
141+
await fs
142+
.stat(sentinel)
143+
.then(() => true)
144+
.catch(() => false),
145+
).toBe(false)
142146
const args = (await fs.readFile(capture, "utf8")).split("\0").filter(Boolean)
143147
expect(args).toEqual([
144148
"review",
@@ -197,9 +201,12 @@ describe("v0.8.5 adversarial - composite action", () => {
197201
expect(result.exitCode).toBe(1)
198202
expect(result.stdout + result.stderr).not.toContain(secret)
199203
expect(result.stdout + result.stderr).toContain("altimate_instance is empty")
200-
expect(await fs.stat(path.join(tmp.path, ".altimate/altimate.json")).then(() => true).catch(() => false)).toBe(
201-
false,
202-
)
204+
expect(
205+
await fs
206+
.stat(path.join(tmp.path, ".altimate/altimate.json"))
207+
.then(() => true)
208+
.catch(() => false),
209+
).toBe(false)
203210
})
204211

205212
test("the committed archive contains regular, non-empty action dependencies", async () => {
@@ -225,15 +232,102 @@ describe("v0.8.5 adversarial - composite action", () => {
225232
const version = changelog.match(/^## \[(\d+\.\d+\.\d+)\] - Unreleased$/m)?.[1]
226233
expect(version).toBe("0.8.5")
227234

228-
for (const relative of [
229-
"docs/docs/usage/dbt-pr-review.md",
230-
"github/review/examples/altimate-ingestion.yml",
231-
]) {
235+
for (const relative of ["docs/docs/usage/dbt-pr-review.md", "github/review/examples/altimate-ingestion.yml"]) {
232236
const content = await fs.readFile(path.join(repoRoot, relative), "utf8")
233237
expect(content).toContain(`AltimateAI/altimate-code/github/review@v${version}`)
234238
expect(content).not.toContain("AltimateAI/altimate-code/github/review@v0.8.4")
235239
}
236240
})
241+
242+
test("the release lookup authenticates the api when a github token is present", async () => {
243+
await using tmp = await tmpdir()
244+
const bin = path.join(tmp.path, "bin")
245+
const output = path.join(tmp.path, "github-output")
246+
const argsPath = path.join(tmp.path, "curl-args")
247+
await fs.mkdir(bin)
248+
await Bun.write(
249+
path.join(bin, "curl"),
250+
`#!/usr/bin/env bash\nprintf '%s\\0' "$@" > "$CURL_ARGS"\nprintf '{"tag_name":"v0.8.5"}\\n'\n`,
251+
)
252+
await fs.chmod(path.join(bin, "curl"), 0o755)
253+
254+
const result = await runBash(await actionScript("Get altimate-code version"), {
255+
ACTION_REF: "main",
256+
GITHUB_TOKEN: "ghs-test-token",
257+
CURL_ARGS: argsPath,
258+
GITHUB_OUTPUT: output,
259+
PATH: `${bin}:${process.env.PATH}`,
260+
})
261+
262+
expect(result.exitCode, result.stderr).toBe(0)
263+
const curlArgs = (await fs.readFile(argsPath, "utf8")).split("\0").filter(Boolean)
264+
expect(curlArgs).toContain("-H")
265+
expect(curlArgs).toContain("Authorization: Bearer ghs-test-token")
266+
})
267+
268+
test("the release lookup omits auth and still succeeds when no token is present", async () => {
269+
await using tmp = await tmpdir()
270+
const bin = path.join(tmp.path, "bin")
271+
const output = path.join(tmp.path, "github-output")
272+
const argsPath = path.join(tmp.path, "curl-args")
273+
await fs.mkdir(bin)
274+
await Bun.write(
275+
path.join(bin, "curl"),
276+
`#!/usr/bin/env bash\nprintf '%s\\0' "$@" > "$CURL_ARGS"\nprintf '{"tag_name":"v0.8.5"}\\n'\n`,
277+
)
278+
await fs.chmod(path.join(bin, "curl"), 0o755)
279+
280+
// An empty token must not crash under `set -u` (the ${AUTH[@]+...} idiom)
281+
// and must not attach an Authorization header.
282+
const result = await runBash(await actionScript("Get altimate-code version"), {
283+
ACTION_REF: "main",
284+
GITHUB_TOKEN: "",
285+
CURL_ARGS: argsPath,
286+
GITHUB_OUTPUT: output,
287+
PATH: `${bin}:${process.env.PATH}`,
288+
})
289+
290+
expect(result.exitCode, result.stderr).toBe(0)
291+
expect(await fs.readFile(output, "utf8")).toBe("version=0.8.5\n")
292+
const curlArgs = (await fs.readFile(argsPath, "utf8")).split("\0").filter(Boolean)
293+
expect(curlArgs).not.toContain("-H")
294+
expect(curlArgs.some((arg) => arg.startsWith("Authorization:"))).toBe(false)
295+
})
296+
297+
test("the cache step is skipped for the floating latest version", async () => {
298+
const action = YAML.parse(await fs.readFile(actionPath, "utf8"))
299+
const cache = action.runs.steps.find((step: { name?: string }) => step.name === "Cache altimate-code")
300+
expect(cache?.if).toBe("steps.version.outputs.version != 'latest'")
301+
})
302+
303+
test("the hosted credential write keeps the api key out of the jq argv", async () => {
304+
await using tmp = await tmpdir()
305+
const bin = path.join(tmp.path, "bin")
306+
const argsPath = path.join(tmp.path, "jq-args")
307+
const secret = "argv-secret-must-not-appear"
308+
await fs.mkdir(bin)
309+
// Fake jq captures its argv (and writes nothing), so we can assert the
310+
// secret is delivered via the environment, not the process arg list.
311+
await Bun.write(path.join(bin, "jq"), `#!/usr/bin/env bash\nprintf '%s\\0' "$@" > "$JQ_ARGS"\n`)
312+
await fs.chmod(path.join(bin, "jq"), 0o755)
313+
314+
const result = await runBash(await actionScript("Configure advisory reviewer model + credentials"), {
315+
HOME: tmp.path,
316+
GITHUB_ENV: path.join(tmp.path, "github-env"),
317+
JQ_ARGS: argsPath,
318+
IN_ALT_KEY: secret,
319+
IN_ALT_INSTANCE: "demo",
320+
IN_ALT_URL: "https://api.example.test",
321+
IN_MODEL: "",
322+
IN_MODEL_API_KEY: "",
323+
PATH: `${bin}:${process.env.PATH}`,
324+
})
325+
326+
expect(result.exitCode, result.stderr).toBe(0)
327+
const jqArgs = (await fs.readFile(argsPath, "utf8")).split("\0").filter(Boolean)
328+
expect(jqArgs.some((arg) => arg.includes(secret))).toBe(false)
329+
expect(jqArgs).toContain("{altimateUrl:$url, altimateInstanceName:$inst, altimateApiKey:$ENV.IN_ALT_KEY}")
330+
})
237331
})
238332

239333
describe("v0.8.5 end-to-end - real review pipeline", () => {

0 commit comments

Comments
 (0)