Skip to content

NO-ISSUE: Add failure isolation to Apply configuration workflow #169

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

Merged
eliorerz merged 2 commits into from
Aug 5, 2026
Merged

NO-ISSUE: Add failure isolation to Apply configuration workflow #169

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ce79636
Add failure isolation to Apply configuration workflow
eliorerz Aug 5, 2026
3c9f428
Trim whitespace in state_rm_addresses parsing too
eliorerz Aug 5, 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
86 changes: 84 additions & 2 deletions .github/workflows/apply.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,10 @@ on:
description: "One-time: comma-separated Terraform resource addresses to remove from state (e.g. for a member who left the org and whose resource can no longer be refreshed). Leave empty for a normal apply."
required: false
default: ""
exclude_addresses:
description: "One-time: comma-separated Terraform resource addresses to exclude from this apply (e.g. a resource that's known-broken and blocking every other pending change atomically, while a permanent fix is prepared). Leave empty for a normal apply."
required: false
default: ""
schedule:
- cron: "17 */4 * * *"
push:
Expand Down Expand Up @@ -100,17 +104,95 @@ jobs:
exit 1
fi
IFS=',' read -ra ADDRS <<< "${STATE_RM_ADDRESSES}"
for addr in "${ADDRS[@]}"; do
for i in "${!ADDRS[@]}"; do
addr="${ADDRS[$i]}"
# Trim surrounding whitespace, since "addr1, addr2" (space after
# the comma) is the natural way to type this list by hand.
addr="${addr#"${addr%%[![:space:]]*}"}"
addr="${addr%"${addr##*[![:space:]]}"}"
if [[ -z "${addr}" || "${addr}" == -* ]]; then
echo "::error::invalid resource address '${addr}' -- addresses must be non-empty and cannot start with '-' (tofu would parse it as an option)." >&2
exit 1
fi
ADDRS[$i]="${addr}"
done
tofu state rm "${ADDRS[@]}"
env:
STATE_RM_ADDRESSES: ${{ inputs.state_rm_addresses }}
# A one-time, manual escape hatch: a single broken/unrefreshable
# resource aborts this entire apply atomically (see #162, #165 for two
# real instances), silently blocking every other repo's pending
# changes until someone happens to notice. -exclude lets a maintainer
# immediately unblock everyone else while the permanent fix for the
# broken resource is prepared, without giving up dependency-complete
# single-pass apply for the normal case. A no-op for the normal
# scheduled/push triggers, which never set this input.
#
# A permanent per-module `-target` loop was considered and rejected:
# several modules share cross-module resources (e.g.
# github_team.all["wg-infra"], referenced by ruleset_bypass_team_ids
# in multiple repo modules), so looping per module would re-plan/
# re-apply those shared resources on every iteration that references
# them -- wasteful, and it gives up Terraform's normal whole-graph
# dependency ordering for no real isolation benefit in the common
# case where nothing is broken. -exclude only sacrifices ordering
# guarantees for the specific resources a maintainer has deliberately
# chosen to skip, one time -- note that -exclude also skips anything
# that depends on the excluded resource, so it unblocks everything
# *not* downstream of the broken one, not literally everything else.
- name: TF Apply
id: tofu_apply
run: |
tofu apply -concise -auto-approve
EXCLUDE_ARGS=()
if [[ -n "${EXCLUDE_ADDRESSES}" ]]; then
if [[ "${EXCLUDE_ADDRESSES}" == *$'\n'* ]]; then
echo "::error::exclude_addresses must be comma-separated on a single line, not newline-separated." >&2
exit 1
fi
IFS=',' read -ra ADDRS <<< "${EXCLUDE_ADDRESSES}"
for addr in "${ADDRS[@]}"; do
# Trim surrounding whitespace, since "addr1, addr2" (space after
# the comma) is the natural way to type this list by hand.
addr="${addr#"${addr%%[![:space:]]*}"}"
addr="${addr%"${addr##*[![:space:]]}"}"
if [[ -z "${addr}" || "${addr}" == -* ]]; then
echo "::error::invalid resource address '${addr}' -- addresses must be non-empty and cannot start with '-' (tofu would parse it as an option)." >&2
exit 1
fi
EXCLUDE_ARGS+=("-exclude=${addr}")
done
fi
Comment on lines +146 to +164

@coderabbitai coderabbitai Bot Aug 5, 2026 •
edited
Loading

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -eu
file=".github/workflows/apply.yaml"
printf '%s\n' "--- workflow excerpt ---"
sed -n '1,125p' "$file"
printf '%s\n' "--- exclusion references ---"
rg -n -C 2 'EXCLUDE_ADDRESSES|exclude-file|exclude=' "$file" || true
printf '%s\n' "--- action references ---"
rg -n '^[[:space:]]*-?[[:space:]]*uses:' "$file" || true
printf '%s\n' "--- Bash splitting probe ---"
EXCLUDE_ADDRESSES='aws_instance.web["a,b"], aws_instance.other'
IFS=',' read -ra ADDRS <<< "${EXCLUDE_ADDRESSES}"
for addr in "${ADDRS[@]}"; do
  printf '<%s>\n' "$addr"
done

Repository: osac-project/github-config

Length of output: 6805

Fix exclusion parsing and pin workflow actions.

  • IFS=',' splits valid addresses such as aws_instance.web["a,b"] and preserves leading spaces in values such as aws_a, aws_b. Use a quote-aware parser or newline-delimited addresses with -exclude-file.
  • Pin actions/checkout@v7, actions/create-github-app-token@v3, and opentofu/setup-opentofu@v2 to full commit SHAs. This job handles AWS credentials and an App private key.
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/apply.yaml around lines 92 - 98, Update the EXCLUDE_ARGS
parsing in the workflow to preserve commas inside quoted addresses and trim
unintended surrounding spaces, using a quote-aware or newline-delimited approach
with -exclude-file where appropriate. Pin actions/checkout@v7,
actions/create-github-app-token@v3, and opentofu/setup-opentofu@v2 to their full
immutable commit SHAs.

@eliorerz eliorerz Aug 5, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Trimming fixed in 0837f15 (handles the common "addr1, addr2" case). Not switching to -exclude-file though: that only changes how I invoke tofu, it doesn't fix the comma-inside-brackets ambiguity in how I split the input string, which would need a different input format entirely (e.g. newline-separated). No address in this repo's actual config contains a literal comma inside a for_each key today, so I don't think that larger change is justified for a manual, human-operated one-time escape hatch -- noted the limitation explicitly in the commit message. Action SHA pinning: same reply as the other thread, pre-existing pattern across the whole file.

@coderabbitai coderabbitai Bot Aug 5, 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.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- Terraform for_each declarations ---'
rg -n -C 3 --glob '*.tf' '\bfor_each\s*=' . || true

printf '%s\n' '--- Literal commas inside quoted Terraform strings ---'
rg -n --glob '*.tf' '(["'\''])[^"'\'']*,[^"'\'']*\1' . || true

