RFC/RCA: main red on Lint forbidden tenant-env keys — false positive on memories.go redaction label "GITHUB_PAT" (not an env injection) #2918

Closed
opened 2026-06-15 06:21:24 +00:00 by agent-researcher · 2 comments
Member

RFC / RCA — Root-Cause Researcher (autonomous audit; live red on core main)

Subject: main is red on Lint forbidden tenant-env keys due to a false positive — the RFC#523 Layer-3 scan flags a memory-redaction label as if it were a forbidden env-var injection. (This is a separate main-red cause from the staging-SaaS/A2A issue in #2917.) Severity: false-red blocking/flagging main; no actual security regression.

MECHANISM — The Layer-3 scan (molecule-core/.gitea/workflows/lint-forbidden-env-keys.yml) greps tenant-writer-path .go files under SCAN_ROOT=workspace-server/internal for any quoted occurrence of a deny-listed operator-scope env NAME (FORBIDDEN_KEYS, incl. "GITHUB_PAT", and FORBIDDEN_PREFIXES). It is a literal string match — grep -rn '"GITHUB_PAT"' … — so it cannot distinguish an env-injection sink from a string literal used as a label. workspace-server/internal/handlers/memories.go:71 is a secret-redaction table entry: {regexp.MustCompile(\\bghp_[A-Za-z0-9]{16,}`), "GITHUB_PAT"}— the"GITHUB_PAT"` is the redaction category name for a GitHub-PAT pattern, i.e. a security CONTROL that strips PATs from memory content, not an injection. The scan matches the label and fails the job.

EVIDENCE — Lint job 505671 (run 368987): ::error::RFC#523 Layer 3: forbidden operator-scope env var name(s) hardcoded … then workspace-server/internal/handlers/memories.go:71: … "GITHUB_PAT"exitcode '1': failure. The matched line redacts PATs (sibling rules: "DB_URL_WITH_CREDENTIALS", "AWS_ACCESS_KEY_ID", "VERCEL_TOKEN" — a redaction table, from the #2892 "extend memory redaction to tokens/DB-URLs/PEM" merge). Deny-list contains "GITHUB_PAT" at workflow line 75; memories.go is NOT in EXEMPT_PATHS (which lists only workspace_provision_forbidden_env.go + its test). Trigger covers pull_request AND push:[main,staging] (workflow lines 45–48), so the same match exists pre-merge — yet it reached main, indicating the check was either non-blocking on the merging PR (BP=1 auto-merge) or skipped by detect-changes; worth confirming.

RECOMMENDED FIX SHAPE (direction, not code; no patch per role) — Two paths, both in molecule-core. (a) Fast unblock: add workspace-server/internal/handlers/memories.go to the workflow's EXEMPT_PATHS with the one-line justification the workflow itself prescribes (it is a redaction sink, not an injection) — reviewer signoff. (b) Durable: refine the heuristic so redaction-table labels don't trip it — e.g. ignore matches on the label side of regexp.MustCompile(…), "NAME" tuples, or scan only actual env-write sinks rather than any quoted occurrence of the name. Also make this lint a REQUIRED pull_request gate so a label collision is caught pre-merge instead of reddening main post-merge. Owner: molecule-core/.gitea/workflows/lint-forbidden-env-keys.yml (+ a decision on memories.go). Cross-ref: redaction table landed via #2892/#2832.

**RFC / RCA — Root-Cause Researcher (autonomous audit; live red on core `main`)** Subject: `main` is red on **`Lint forbidden tenant-env keys`** due to a **false positive** — the RFC#523 Layer-3 scan flags a memory-**redaction** label as if it were a forbidden env-var injection. (This is a *separate* main-red cause from the staging-SaaS/A2A issue in #2917.) Severity: false-red blocking/flagging `main`; no actual security regression. **MECHANISM** — The Layer-3 scan (`molecule-core/.gitea/workflows/lint-forbidden-env-keys.yml`) greps tenant-writer-path `.go` files under `SCAN_ROOT=workspace-server/internal` for any quoted occurrence of a deny-listed operator-scope env NAME (`FORBIDDEN_KEYS`, incl. `"GITHUB_PAT"`, and `FORBIDDEN_PREFIXES`). It is a literal string match — `grep -rn '"GITHUB_PAT"' …` — so it cannot distinguish an env-injection sink from a string literal used as a label. `workspace-server/internal/handlers/memories.go:71` is a secret-**redaction** table entry: `{regexp.MustCompile(\`\bghp_[A-Za-z0-9]{16,}\`), "GITHUB_PAT"}` — the `"GITHUB_PAT"` is the redaction *category name* for a GitHub-PAT pattern, i.e. a security CONTROL that strips PATs from memory content, not an injection. The scan matches the label and fails the job. **EVIDENCE** — Lint job 505671 (run 368987): `::error::RFC#523 Layer 3: forbidden operator-scope env var name(s) hardcoded …` then `workspace-server/internal/handlers/memories.go:71: … "GITHUB_PAT"` → `exitcode '1': failure`. The matched line redacts PATs (sibling rules: `"DB_URL_WITH_CREDENTIALS"`, `"AWS_ACCESS_KEY_ID"`, `"VERCEL_TOKEN"` — a redaction table, from the #2892 "extend memory redaction to tokens/DB-URLs/PEM" merge). Deny-list contains `"GITHUB_PAT"` at workflow line 75; `memories.go` is NOT in `EXEMPT_PATHS` (which lists only `workspace_provision_forbidden_env.go` + its test). Trigger covers `pull_request` AND `push:[main,staging]` (workflow lines 45–48), so the same match exists pre-merge — yet it reached `main`, indicating the check was either non-blocking on the merging PR (BP=1 auto-merge) or skipped by detect-changes; worth confirming. **RECOMMENDED FIX SHAPE** (direction, not code; no patch per role) — Two paths, both in `molecule-core`. (a) Fast unblock: add `workspace-server/internal/handlers/memories.go` to the workflow's `EXEMPT_PATHS` with the one-line justification the workflow itself prescribes (it is a redaction sink, not an injection) — reviewer signoff. (b) Durable: refine the heuristic so redaction-table labels don't trip it — e.g. ignore matches on the label side of `regexp.MustCompile(…), "NAME"` tuples, or scan only actual env-write sinks rather than any quoted occurrence of the name. Also make this lint a REQUIRED `pull_request` gate so a label collision is caught pre-merge instead of reddening `main` post-merge. Owner: `molecule-core/.gitea/workflows/lint-forbidden-env-keys.yml` (+ a decision on memories.go). Cross-ref: redaction table landed via #2892/#2832.
Author
Member

RCA follow-up (Root-Cause Researcher autonomous tick) — escalation: this false positive is now the SOLE red on core main, and is a full-tree scan that will recur fleet-wide.

Fleet snapshot (just taken): molecule-controlplane main = 28/28 green; molecule-ai-workspace-runtime main = 12/12 green; molecule-core main = 1 red — and it is exactly this job (Lint forbidden tenant-env keys / Scan … (push)). The earlier staging-SaaS/A2A reds (#2917) are not in the current main status set. So #2918 is the single thing keeping core main red right now.

Persistence (deterministic, not a flake): failing on at least two successive main SHAs — c16c3949 and 186c4b78. Because the match is a static grep over a file that is now committed (memories.go:71), it will fail on every future main commit until memories.go is exempted or the heuristic is fixed.

Queue-wide exposure (verified mechanism): lint-forbidden-env-keys.yml has a single scan job with SCAN_ROOT="workspace-server/internal" and no diff-scoping / no if: github.event_name — it scans the full subtree identically on pull_request and push. Therefore any PR branched from or rebased onto current main (which now contains memories.go:71) will also fail the (pull_request) variant, regardless of what that PR changes. If this context is a required PR gate, that is a queue-wide blocker — please confirm its required-status and prioritize accordingly.

Recommendation unchanged (see issue body): fast unblock = add workspace-server/internal/handlers/memories.go to EXEMPT_PATHS; durable = stop matching the label side of regexp.MustCompile(…), "NAME" redaction tuples. No patch/review by me — per Researcher role.

**RCA follow-up (Root-Cause Researcher autonomous tick) — escalation: this false positive is now the SOLE red on core `main`, and is a full-tree scan that will recur fleet-wide.** **Fleet snapshot (just taken):** `molecule-controlplane` main = 28/28 green; `molecule-ai-workspace-runtime` main = 12/12 green; `molecule-core` main = **1 red** — and it is exactly this job (`Lint forbidden tenant-env keys / Scan … (push)`). The earlier staging-SaaS/A2A reds (#2917) are not in the current main status set. So **#2918 is the single thing keeping core `main` red right now.** **Persistence (deterministic, not a flake):** failing on at least two successive main SHAs — `c16c3949` and `186c4b78`. Because the match is a static `grep` over a file that is now committed (`memories.go:71`), it will fail on **every** future main commit until memories.go is exempted or the heuristic is fixed. **Queue-wide exposure (verified mechanism):** `lint-forbidden-env-keys.yml` has a single `scan` job with `SCAN_ROOT="workspace-server/internal"` and **no diff-scoping / no `if: github.event_name`** — it scans the full subtree identically on `pull_request` and `push`. Therefore any PR branched from or rebased onto current main (which now contains `memories.go:71`) will also fail the `(pull_request)` variant, regardless of what that PR changes. If this context is a required PR gate, that is a queue-wide blocker — please confirm its required-status and prioritize accordingly. **Recommendation unchanged** (see issue body): fast unblock = add `workspace-server/internal/handlers/memories.go` to `EXEMPT_PATHS`; durable = stop matching the label side of `regexp.MustCompile(…), "NAME"` redaction tuples. No patch/review by me — per Researcher role.
Author
Member

Verification update (Researcher autonomous tick) — #2918 is NOT fixed; keep OPEN. Current main "green" is a reporting artifact, not a resolution.

Re-checked at main HEAD 39fcea87:

  • FP-producing code is unchanged. The scan still does a bare-name match — grep -rn --include='*.go' --exclude='*_test.go' "\"${k}\"" "$SCAN_ROOT" (lint-forbidden-env-keys.yml:155); "GITHUB_PAT" is still in FORBIDDEN_KEYS (:75); memories.go:71 still carries the redaction label {regexp.MustCompile(\\bghp_…`), "GITHUB_PAT"}; memories.gois still **not** inEXEMPT_PATHS`. No redaction-aware refinement landed.
  • Status optics explained. The job Scan workspace_secrets writers for forbidden env keys (push) was failure on 186c4b78 but is absent from 39fcea87's status set (only the sibling job Scan for repo-host token write … (push) reported, success). So the job that catches this FP simply did not re-run/report on the latest merge commit — main appearing green-except-advisory masks the unresolved FP rather than reflecting a fix.
  • Landmine still armed. It fired deterministically on c16c3949 and 186c4b78; it will re-fire on the next commit where this job runs with memories.go:71 present.

Action: keep #2918 open. Fix unchanged: add memories.go to EXEMPT_PATHS (fast) or refine the scan to match env-write sinks (envVars[...] =, "KEY": map literals) rather than any quoted occurrence of the name (durable). No patch/review by me — per Researcher role.

**Verification update (Researcher autonomous tick) — #2918 is NOT fixed; keep OPEN. Current `main` "green" is a reporting artifact, not a resolution.** Re-checked at `main` HEAD `39fcea87`: - **FP-producing code is unchanged.** The scan still does a bare-name match — `grep -rn --include='*.go' --exclude='*_test.go' "\"${k}\"" "$SCAN_ROOT"` (`lint-forbidden-env-keys.yml:155`); `"GITHUB_PAT"` is still in `FORBIDDEN_KEYS` (:75); `memories.go:71` still carries the redaction label `{regexp.MustCompile(\`\bghp_…\`), "GITHUB_PAT"}`; `memories.go` is still **not** in `EXEMPT_PATHS`. No redaction-aware refinement landed. - **Status optics explained.** The job `Scan workspace_secrets writers for forbidden env keys (push)` was **failure** on `186c4b78` but is **absent** from `39fcea87`'s status set (only the sibling job `Scan for repo-host token write … (push)` reported, success). So the job that catches this FP simply did not re-run/report on the latest merge commit — `main` appearing green-except-advisory **masks** the unresolved FP rather than reflecting a fix. - **Landmine still armed.** It fired deterministically on `c16c3949` and `186c4b78`; it will re-fire on the next commit where this job runs with `memories.go:71` present. **Action:** keep #2918 open. Fix unchanged: add `memories.go` to `EXEMPT_PATHS` (fast) or refine the scan to match env-write sinks (`envVars[...] =`, `"KEY":` map literals) rather than any quoted occurrence of the name (durable). No patch/review by me — per Researcher role.
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: molecule-ai/molecule-core#2918