diff --git a/.github/ci_checks.py b/.github/ci_checks.py index 6f674ae..b99f7e7 100644 --- a/.github/ci_checks.py +++ b/.github/ci_checks.py @@ -87,11 +87,83 @@ # the text BEFORE the scan rather than added to a context allowlist. A bare # 32-hex string anywhere else still fails, which is the point. HEX32 = re.compile(r"\b[0-9a-f]{32}\b") -AD_ID_IN_URL = re.compile(r"---[0-9a-f]{32}\b") + +# The three contexts this site publishes its own 32-hex identifiers in. Each +# is subtracted from a line BEFORE the scan, so a bare 32-hex anywhere else +# still fails — which is the point, and is what keeps this from becoming a +# check nobody reads. +# +# 1. the tail of every ad URL, `…-2-536---ac0df37b…` +# 2. the `uuid` / `listing_uuid` field carrying that same value +# 3. the filename of every photo on dbz-images.dubizzle.com +# +# 2 and 3 were invisible until the scan stopped filtering by suffix: `.json` +# was not in the old allowlist, so neither `sample_output.json` nor +# `fixtures_generated.json` had ever been opened by this check. +SITE_PUBLIC_IDS = ( + re.compile(r"---[0-9a-f]{32}\b"), + re.compile(r'"(?:listing_)?uuid"\s*:\s*"[0-9a-f]{32}"'), + re.compile(r"dbz-images\.dubizzle\.com/[^\s\"']*?[0-9a-f]{32}"), +) + + +def _without_site_ids(line): + """A line with this site's own published identifiers taken out. + + Two passes, and the second is what makes this precise rather than broad. + The first removes the identifier in the CONTEXTS the site publishes it + in. The second removes those exact VALUES anywhere else on the same line + — because a row that has already shown a hex as the ad's public id in its + URL is not also carrying it as a separate secret, and in CSV that is + exactly what happens: the URL column and the `listing_uuid` column hold + the same string, one of them with no surrounding context at all. + + Anything left is a 32-hex the line never justified, and it still fails. + """ + known = set() + for pattern in SITE_PUBLIC_IDS: + for match in pattern.finditer(line): + known.update(HEX32.findall(match.group(0))) + line = pattern.sub("SITE-AD-ID", line) + for value in known: + line = line.replace(value, "SITE-AD-ID") + return line + + # Contexts in which a 32-hex string is plainly not a key. HEX32_ALLOWED = ("sha", "hash", "nonce", "example", "md5", "digest", "checksum") +# Files the BARE-HEX rule is not applied to, and the reason it is not. +# +# These are verbatim site markup and verbatim run output. This site emits +# 32-hex identifiers in at least five public contexts — the tail of an ad +# URL, the `uuid` field, a photo filename, the `location_list.uuids` array, +# and Imperva's own resource token — so a bare-hex rule over them produces +# hundreds of findings that are all correct data. A check that cries wolf 221 +# times is a check somebody switches off, and then it protects nothing. +# +# What covers them instead is STRONGER, not weaker, because it looks for the +# shape of a secret rather than the shape of a hex string: +# +# * every rule below still applies here — a credentialled URL and a +# key-shaped field both fail in these files; +# * `make_fixtures.py` refuses to write a fixture whose scrub left an +# agent's name, a per-seller UUID or a key-shaped value in it; +# * `smoke_test.py` re-scans the whole committed fixture corpus for JWTs, +# access tokens, API keys, Sentry DSNs, session ids, emails and proxy +# credentials, and FAILS if the corpus it scanned was empty. +GENERATED_DATA_FILES = ("fixtures_generated.json", "sample_output.json", + "sample_output.csv") + +# A secret sitting in a field named like one. This is what the bare-hex rule +# was reaching for, said precisely, and it applies to EVERY tracked file +# including the generated ones. +KEY_SHAPED_FIELD = re.compile( + r'"(?:[a-zA-Z_-]*(?:api[_-]?key|apikey|secret|token|password|' + r'client[_-]?key|access[_-]?key|site[_-]?key))"\s*[:=]\s*' + r'"(?!REDACTED-|your_|\{|\*\*\*)[A-Za-z0-9_-]{16,}"', re.I) + # History findings that have been LOOKED AT and cleared, each with its # reason. This exists because the history scan is a pre-publication gate: a # later commit cannot reach what a published tag and a merged PR's refs @@ -110,15 +182,104 @@ "a dubizzle listing id in the legacy README's JSON example, not a key", } +# Raw captures that HAVE been committed at some point, each with the decision +# taken about it. A blob in history cannot be removed by a later commit — a +# merged PR's refs and any published tag keep it — so this is the record the +# pre-publication step asks for: read it once, decide once, before the repo +# goes public. +CAPTURES_IN_HISTORY_DECIDED = { + # Committed by the v0.1.0 squash merge, because `--dump-html live_results` + # writes FILES (`live_results.page1`) and .gitignore only covered the + # DIRECTORY form. Both removed in the commit after, and both are in + # history for good. + # + # Read before deciding. Each is one motors listing page as the site + # served it, and carries: 26 per-seller UUIDs (opaque tokens the site + # ships to every visitor), 12 copies of its public Algolia search key, 2 + # Sentry public keys, and ~820 of its own 32-hex ad identifiers — all of + # it data dubizzle publishes to anyone who loads the page. + # + # It carries NOTHING OF OURS: no 2Captcha key, no proxy credential, no + # session cookie, no Bearer token, no email address. Verified by pattern + # before the decision was taken. A motors page also carries no + # `agent_profile`, so no individual is named. + # + # Decision: not a leak, and not worth recreating the repository over. + # What it IS is 3 MB of unscrubbed markup that this project's own rules + # keep out, so the gitignore was widened, the working-tree scan stopped + # filtering by suffix, and a tracked-capture rule was added — see + # CAPTURE_SHAPES. + "live_results.page1": + "one motors listing page, site-public data only, nothing of ours", + "live_results.page2": + "one motors listing page, site-public data only, nothing of ours", +} + +# Suffixes the HISTORY scan walks. It reads blobs out of git, where a +# binary is expensive to decode and useless to grep, so it stays narrow. SCANNED_SUFFIXES = (".py", ".md", ".txt", ".yml", ".yaml", ".example") +# The WORKING-TREE scan is the opposite: it reads whatever is TRACKED, at any +# suffix, and that is not tidiness. +# +# A suffix allowlist is a scanner that cannot see the thing most likely to +# leak. `--dump-html live_results` writes `live_results.page1` — no suffix +# the list knew — and a merge committed two of them, 1.5 MB each, carrying 26 +# per-seller UUIDs, 12 copies of the site's Algolia key and its Sentry keys. +# The check that exists to stop exactly that ran, passed, and never opened +# them. +# +# So the rule inverted: scan every tracked file, skip only what cannot be +# grepped. +BINARY_SUFFIXES = (".png", ".jpg", ".jpeg", ".gif", ".webp", ".ico", ".pdf", + ".zip", ".gz", ".tar", ".whl", ".woff", ".woff2", ".ttf") + +# A raw page dump has no business being tracked at all — it is 1.5 MB of +# someone else's session material, and scrubbing it is `make_fixtures.py`'s +# job. Matched on SHAPE rather than on the two names that got through once. +CAPTURE_SHAPES = ( + re.compile(r"\.page\d+$"), + re.compile(r"_debug\.(?:html|png)$"), + re.compile(r"^live_results\."), + re.compile(r"^captures?/"), +) + + +def _git_files(*flags): + out = subprocess.run(["git", "ls-files", "-z", *flags], cwd=REPO, + capture_output=True, text=True) + if out.returncode != 0: + return + for rel in out.stdout.split("\0"): + if rel: + yield REPO / rel + + +def tracked_files(): + """Only what git already tracks. Used by the raw-capture rule, whose + question is literally "is this committed".""" + return _git_files("--cached") + def scanned_files(): - for path in sorted(REPO.rglob("*")): - if not path.is_file() or path.suffix not in SCANNED_SUFFIXES: - continue - if any(part in {".git", "__pycache__", ".venv", "venv"} - for part in path.parts): + """What the CONTENT rules read: tracked files PLUS new files that are not + ignored. + + Asked of GIT rather than of the disk, and the two flags are the whole + design: + + --cached what is committed, which is what can leak; + --others what is new, so a secret is caught BEFORE it is + added rather than after; + --exclude-standard which drops everything `.gitignore` covers — a + developer's own `.env`, their captures and their + run output are EXPECTED beside the scripts, and a + check that went red on them would be red on every + machine that had ever run the scraper for real, + which is the machine most likely to run it. + """ + for path in _git_files("--cached", "--others", "--exclude-standard"): + if not path.is_file() or path.suffix.lower() in BINARY_SUFFIXES: continue yield path @@ -202,7 +363,16 @@ def secret_check(): failed.append(f"{rel}:{lineno} looks like a URL with real " f"credentials in it") - for match in HEX32.findall(AD_ID_IN_URL.sub("---AD-ID", line)): + # Applies everywhere, generated data included — this is the rule + # the bare-hex one was reaching for, said precisely. + for hit in KEY_SHAPED_FIELD.findall(line): + failed.append(f"{rel}:{lineno} has a secret-shaped value in a " + f"key-shaped field") + + if rel.name in GENERATED_DATA_FILES: + continue + + for match in HEX32.findall(_without_site_ids(line)): if any(token in line.lower() for token in HEX32_ALLOWED): continue # A value already decided for the history scan is decided @@ -215,8 +385,23 @@ def secret_check(): failed.append(f"{rel}:{lineno} contains {match[:6]}… — a " f"32-char hex string, the shape of a 2captcha key") + # A raw capture must not be tracked AT ALL, whatever is in it. This is + # separate from the content scan on purpose: the two dumps that got + # through carried nothing of OURS -- no key, no proxy password, no + # cookie -- so a content rule would have passed them. What was wrong was + # that they were committed: 3 MB of someone else's session material, + # unscrubbed, in a repository whose own rules say captures stay out. + for path in tracked_files(): + rel = path.relative_to(REPO).as_posix() + if any(shape.search(rel) for shape in CAPTURE_SHAPES): + failed.append(f"{rel} is a raw page capture and is TRACKED. " + f"Captures stay out of the repo; run them through " + f"make_fixtures.py, which scrubs them and proves " + f"the trim parses identically.") + if not failed: - print(f"ok {scanned} files scanned, nothing credential-shaped") + print(f"ok {scanned} files scanned, nothing credential-shaped, " + f"no raw capture tracked") return failed @@ -254,6 +439,29 @@ def history_check(): objects.append((parts[0], parts[1] if len(parts) > 1 else "")) failed, decided, scanned = [], [], 0 + + # RAW CAPTURES THAT HAVE EVER BEEN COMMITTED, reported by name and size + # whether or not they are still in the tree. Removing one in a later + # commit does not reach the blob: a merged PR's refs and any published + # tag keep it, and only a fresh repository removes it. So this is not a + # pass/fail rule — it is the thing a reader has to make a decision about + # before the repo goes public, which is the whole reason this scan + # exists. A decision taken is recorded in CAPTURES_IN_HISTORY_DECIDED. + for sha, path in objects: + if not path or not any(s.search(path) for s in CAPTURE_SHAPES): + continue + size = subprocess.run(["git", "cat-file", "-s", sha], cwd=REPO, + capture_output=True, text=True).stdout.strip() + note = CAPTURES_IN_HISTORY_DECIDED.get(path) + if note: + decided.append(f"{path} ({size} bytes, in history forever) — {note}") + else: + failed.append( + f"{path} ({size} bytes) is a raw page capture that has been " + f"COMMITTED at some point. A later commit cannot remove the " + f"blob. Read it, decide, and record the decision in " + f"CAPTURES_IN_HISTORY_DECIDED — or start a fresh repository.") + for sha, path in objects: if not (path.endswith(SCANNED_SUFFIXES) or path in ("Dockerfile",)): continue @@ -270,7 +478,7 @@ def history_check(): token in line for token in CREDENTIAL_ALLOWED): failed.append(f"{path}:{lineno} (in a past commit) looks like " f"a URL with real credentials in it") - for match in HEX32.findall(AD_ID_IN_URL.sub("---AD-ID", line)): + for match in HEX32.findall(_without_site_ids(line)): if any(token in line.lower() for token in HEX32_ALLOWED): continue if match in HISTORY_DECIDED: diff --git a/.gitignore b/.gitignore index 8f9abcc..5f5fb61 100644 --- a/.gitignore +++ b/.gitignore @@ -19,7 +19,16 @@ dubizzle_listings.* dubizzle_listings_*.* # --- the live-test harness's working directory --------------------------- +# `--dump-html PATH` writes PATH.page1, PATH.page2, ... — FILES, not a +# directory. Ignoring only `live_results/` let two 1.5 MB page dumps be +# committed by a merge; each carried 26 per-seller UUIDs, 12 copies of the +# site's Algolia key and its Sentry keys, none of which belongs here. Both +# spellings are ignored now, and the offline suite asserts that no raw +# capture is tracked. live_results/ +live_results.* +*.page[0-9] +*.page[0-9][0-9] live_results.zip # --- debug artifacts the engines drop next to themselves ----------------- diff --git a/CHANGELOG.md b/CHANGELOG.md index 6ce9895..256a939 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -41,7 +41,7 @@ layout survives. `.env` loading with documented precedence (`env_config.py`), captcha detection and solving (`captcha_solver.py`), 2Captcha fingerprints (`fingerprint_client.py`) and a two-run differ (`diff_runs.py`). -- **510 offline checks** (`smoke_test.py`, wrapped for `pytest`), with every +- **520 offline checks** (`smoke_test.py`, wrapped for `pytest`), with every fixture cut from a real capture by `make_fixtures.py` and verified to parse identically to the untrimmed original before being committed. - **CI**: the offline suite on the oldest and newest supported Python, a @@ -116,7 +116,17 @@ found them: branch to execute. - The repo's own secret check flagged this site's public ad identifiers — a 32-hex string is both an API key's shape and every dubizzle ad's id — so it - failed on its own repository. It now subtracts the `---{ad id}` shape - before scanning. + failed on its own repository. It now subtracts that identifier in all three + contexts the site publishes it in. +- **The secret check scanned only six file suffixes**, so it had never opened + `fixtures_generated.json`, `sample_output.json` or `sample_output.csv` — + and it could not see a page dump at all, because `--dump-html live_results` + writes `live_results.page1`, a name with no suffix it knew. Two 1.5 MB + dumps were committed by the release merge before this was found. The scan + now reads everything git tracks (plus anything new that is not ignored) at + any suffix, a raw capture is refused by SHAPE whether or not its content + looks dangerous, and the history scan reports any capture that has ever + been committed so the decision is taken once, in writing, before + publication. [0.1.0]: https://github.com/2scraper/dubizzle-scraper/releases/tag/v0.1.0 diff --git a/README.md b/README.md index 193be9c..1fd5f05 100644 --- a/README.md +++ b/README.md @@ -412,7 +412,7 @@ All 2026-09-14, through a UAE residential exit unless stated. | Asset-host references, served page vs refusal | 126–2,571 vs **0** | | Scroll rounds needed | none — captures taken with 0 scrolls matched those taken with 4 | | Block-page shapes seen | 1,160 B (HTTP 403) and 6,183 B (HTTP **200**) | -| Offline checks | 510 | +| Offline checks | 520 | Everything except the DOM cross-check comes out of `__NEXT_DATA__`, which is in the first response and needs no JavaScript, so a readiness wait that times @@ -464,7 +464,7 @@ to compare, not the positions. ## Testing ```bash -python3 smoke_test.py # 510 offline checks, no engine library needed +python3 smoke_test.py # 520 offline checks, no engine library needed pytest # the same checks, wrapped as one test python3 env_config.py # what config was picked up, without secrets python3 .github/ci_checks.py --all # what CI runs diff --git a/live_results.page1 b/live_results.page1 deleted file mode 100644 index 6ac3344..0000000 --- a/live_results.page1 +++ /dev/null @@ -1,679 +0,0 @@ -Used Cars for Sale in UAE | dubizzle
verified-white

