Skip to content

Two fail-open paths in trustabl-scan.sh: missing overall_score scores 100, checksum failure proceeds #9

Description

@fxmedus

Two places in scan/trustabl-scan.sh fail open. Both are one-line changes, but both alter results for existing pipelines, so they seem worth a decision rather than a surprise PR. Happy to send patches for whichever direction you prefer.

1. A missing overall_score produces a perfect score

Line ~145:

RAW_SCORE=$(jq -r '.overall_score // 1' "$JSON_FILE")
SCORE=$(awk -v s="$RAW_SCORE" 'BEGIN{ v = s*100; ... }')
RISK=$(( 100 - SCORE ))

// 1 supplies the maximum value when the field is absent. 1 scales to SCORE=100
and therefore RISK=0.

Reproduced with JSON containing a critical finding but no overall_score key:

READINESS  100
RISK         0

So a truncated, malformed, or schema-drifted report reads as a clean repository. The
neighbouring default on the next line is the safe direction already
(.findings | length // 0), which is why this one stands out.

The two candidate fixes differ in blast radius:

  • // 0 — a missing score becomes readiness 0 / risk 100. Fails closed, but will
    red-light any pipeline currently relying on the silent pass.
  • Treat the absence as an error and exit 2 — arguably more accurate, since the scan did
    not produce a usable result, and it composes with the exit-code distinction in
    docs/EVALUATION.md.

I lean toward the second, but it is your call which one matches intent.

2. Checksum verification degrades to a warning and proceeds

Lines ~90 to ~106:

if curl -fsSL "${AUTH[@]}" -o "$DEST/checksums.txt" ... ; then
  ...
  else
    echo "WARNING: $ASSET not listed in checksums.txt — skipping verification"
  fi
else
  echo "WARNING: could not fetch checksums.txt — skipping verification"
fi

tar -xzf "$DEST/$ASSET" -C "$DEST"

Both warning branches fall through to extraction and execution. A transient network
failure, a proxy returning a non-2xx for checksums.txt, or an asset simply absent from
the manifest all result in an unverified binary running, with the only signal a line in
the build log that nothing gates on.

I did not send this as a PR because hard-failing would break anyone on a network where
checksums.txt is unreachable, and that is a deliberate policy choice. A middle option,
if a hard fail is too aggressive: keep the fallback but make it opt-in via a flag or
environment variable, so the permissive path is chosen rather than defaulted into.

Context

Found while working on the PRs opened today (artifact publication, exit-code
propagation, and PATH resolution during install). Both of these sit in the same file but
are behavioural rather than mechanical, hence the issue instead.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions