Skip to content

bug(infrastructure): 56 shell scripts source lib/ssot-config.sh — the file does not exist, so every one silently runs on hardcoded fallbacks #14041

Closed

Description

@mrveiss

Found while reviewing PR #14037 (#13076). Verified directly.

The finding

56 shell scripts under autobot-infrastructure/ contain a line of this shape:

source "${SCRIPT_DIR}/lib/ssot-config.sh" 2>/dev/null || true

lib/ssot-config.sh does not exist anywhere in the repository:

$ find . -name "ssot-config.sh" -not -path "./.worktrees/*"
(no results)

$ grep -rln "ssot-config.sh" --include=*.sh autobot-infrastructure/ | wc -l
56

Because of 2>/dev/null || true, the failure is silent. Every one of those scripts continues, and every configurable value in them resolves to its literal fallback — ${AUTOBOT_SOMETHING:-/some/hard/path} always takes the right-hand side.

Why this matters more than the individual hardcodes

This is the mechanism the hardcode cleanup keeps assuming exists. A script that reads:

source "${SCRIPT_DIR}/lib/ssot-config.sh" 2>/dev/null || true
websockify --web "${AUTOBOT_NOVNC_PATH:-/opt/novnc}" ...

looks config-driven in review and in a diff. It is not. The env var is only ever set if the caller happened to export it; the SSOT helper that was supposed to populate it was never shipped.

So:

The || true is what makes it invisible: a missing SSOT source is treated as a normal, expected condition.

Suggested approach

Two parts, in order:

  1. Decide whether the helper should exist. Either write autobot-infrastructure/shared/scripts/lib/ssot-config.sh so it actually exports the values these scripts reference, or accept that shell scripts read config from the environment only and delete the misleading source lines. Either is defensible; the current state is the one that is not.
  2. Stop it being silent. If the helper is meant to exist, sourcing it should fail loudly when missing — || true on a config bootstrap turns a deployment error into a wrong-value-at-runtime. If it is not meant to exist, remove the lines rather than leaving 56 dead references.

