Conversation
`curl` without --fail exits 0 on HTTP 401/403/500, so the error body was written to malpedia.json and mv'd straight over the curated cluster with no check of any kind. A JSON-syntax check alone would not help: a REST error response is usually valid JSON and would pass. With no `set -e`, the four cleanup scripts then ran over the wreckage, and a crash in one of them left the overwrite already applied and the cleanup half done. Stage the whole update in a temp file and only replace clusters/malpedia.json once the download, the sanity check and every cleanup script have succeeded: - set -euo pipefail - curl --fail --show-error --silent, output to a mktemp file with a trap - require a non-empty `values` array (shape, not just syntax) - run the cleanup scripts on the temp file; mv into place last Guard verified: rejects an HTML error page, a valid-JSON API error, an empty cluster and an empty body; accepts the real cluster. All four cleanup scripts run clean over the current malpedia.json, so set -e regresses nothing. The token line is otherwise untouched. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HZGwPoa8MMfkhCw47rDLA4
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
BLUF —
fetch_malpedia.shoverwrites the curatedclusters/malpedia.jsonwith API error bodies.tools/fetch_malpedia.shrunscurlwithout--failandmvs the response straight over the curatedclusters/malpedia.json, so an HTTP 401, 403 or 500 writes the API error body over the committed cluster. With noset -ethe fourdel_*.pycleanup scripts then run over the wreckage, and a JSON syntax check would not help since API error bodies are valid JSON.mktempfile, require a non-emptyvaluesarray, run the cleanup scripts there, andmvinto place as the final step underset -euo pipefail.Problem
tools/fetch_malpedia.shoverwrites the committedclusters/malpedia.jsonwith whatever the API returned:curlwithout--failexits 0 on HTTP 401/403/500 and writes the error body tomalpedia.json.mv'd straight over the curated cluster. No check of any kind runs first.set -e, so the four cleanup scripts then run over the wreckage — and if one of them raises, the overwrite has already landed and the cleanup is left half-applied.A JSON-syntax check alone would not be enough: a REST API's error response is usually valid JSON (
{"detail": "Invalid token."}), so it would sail through and still destroy the cluster.Fix
Stage the entire update in a temp file and only replace
clusters/malpedia.jsononce the download, the sanity check and all four cleanup scripts have succeeded. The update is now atomic — a failure at any stage leaves the committed cluster untouched.set -euo pipefailcurl --fail --show-error --silentso an HTTP error is a non-zero exitmktempfile with a cleanuptrapvaluesarraymvinto place is the last stepThe token line is otherwise untouched — rotating that credential is a separate matter and deliberately not addressed here.
Verification
The guard against realistic failure modes, and against the real cluster:
And confirming
set -eintroduces no regression — all four cleanup scripts run clean over the current cluster:🤖 Generated with Claude Code