#47 - Fix slack_webhook_url GPG — empty string breaks Salt pillar - forgejo_admin/pal-e-platform

forgejo_admin commented

2026-03-14 16:18:55 +00:00

Owner

Summary

Salt GPG renderer fails on PLAINTEXT_LENGTH 0 (empty string decryption)
slack_webhook_url was encrypted from echo -n '' in PR #45
Entire secrets:platform pillar returns empty, breaking make tofu-secrets
Fix: re-encrypt with 'unused' as the dormant placeholder value

Changes

salt/pillar/secrets/platform.sls — replaced slack_webhook_url GPG block with non-empty value

Terraform Changes

No infra changes. Fixes make tofu-secrets rendering (was returning 0 secrets, now returns 16).

Test Plan

sudo salt-call pillar.get secrets:platform --out=json returns 16 secrets (verified locally)
make tofu-secrets renders all 15 TF vars

Review Checklist

No plaintext secret values in committed files
GPG block encrypted with correct key ID

Constraints

Salt GPG renderer cannot handle PLAINTEXT_LENGTH 0 — never encrypt empty strings into pillar

PR #45 — introduced the empty-string GPG block
arch-secrets-pipeline — private architecture doc
sop-secrets-management — procedures

Closes #46

Discovered Scope

Add to sop-secrets-management: "Never encrypt empty strings — Salt GPG renderer treats PLAINTEXT_LENGTH 0 as failure. Use a placeholder like 'unused' for dormant secrets."

## Summary - Salt GPG renderer fails on PLAINTEXT_LENGTH 0 (empty string decryption) - `slack_webhook_url` was encrypted from `echo -n ''` in PR #45 - Entire `secrets:platform` pillar returns empty, breaking `make tofu-secrets` - Fix: re-encrypt with `'unused'` as the dormant placeholder value ## Changes - `salt/pillar/secrets/platform.sls` — replaced `slack_webhook_url` GPG block with non-empty value ## Terraform Changes No infra changes. Fixes `make tofu-secrets` rendering (was returning 0 secrets, now returns 16). ## Test Plan - [x] `sudo salt-call pillar.get secrets:platform --out=json` returns 16 secrets (verified locally) - [ ] `make tofu-secrets` renders all 15 TF vars ## Review Checklist - [x] No plaintext secret values in committed files - [x] GPG block encrypted with correct key ID ## Constraints - Salt GPG renderer cannot handle PLAINTEXT_LENGTH 0 — never encrypt empty strings into pillar ## Related - PR #45 — introduced the empty-string GPG block - `arch-secrets-pipeline` — private architecture doc - `sop-secrets-management` — procedures Closes #46 ## Discovered Scope - Add to `sop-secrets-management`: "Never encrypt empty strings — Salt GPG renderer treats PLAINTEXT_LENGTH 0 as failure. Use a placeholder like 'unused' for dormant secrets."

forgejo_admin added 1 commit

2026-03-14 16:18:55 +00:00

Fix slack_webhook_url GPG — encrypt 'unused' not empty string 379f3d3a09

Salt GPG renderer treats PLAINTEXT_LENGTH 0 as decryption failure.
Empty string encryption produces a valid GPG block that decrypts
to nothing, which Salt rejects. Use 'unused' as the dormant value.

All 16 pillar secrets now decrypt successfully.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

forgejo_admin commented

2026-03-14 16:20:21 +00:00

Author

Owner

PR #47 Review

BLOCKERS

1. Terraform conditional guard will break with 'unused' placeholder

The Terraform code in terraform/main.tf uses var.slack_webhook_url != "" as a guard to conditionally enable the Slack Alertmanager receiver (lines 168, 190, 300). With the GPG block now decrypting to "unused" instead of "", this guard evaluates to true, which means:

A Slack route is created for severity=~"critical|warning" alerts (line 168-173)
A Slack receiver is created (line 190-202)
api_url is set to "unused" via set_sensitive (line 300-304)

Alertmanager will attempt to POST alerts to "unused" as a URL and fail. This is a functional regression -- the old empty-string value (when the whole pillar worked) correctly disabled Slack routing via the != "" guard.

Fix options:

(a) Change the Terraform guard to something like var.slack_webhook_url != "" && var.slack_webhook_url != "unused" (or use startswith("https://"))
(b) Use a different placeholder that still passes Salt GPG decryption but evaluates as "disabled" in Terraform (e.g., a single space, if Salt GPG accepts that)
(c) Keep "unused" in Salt but update the TF conditional to match

2. secrets_registry.sls description is stale

salt/pillar/secrets_registry.sls line 107 still says "dormant — empty string" and line 111 says "Empty string = dormant". The value is now "unused", not an empty string. The registry should be updated to match.

NITS

The PR body's Test Plan shows one item unchecked: [ ] make tofu-secrets renders all 15 TF vars. Given that this is the exact pipeline the fix is meant to repair, running this to completion before merge would provide confidence. (Though the salt-call pillar.get verification is the more critical one and is checked.)
The Discovered Scope section correctly identifies the SOP update needed for sop-secrets-management. Good practice.
The PR body says "returns 16 secrets" in one place and "renders all 15 TF vars" in another. The discrepancy is fine (16 pillar keys vs 15 TF vars could mean one pillar key is not mapped to TF), but worth a quick sanity check that the counts are intentional.