A guard is worth adding either way: a check that every source .../lib/*.sh in the tree resolves to a real file.

Acceptance criteria

  • lib/ssot-config.sh either exists and exports the values the 56 scripts reference, or the dead source lines are removed
  • A missing config bootstrap fails loudly rather than continuing on fallbacks
  • A repo check flags any source of a non-existent file under autobot-infrastructure/
  • The scripts whose ${VAR:-literal} forms were previously inert are listed, so their values can be re-reviewed as genuine hardcodes if the helper is not built

Related

Activity

  1. mrveiss commented on Aug 12, 2026

    @mrveiss
    OwnerAuthor

    Decision — build it, not delete it

    This was parked as an owner decision: build lib/ssot-config.sh, or remove the 56 dead source lines.

    Taking the first, because the project's standing rule already answers it: anything that looks like debris is unfinished work — complete and wire it in, never delete. Deletion is correct only where the work has demonstrably landed elsewhere, and here nothing has: 56 scripts source a file that does not exist, so every ${VAR:-literal} fallback in them is inert and each script is silently running on its hardcoded default.

    Removing the source lines would make that permanent and invisible. Building the library makes 56 scripts start honouring the SSOT they already believe they are reading.

    That is also the risk to handle carefully. Today every one of those scripts uses its literal default. The moment the library exists, they start taking real values — so this is not a no-op refactor, it is 56 scripts changing behaviour at once. The implementation needs to enumerate what each ${VAR:-literal} would resolve to and flag every case where the SSOT value differs from the literal, before it lands.

    Unlabelling needs-decision. Queued behind #14151 and #14124.

  2. mrveiss commented on Aug 12, 2026

    @mrveiss
    OwnerAuthor

    The library landed in 526586c68 on Dev_new_gui via PR #14174. Leaving this open — two of the original acceptance criteria are deferred to #14172 and #14173, and partial delivery does not close an issue.

    Verified in base

    autobot-infrastructure/shared/scripts/lib/ssot-config.sh          exists
    bootstrap-slm.sh:27   _OPERATOR_REDIS_HOST="${AUTOBOT_REDIS_HOST:-}"
    bootstrap-slm.sh:29   INFRA_ROOT="${PROJECT_ROOT}/autobot-infrastructure"
    bootstrap-slm.sh:415  REDIS_HOST=${_OPERATOR_REDIS_HOST:?AUTOBOT_REDIS_HOST is required ...}
    tracked *.sh still sourcing the non-autobot- path: 0  (was 23 files / 24 lines)
    

    The finding that nearly shipped

    This PR does not add a code path — it activates several dozen dormant ones at once, and one of them was a deliberate fail-loud guard. bootstrap-slm.sh:406 used ${AUTOBOT_REDIS_HOST:?...}, added by #2224 because this script bakes the value into a remote node's generated .env over SSH and a loopback default there silently points a distributed Redis at the wrong host. Line 19 of that same file was one of the 23 source paths this PR corrects, and the library assigns AUTOBOT_REDIS_HOST at source time, ~390 lines before the guard. The guard would have become permanently unreachable.

    The enumeration missed it because it searched for ${VAR:-literal} and this is a different operator. The re-sweep across all 70 sourcing scripts confirmed it was the only :? in the set and there are zero :=. Fixed by capturing the operator's value before the source rather than by weakening the guard.

    Also found and fixed in the same file: INFRA_ROOT="${PROJECT_ROOT}/infrastructure" one line below the corrected source path, feeding eight cat calls at lines 453-518 that read service templates and unit files — all of which resolved to a directory that does not exist.

    The path fix itself went from 9 files to 23 after a first pass scoped its search to one directory. A git ls-files-anchored regression guard now asserts no tracked *.sh sources the non-autobot- path; it was verified failing against base with 23 offenders before being relied on.

    Correction on the guard's reach

    The guard is registered in .pre-commit-config.yaml, so it enforces in the local pre-commit stage. It does not enforce in CI, contrary to what the PR body says. .github/workflows/enforce-precommit.yml runs pre-commit run --all-files || { echo "::warning::..."; exit 0; } — the step cannot fail. That is filed as #14181, where the same swallowing is hiding sixteen hooks that fail on every run.

    Remaining for this issue

  3. mrveiss commented on Aug 15, 2026

    @mrveiss
    OwnerAuthor

    Measurement against current Dev_new_gui, in case it is useful for #14172/#14173 — the library
    landed, but almost nothing can reach it yet.

    ssot-config.sh copies tracked : 1   (autobot-infrastructure/shared/scripts/lib/ssot-config.sh)
    scripts sourcing it           : 72  (up from the 56 in this issue's text)
      resolve correctly           : 7
      silently fall through       : 65
    

    "Resolve correctly" means <script_dir>/<relative path in its own source line> is a tracked file.

    Four distinct shapes are in use, and they cannot all be right from one library location:

    31x  source "${SCRIPT_DIR}/../lib/ssot-config.sh" 2>/dev/null || true
    21x  source "$_PROJECT_ROOT/autobot-infrastructure/shared/scripts/lib/ssot-config.sh" 2>/dev/null || true
     6x  source "${SCRIPT_DIR}/lib/ssot-config.sh" 2>/dev/null || true
     3x  source "${SCRIPT_DIR}/../../lib/ssot-config.sh" 2>/dev/null || true
    

    The 21 $_PROJECT_ROOT ones are the shape that works. The 40 relative ones point at a lib/
    sibling most of those directories do not have.

    Every shape still ends 2>/dev/null || true, so this is the same silence the issue describes, one
    layer further along: the library now exists, the scripts still take their literal fallbacks, and
    nothing reports it. A reviewer reading ${VAR:-default} in one of those 65 still has no way to see
    that the SSOT was never loaded.

    Two things that would make the remaining work self-verifying, if they are not already in
    #14172/#14173:

    1. A guard that every source path in a tracked .sh resolves to a tracked file. That converts
      this class from silent to a CI failure, and would have caught the original 56 the day they were
      written.
    2. Dropping 2>/dev/null || true once the path is correct. While the redirect is there, a
      future move of the library re-creates the whole defect silently — the failure mode is the
      suppression, not the path.

    Not touching this myself — #14174 is yours and the remaining criteria are already tracked. Recording
    the numbers so "the library landed" is not mistaken for "the scripts read it".

  4. mrveiss commented on Aug 23, 2026

    @mrveiss
    OwnerAuthor

    Addressed by PR #14877, which delivers the two acceptance criteria that were still open after the library itself was built:

    • a missing config bootstrap now fails loudly — all 56 scripts converted from 2>/dev/null || true to an explicit failure block that writes to stderr and exits non-zero
    • a repo check flags a dangling or silenced lib source — repo_tests/shell_lib_sources_resolve_test.py

    Measured against the pristine base, that guard flags 57 of 57 sites across 56 of 56 scripts — the exact population this issue enumerated.

    The build-vs-delete decision this issue raised was settled when the library was built; the follow-on scope question was #14172, decided and recorded there.

  5. mrveiss commented on Aug 23, 2026

    @mrveiss
    OwnerAuthor

    Delivered and merged.

    Verified in base, not just in the PR:

    origin/Dev_new_gui:autobot-infrastructure/shared/scripts/lib/ssot-config.sh   PRESENT
    origin/Dev_new_gui:repo_tests/shell_lib_sources_resolve_test.py              PRESENT
    

    Sample of the landed call-site shape (vm-management/status-all-vms.sh:22):

    source "$SCRIPT_DIR/../lib/ssot-config.sh" || {
        echo "FATAL: ... refusing to run on hardcoded config fallbacks (#14172)" >&2
        return 1 2>/dev/null || exit 1
    }
    

    Coverage, counted against the pristine base and independently re-derived in review: 57 of 57 decisive source sites, across 56 of 56 scripts. 51 single-path sites, 5 two-path sites, 1 already-bare.

    Caveat recorded rather than glossed: the new guards live in repo_tests/, and the Python suite is not a required check on Dev_new_gui — so they run and report but gate no merge today. Tracked as #13162 / #14353, with the specific gap filed as #14878.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions