From 39e227f1c8a2c657fd3927a8a46aed73466b10fb Mon Sep 17 00:00:00 2001 From: Bastien Chanot Date: Fri, 10 Jul 2026 01:42:23 +0200 Subject: [PATCH] fix(seo-data): fail-open CLI contract (corrupt store + bad usage always emit JSON) --- lib/seo-data/google_seo.py | 40 +++++++++++++++++++---------------- lib/seo-data/seo-data.test.sh | 19 ++++++++++++++++- lib/seo-data/tokenstore.py | 23 +++++++++++++------- 3 files changed, 55 insertions(+), 27 deletions(-) diff --git a/lib/seo-data/google_seo.py b/lib/seo-data/google_seo.py index acf6124..d73277d 100644 --- a/lib/seo-data/google_seo.py +++ b/lib/seo-data/google_seo.py @@ -133,25 +133,25 @@ def inspect(store_path, account, property, url): "last_crawl": isr.get("lastCrawlTime")} def _cli(): - p = argparse.ArgumentParser() - sub = p.add_subparsers(dest="cmd", required=True) - pc = sub.add_parser("crux") - pc.add_argument("--url", required=True) - pc.add_argument("--strategy", default="mobile", choices=["mobile", "desktop"]) - pc.add_argument("--store", default=None) # accepted+ignored: uniform fetch.sh dispatch - pq = sub.add_parser("queries") - pq.add_argument("--store", required=True) - pq.add_argument("--account", required=True) - pq.add_argument("--property", required=True) - pq.add_argument("--days", type=int, default=90) - pq.add_argument("--dim", default="query") - pi = sub.add_parser("inspect") - pi.add_argument("--store", required=True) - pi.add_argument("--account", required=True) - pi.add_argument("--property", required=True) - pi.add_argument("--url", required=True) - args = p.parse_args() try: + p = argparse.ArgumentParser() + sub = p.add_subparsers(dest="cmd", required=True) + pc = sub.add_parser("crux") + pc.add_argument("--url", required=True) + pc.add_argument("--strategy", default="mobile", choices=["mobile", "desktop"]) + pc.add_argument("--store", default=None) # accepted+ignored: uniform fetch.sh dispatch + pq = sub.add_parser("queries") + pq.add_argument("--store", required=True) + pq.add_argument("--account", required=True) + pq.add_argument("--property", required=True) + pq.add_argument("--days", type=int, default=90) + pq.add_argument("--dim", default="query") + pi = sub.add_parser("inspect") + pi.add_argument("--store", required=True) + pi.add_argument("--account", required=True) + pi.add_argument("--property", required=True) + pi.add_argument("--url", required=True) + args = p.parse_args() if args.cmd == "crux": print(json.dumps(crux(args.url, args.strategy), indent=2)) elif args.cmd == "queries": @@ -160,6 +160,10 @@ def _cli(): elif args.cmd == "inspect": print(json.dumps(inspect(args.store, args.account, args.property, args.url), indent=2)) + except SystemExit as e: # argparse usage error + if e.code not in (0, None): + print(json.dumps({"status": "error", "reason": "bad_usage"})) + raise # preserve argparse's exit code except Exception: # Fail-open data contract: ANY unexpected error (HTTP 403/5xx, DNS, # timeout) degrades with exit 0 — never a traceback, never empty stdout. diff --git a/lib/seo-data/seo-data.test.sh b/lib/seo-data/seo-data.test.sh index 8e9517f..d8589c6 100644 --- a/lib/seo-data/seo-data.test.sh +++ b/lib/seo-data/seo-data.test.sh @@ -77,7 +77,24 @@ SEO_DATA_ENV_FILE=$NOENV bash "$FETCH" bogus-subcmd >/dev/null 2>&1; RC=$? DG="$(SEO_DATA_ENV_FILE=$NOENV env -u SEO_DATA_MOCK_DIR -u CRUX_API_KEY bash "$FETCH" crux --url https://ex.com)"; RC=$? has "degrade json" "$DG" '"status": "degraded"' [ "$RC" = "0" ] && ok "degrade exit 0" || no "degrade exit 0" "got $RC" -hasnt "no secret echoed" "$DG" 'RT_' +# redaction through the real fetch.sh dispatch layer +TMP4="$(mktemp -d)"; RSTORE="$TMP4/rt.json" +python3 "$SD/tokenstore.py" set --file "$RSTORE" --label leaky --refresh-token RT_SECRET_XYZ \ + --scopes https://www.googleapis.com/auth/webmasters.readonly --properties sc-domain:z.com >/dev/null +ACCJSON="$(SEO_DATA_ENV_FILE=$NOENV SEO_DATA_STORE="$RSTORE" bash "$FETCH" accounts)" +has "accounts lists label" "$ACCJSON" 'leaky' +hasnt "accounts hides token" "$ACCJSON" 'RT_SECRET_XYZ' +rm -rf "$TMP4" +# corrupted store must degrade with JSON + exit 0 (Fix 1) +TMP5="$(mktemp -d)"; CSTORE="$TMP5/corrupt.json"; printf 'not json {{' > "$CSTORE" +CJ="$(SEO_DATA_ENV_FILE=$NOENV SEO_DATA_STORE="$CSTORE" bash "$FETCH" accounts)"; CRC=$? +has "corrupt store degrades" "$CJ" '"status"' +[ "$CRC" = "0" ] && ok "corrupt store exit 0" || no "corrupt store exit 0" "got $CRC" +rm -rf "$TMP5" +# bad usage (known subcmd, missing flag) must still emit JSON + exit 2 (Fix 2) +BU="$(SEO_DATA_ENV_FILE=$NOENV bash "$FETCH" crux)"; BURC=$? +has "bad usage emits json" "$BU" '"status"' +[ "$BURC" = "2" ] && ok "bad usage exit 2" || no "bad usage exit 2" "got $BURC" echo "" echo "seo-data engine: $PASS pass, $FAIL fail" diff --git a/lib/seo-data/tokenstore.py b/lib/seo-data/tokenstore.py index 2f5a6f5..1165013 100644 --- a/lib/seo-data/tokenstore.py +++ b/lib/seo-data/tokenstore.py @@ -58,14 +58,21 @@ def _cli(): ps.add_argument(flag, required=True) ps.add_argument("--scopes", default="") ps.add_argument("--properties", default="") - args = p.parse_args() - if args.cmd == "list": - print(json.dumps({"status": "ok", "accounts": list_accounts(args.file)})) - else: - save_account(args.file, args.label, getattr(args, "refresh_token"), - [s for s in args.scopes.split(",") if s], - [x for x in args.properties.split(",") if x]) - print(json.dumps({"status": "ok"})) + try: + args = p.parse_args() + if args.cmd == "list": + print(json.dumps({"status": "ok", "accounts": list_accounts(args.file)})) + else: + save_account(args.file, args.label, getattr(args, "refresh_token"), + [s for s in args.scopes.split(",") if s], + [x for x in args.properties.split(",") if x]) + print(json.dumps({"status": "ok"})) + except SystemExit as e: # argparse usage error + if e.code not in (0, None): + print(json.dumps({"status": "error", "reason": "bad_usage"})) + raise # preserve argparse's exit code + except Exception: # e.g. corrupted store JSON + print(json.dumps({"status": "degraded", "reason": "unexpected_error"})) if __name__ == "__main__": _cli()