SOP COMPLIANCE

Branch named after issue -- Branch is hotfix-slack-webhook-empty-string, not 46-*. Hotfix naming is reasonable for urgency, but SOP expects issue-number prefix.
PR body has: Summary, Changes, Test Plan, Related
Related section references plan context (PR #45 origin, architecture docs)
Closes #46 present in PR body
No secrets, .env files, or credentials committed -- only GPG-encrypted blocks, no plaintext
No unnecessary file changes -- exactly 1 file changed, only the slack_webhook_url GPG block replaced (14 lines removed, 14 lines added). No other secrets touched.
Commit messages are descriptive (PR title is clear)

VERDICT: NOT APPROVED

The core fix (re-encrypting from 'unused' instead of empty string) correctly solves the Salt GPG PLAINTEXT_LENGTH 0 failure. However, the downstream Terraform conditional logic will silently break: Slack routing will activate with an invalid URL. This must be addressed before merge -- either by updating the TF guard or choosing a placeholder value that the existing != "" guard handles correctly. The stale registry description should also be updated in the same PR.

## PR #47 Review ### BLOCKERS **1. Terraform conditional guard will break with `'unused'` placeholder** The Terraform code in `terraform/main.tf` uses `var.slack_webhook_url != ""` as a guard to conditionally enable the Slack Alertmanager receiver (lines 168, 190, 300). With the GPG block now decrypting to `"unused"` instead of `""`, this guard evaluates to `true`, which means: - A Slack route is created for `severity=~"critical|warning"` alerts (line 168-173) - A Slack receiver is created (line 190-202) - `api_url` is set to `"unused"` via `set_sensitive` (line 300-304) Alertmanager will attempt to POST alerts to `"unused"` as a URL and fail. This is a functional regression -- the old empty-string value (when the whole pillar worked) correctly disabled Slack routing via the `!= ""` guard. **Fix options:** - (a) Change the Terraform guard to something like `var.slack_webhook_url != "" && var.slack_webhook_url != "unused"` (or use `startswith("https://")`) - (b) Use a different placeholder that still passes Salt GPG decryption but evaluates as "disabled" in Terraform (e.g., a single space, if Salt GPG accepts that) - (c) Keep `"unused"` in Salt but update the TF conditional to match **2. `secrets_registry.sls` description is stale** `salt/pillar/secrets_registry.sls` line 107 still says `"dormant — empty string"` and line 111 says `"Empty string = dormant"`. The value is now `"unused"`, not an empty string. The registry should be updated to match. ### NITS 1. The PR body's Test Plan shows one item unchecked: `[ ] make tofu-secrets renders all 15 TF vars`. Given that this is the exact pipeline the fix is meant to repair, running this to completion before merge would provide confidence. (Though the `salt-call pillar.get` verification is the more critical one and is checked.) 2. The Discovered Scope section correctly identifies the SOP update needed for `sop-secrets-management`. Good practice. 3. The PR body says "returns 16 secrets" in one place and "renders all 15 TF vars" in another. The discrepancy is fine (16 pillar keys vs 15 TF vars could mean one pillar key is not mapped to TF), but worth a quick sanity check that the counts are intentional. ### SOP COMPLIANCE - [ ] Branch named after issue -- Branch is `hotfix-slack-webhook-empty-string`, not `46-*`. Hotfix naming is reasonable for urgency, but SOP expects issue-number prefix. - [x] PR body has: Summary, Changes, Test Plan, Related - [x] Related section references plan context (PR #45 origin, architecture docs) - [x] `Closes #46` present in PR body - [x] No secrets, .env files, or credentials committed -- only GPG-encrypted blocks, no plaintext - [x] No unnecessary file changes -- exactly 1 file changed, only the `slack_webhook_url` GPG block replaced (14 lines removed, 14 lines added). No other secrets touched. - [x] Commit messages are descriptive (PR title is clear) ### VERDICT: NOT APPROVED The core fix (re-encrypting from `'unused'` instead of empty string) correctly solves the Salt GPG PLAINTEXT_LENGTH 0 failure. However, the downstream Terraform conditional logic will silently break: Slack routing will activate with an invalid URL. This must be addressed before merge -- either by updating the TF guard or choosing a placeholder value that the existing `!= ""` guard handles correctly. The stale registry description should also be updated in the same PR.

forgejo_admin added 1 commit

2026-03-14 16:21:30 +00:00

Fix Slack guard for 'unused' placeholder + update registry 0e447c45d3

QA caught: var.slack_webhook_url != "" evaluates true with 'unused',
creating a broken Slack receiver. Add '&& var.slack_webhook_url != "unused"'
guard to all 3 conditional checks (lines 168, 190, 300).

Update secrets_registry.sls to reflect 'unused' instead of 'empty string'.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>