Join us in building a safer community. Get verified to boost your credibility and assist us in creating trust amongst our users!

Log in or sign up
Make and Model
Done

Request dubizzle

Car Inspection

Trusted by 10K+ car buyers, dubizzle is the UAE's go-to platform

inspection-banner-tick

120 - 240

Points Covered


inspection-banner-location

Inspect

Anywhere!


inspection-banner-calendar

Same Day

Inspection


- - - -
\ No newline at end of file diff --git a/live_results.page2 b/live_results.page2 deleted file mode 100644 index 06b93be..0000000 --- a/live_results.page2 +++ /dev/null @@ -1,253 +0,0 @@ -Used Cars for Sale in UAE | dubizzle Page-2
verified-white

Join us in building a safer community. Get verified to boost your credibility and assist us in creating trust amongst our users!

Log in or sign up
Make and Model
Done

Request dubizzle

Car Inspection

Trusted by 10K+ car buyers, dubizzle is the UAE's go-to platform

inspection-banner-tick

120 - 240

Points Covered


inspection-banner-location

Inspect

Anywhere!


inspection-banner-calendar

Same Day

Inspection


- - - - \ No newline at end of file diff --git a/smoke_test.py b/smoke_test.py index 6f592f9..44d8c56 100644 --- a/smoke_test.py +++ b/smoke_test.py @@ -1285,6 +1285,77 @@ def test_ci_checks_is_actually_wired_up(): ok &= check("the secret check specifically is run", "--secret-check" in workflows or "--all" in workflows) + # IT SCANS WHAT IS TRACKED, AT ANY SUFFIX — pinned because a suffix + # allowlist is how this check failed once. `--dump-html live_results` + # writes `live_results.page1`, a name with no suffix the old list knew, + # and a merge committed two of them at 1.5 MB each while this check ran, + # passed and never opened them. `.json` was not on the list either, so + # `fixtures_generated.json` and `sample_output.json` had never been + # scanned at all. + import importlib.util as _ilu + spec = _ilu.spec_from_file_location("ci_checks", script) + ci = _ilu.module_from_spec(spec) + spec.loader.exec_module(ci) + scanned = {os.path.relpath(p, REPO_ROOT) for p in ci.scanned_files()} + ok &= check("the working-tree scan reads the generated data files, which " + "a suffix allowlist never did", + {"fixtures_generated.json", "sample_output.json", + "sample_output.csv"} <= scanned) + ok &= check("...and the workflows, the Dockerfile and the env example", + {".env.example", "Dockerfile"} <= scanned) + ok &= check("it asks GIT what is tracked, not the filesystem — a " + "developer's own .env and captures beside the scripts are " + "expected and must not turn it red", + ".env" not in scanned) + + # A RAW CAPTURE MUST NOT BE TRACKED, whatever is inside it. The two that + # got through carried nothing of ours — no key, no proxy password, no + # cookie — so a content rule would have passed them. What was wrong was + # that they were committed at all. + ok &= check("a tracked page dump is refused by shape, not by name", + any(s.search("live_results.page7") for s in ci.CAPTURE_SHAPES) + and any(s.search("out_page1_debug.html") + for s in ci.CAPTURE_SHAPES) + and any(s.search("captures/uae_usedcars_p1.html") + for s in ci.CAPTURE_SHAPES)) + ok &= check("and no such file is tracked here", + not [p for p in ci.tracked_files() + if any(s.search(p.relative_to(ci.REPO).as_posix()) + for s in ci.CAPTURE_SHAPES)]) + + # The key-shaped-field rule applies EVERYWHERE, generated data included — + # it is what the bare-hex rule was reaching for, said precisely. + ok &= check("a secret in a key-shaped field is caught", + ci.KEY_SHAPED_FIELD.search( + # Split so this file does not itself carry a bare 32-hex + # run: the check it is testing scans this file too, and a + # fixture that trips the rule it proves would either turn + # the build red or force the rule to be widened. + '"x-algolia-api-key": "%s%s"' + % ("cdd839b4fdac8402", "89e88633779e8634"))) + ok &= check("...and the scrubbed placeholder is not", + not ci.KEY_SHAPED_FIELD.search( + '"x-algolia-api-key": "REDACTED-SEARCH-KEY"')) + + # The bare-hex rule is deliberately NOT applied to the generated data + # files, and that exemption is pinned so it stays a decision: this site + # publishes 32-hex identifiers in at least five contexts, and a rule that + # fires 221 times on correct data is a rule somebody switches off. + ok &= check("the bare-hex rule exempts the generated data files, with " + "the corpus scan covering them instead", + set(ci.GENERATED_DATA_FILES) == {"fixtures_generated.json", + "sample_output.json", + "sample_output.csv"}) + ok &= check("...but this site's public ad id is still subtracted from " + "every other file, in all three of its spellings", + not ci.HEX32.findall(ci._without_site_ids( + '"url": "https://dubai.dubizzle.com/x/2026/1/1/' + 'a---ac0df37b468f4757aebf3a7c36ccf0a1/", ' + '"listing_uuid": "ac0df37b468f4757aebf3a7c36ccf0a1"'))) + ok &= check("...and a hex the line never justified still fails", + ci.HEX32.findall(ci._without_site_ids( + 'key = "%s"' % ("deadbeefdeadbeef" * 2)))) + # AND IT PASSES ON THIS REPO. A check that is always red teaches everyone # to ignore checks; this one WAS red, on six documented placeholders. done = subprocess.run([sys.executable, script, "--all"],