printf '%s\n' '--- Indexed references with quoted comma-containing keys ---'
rg -n --glob '*.tf' '\[[^]]*["'\''][^"'\'']*,[^"'\'']*["'\''][^]]*\]' . || true

Length of output: 10140

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- CSV files ---'
fd --type f --extension csv .

printf '%s\n' '--- CSV fields containing literal commas ---'
python3 - <<'PY'
import csv
from pathlib import Path

for path in sorted(Path(".").rglob("*.csv")):
    with path.open(newline="", encoding="utf-8") as source:
        for row_number, row in enumerate(csv.DictReader(source), start=2):
            for field, value in row.items():
                if value is not None and "," in value:
                    print(f"{path}:{row_number}: field={field!r} value={value!r}")
PY

printf '%s\n' '--- Terraform quoted index expressions containing literal commas ---'
rg -nP --glob '*.tf' '\[[^]\r\n]*["'\''][^"'\'']*,[^"'\'']*["'\''][^]\r\n]*\]' . || true

Length of output: 3179

@eliorerz, verified. The CSV data contains no literal commas. The current for_each key sources therefore do not produce an address with a comma in its quoted index.

I withdraw the parsing portion of the finding. The current comma-separated manual input is adequate for this repository and this one-time use case.

The action SHA pinning concern remains separate and valid.

✏️ Learnings added 
Learnt from: eliorerz
Repo: osac-project/github-config PR: 169
File: .github/workflows/apply.yaml:95-113
Timestamp: 2026-08-05T22:33:57.616Z
Learning: In `osac-project/github-config`, the Terraform `for_each` keys used for resource addresses derive from current CSV data or fixed values. The current CSV data contains no literal commas, so `.github/workflows/apply.yaml` can use a comma-separated `exclude_addresses` input for its documented one-time manual escape hatch. The workflow trims surrounding whitespace and rejects newline-separated, empty, and option-like entries.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

You are interacting with an AI system.

tofu apply -concise -auto-approve "${EXCLUDE_ARGS[@]}"
env:
GITHUB_TOKEN: ${{ steps.generate-token.outputs.token }}
EXCLUDE_ADDRESSES: ${{ inputs.exclude_addresses }}
Comment on lines 143 to +168

@coderabbitai coderabbitai Bot Aug 5, 2026 •
edited
Loading

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -eu
file=".github/workflows/apply.yaml"
printf '%s\n' "Action references:"
rg -n 'uses:' "$file"
printf '%s\n' "Workflow context:"
sed -n '1,115p' "$file"

Repository: osac-project/github-config

Length of output: 5458

Security Misconfiguration (CWE-829): Inclusion of Functionality from Untrusted Control Sphere

Reachability: External · Exploitability: Moderate

Pin all third-party actions to full commit SHAs.

Replace actions/checkout@v7, actions/create-github-app-token@v3, and opentofu/setup-opentofu@v2 with reviewed full commit-SHA references.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/apply.yaml around lines 90 - 102, Update the workflow’s
third-party action references for actions/checkout,
actions/create-github-app-token, and opentofu/setup-opentofu to reviewed
immutable full commit SHA pins, preserving their current major-version behavior
and configuration.

Source: Path instructions

@eliorerz eliorerz Aug 5, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Skipping — this is a pre-existing pattern across the entire file (all three uses: lines predate this PR, which doesn't add or touch any). Fixing it here would be inconsistent with the rest of the file; better addressed as its own dedicated hardening PR touching all of them at once.

@coderabbitai coderabbitai Bot Aug 5, 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.

@eliorerz, acknowledged. The SHA-pinning finding remains valid, but it is outside the scope of this PR. A dedicated hardening PR that updates all workflow action references consistently is appropriate.

Would you like me to create a follow-up GitHub issue for the action-SHA pinning work?

ᕱ⑅ᕱ

You are interacting with an AI system.

# tofu apply isn't atomic and doesn't roll back changes already
# applied earlier in the same run if a later resource fails -- but a
# hard error on one resource does still stop the run before anything
# after it in the plan is attempted, which is what actually blocked
# every other repo's pending changes here (the host-management-
# openstack archived-repo bug, #165) with no signal beyond a red
# Actions run. File or update a tracking issue so a stuck apply is
# never silent again. Scoped to TF Apply's own outcome specifically
# (not the broader failure() built-in, which would also match an
# earlier checkout/init/validate/import failure).
- name: File an issue on apply failure
if: ${{ failure() && steps.tofu_apply.outcome == 'failure' }}
env:
# Only gh + GITHUB_TOKEN are needed here -- explicitly clear the
# job-level AWS backend credentials rather than let this step
# inherit them unnecessarily.
AWS_ACCESS_KEY_ID: ""
AWS_SECRET_ACCESS_KEY: ""
GITHUB_TOKEN: ${{ steps.generate-token.outputs.token }}
RUN_URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
run: |
gh label create apply-failure --repo "${{ github.repository }}" \
--color d73a4a --description "tofu apply is failing" --force
EXISTING=$(gh issue list --repo "${{ github.repository }}" --label apply-failure --state open --json number --jq '.[0].number')
BODY="\`tofu apply\` failed at ${RUN_URL}. This blocks *every* repo's pending Terraform changes until resolved -- see the run log for which resource caused it."
if [[ -n "${EXISTING}" ]]; then
gh issue comment "${EXISTING}" --repo "${{ github.repository }}" --body "${BODY}"
else
gh issue create --repo "${{ github.repository }}" --title "Apply configuration is failing" --label apply-failure --body "${BODY}"
Comment thread
coderabbitai[bot] marked this conversation as resolved.
fi
Loading