Skip to content

feat: establish local-first scraping capability - #1

Merged
sfloess merged 32 commits into
mainfrom
feat/initial-acquisition-capability
Sep 4, 2026
Merged

feat: establish local-first scraping capability#1
sfloess merged 32 commits into
mainfrom
feat/initial-acquisition-capability

Conversation

@sfloess

@sfloesssfloess commented Sep 4, 2026

Copy link
Copy Markdown
Member

Summary

  • add scraping Python package and scrape CLI
  • support HTTP(S), FTP, and filesystem/file:// acquisition
  • add URL, sitemap, filesystem, and link-crawl discovery
  • preserve raw artifacts in a local corpus with SHA-256 identity and JSONL manifest
  • add curated patent, medical, engineering, and science URI catalogs
  • establish X.Y package versioning
  • add CI, package validation, and SonarQube integration

Build and release policy

Builds, tests, pull requests, and merges are tag-independent. The package version is read from pyproject.toml; no Git tag is required for normal CI or artifact builds.

The optional release workflow is manually invoked and publishes the immutable artifacts produced by that workflow. It does not gate ordinary builds or merges.

Notes

The implementation is stdlib-first and keeps parsing, normalization, chunking, embedding, indexing, and retrieval downstream of acquisition.

@sfloessGrok (by xAI)

Copy link
Copy Markdown
MemberAuthor

Adversarial review — DO NOT SHIP

Comprehensive review of the full diff against the stated architecture, README claims, and release readiness for 0.1.

Verdict: DO NOT SHIP. The package does not import, several safety/limit invariants are missing or inverted, discovery is fused with acquisition, tests give false confidence, and multiple README/architecture promises are not implemented.


Critical

  1. Broken import (release blocker)src/scraping/acquisition/corpus.py

    • from .models import AcquiredResource is wrong; models lives in scraping.models.
    • LocalCorpus / fetch_uri / scrape CLI cannot load.
    • Fix:from ..models import AcquiredResource and add an install/import smoke test.
  2. Robots.txt unreachable → deny allsrc/scraping/discovery/web.py (RobotsPolicy.allowed)

    • except OSError: return False means missing/unreachable robots.txt blocks every URL for that origin.
    • Contradicts “robots.txt: respected” and normal allow-when-unreachable practice.
    • Fix: On read failure, treat as allowed; only deny on explicit disallow rules. Add tests.
  3. Unbounded downloads on crawl pathsrc/scraping/discovery/web.py (fetch)

    • response.read() with no size limit. CLI checks size only after the body is in memory.
    • fetch_uri has a limit; crawl does not. Defeats the stated 50 MB max.
    • Fix: One shared size-limited fetch used by crawl, fetch_uri, and sitemap.
  4. SSRF / unsafe remote fetchcorpus.py, web.py, sitemap.py

    • Raw urlopen, no private/link-local blocking, no redirect policy, redirects followed by default.
    • User/sitemap URLs (or redirect chains) can hit internal targets.
    • Fix: Scheme allowlist, optional private-IP block (or document “trusted input only”), controlled redirects, never redirect to file:.
  5. Sitemap recursion unboundedsrc/scraping/discovery/sitemap.py

    • Nested indexes recurse with no depth/URL/byte caps → resource exhaustion / XML abuse.
    • Fix: Hard limits on recursion, total URLs, and bytes read.
  6. Discovery fused with acquisition (architecture violation)discovery/web.py (crawl), cli.py

    • crawl discovers and downloads full bodies; README/architecture require discovery separate from acquisition.
    • Fix: Discovery yields URIs (and light metadata); acquisition is a separate step that produces AcquiredResource.

High

  1. Duplicate content drops URI provenancecorpus.store returns None and writes nothing to the manifest when the SHA-256 blob already exists. Second URI with same bytes is never recorded.

  2. CLI always exits 0 — errors are printed; main always returns 0. Unusable for scripts/CI.

  3. FTP claimed but not real — scheme allowed; no real size/rate/error handling or tests. Remove claim or implement properly.

  4. Inconsistent max-size / timeout — crawl path ignores configured limits that fetch_uri enforces.

  5. Package name scraping — extremely generic; high PyPI collision risk before first publish. Prefer something like flossware-scraping.

  6. .gitignore is effectively empty — risk of committing scraped-data/, dist/, __pycache__, .venv, etc.

  7. Tests give false confidence — four trivial unit tests; no HTTP, robots, size limits, redirects, sitemap index, CLI, error paths, or import smoke under packaging. CI matrix will pass while the product is broken.


Medium / other

  • Concurrent/interrupted runs: append-only manifest with no locking; partial writes possible.
  • Media-type detection is weak (suffix map / Content-Type only).
  • Premature extracted/ and normalized/ dirs with no consumers.
  • README says unittest; CI and optional deps use pytest.
  • sources/ catalogs are not packaged; examples only work from a checkout.
  • Rate limit is a single sleep after first success; no per-host budget or crawl-delay.
  • Broad except (OSError, ValueError) collapses distinct failure modes.

Release blockers (must fix before 0.1)

  1. Fix import so the package and console script load.
  2. Enforce max size on every download path (especially crawl).
  3. Fix robots unreachable behavior.
  4. Bound sitemap recursion and download sizes.
  5. Separate discovery from acquisition (or drop the architecture claim).
  6. Real tests that would have caught the above.
  7. Decide package name before first PyPI publish.
  8. Non-zero CLI exit codes; usable .gitignore.
  9. Align README with implementation (FTP, architecture, unittest vs pytest, sources packaging).

Release workflow shape (X.Y tag == version, build, twine, GH release, OIDC) is structurally fine for FlossWare policy; it is not the primary blocker. Correctness and safety are.


Recommended order of work

  1. Fix relative import + import/install smoke test.
  2. Unify size- and timeout-limited fetch; use everywhere.
  3. Robots allow-when-unreachable + tests.
  4. Cap sitemap recursion and bytes.
  5. Split crawl into discover-URIs vs acquire-bytes; keep AcquiredResource as the stored object.
  6. Always append manifest rows even on content-hash hit.
  7. CLI exit codes and argument validation.
  8. Expand tests (mocks / local HTTP server) for security and limit cases.
  9. Package name, .gitignore, README accuracy.
  10. Only then tag 0.1.

Until the critical items are fixed, this PR should not merge and must not be released as 0.1.

@sfloessGrok (by xAI)

Copy link
Copy Markdown
MemberAuthor

Re-review of head 900709aSHIP WITH FIXES

Follow-up adversarial review after the harden/fix commits. Prior critical blockers are largely resolved. CI is green on 3.10–3.14, package, and Sonar.

What was fixed

Prior issueStatus
Broken from .models importFixed (from ..models)
Robots unreachable → denyFixed (cache miss → allow)
Unbounded crawl downloadsFixed (shared fetch_uri + _read_limited)
Discovery fused with acquisitionFixed (discover_links returns URI list only)
Unbounded sitemap recursionFixed (depth / max_urls / max_total_size / DOCTYPE reject)
Duplicate content dropped URI provenanceFixed (always append manifest)
CLI always exit 0Fixed (nonzero on failures)
Package name scrapingFixed (flossware-scraping)
Empty .gitignoreFixed
Thin tests / false confidenceExpanded (import, robots, size, redirect, sitemap, CLI, private IP, FTP mock)
README driftAligned with architecture and defaults

Architecture (discovery → URI set → fetch_uriAcquiredResource → corpus) now matches the README. SHA-256 identity + multi-URI manifest provenance is correct.


Remaining findings

Medium

  1. Double-fetch on link discovery (cli.py, discovery/web.py)

    • Discovery fetches every page to extract links; acquisition fetches the same URIs again.
    • Correct separation; 2× bandwidth and rate-limit pressure on real hosts.
    • Fix (optional for 0.1): document the cost clearly, or add a same-run transient body cache / optional acquire-during-discovery mode later.
  2. DNS TOCTOU / residual SSRF (acquisition/corpus.py)

    • Host is resolved and checked once; the later connect can hit a different address (rebinding).
    • Fix: pin to allowed IPs, or document as accepted residual risk for a local CLI before calling the surface “SSRF-safe.”
  3. getaddrinfo has no timeout (_blocked_address)

    • Hung DNS can stall the process indefinitely.
    • Fix: bound DNS, or document dependency on system resolver timeouts.
  4. crawl() compatibility alias drops security/limit knobs (discovery/web.py)

    • Alias does not forward timeout, max_size, allow_private.
    • Fix: forward all kwargs, or remove the alias before public API freeze.
  5. Empty invocation succeeds (cli.py)

    • scrape with no sources / --uris / --sitemap stores 0 and exits 0.
    • Fix: non-zero exit (or explicit warning) when zero targets were planned.

Low

  • Premature empty extracted/ / normalized/ dirs (documented layout, no consumers yet).
  • Weak media-type detection (acceptable for acquisition-only scope if documented).
  • No explicit max-redirects beyond urllib default.
  • Tests are fully mocked; no live local HTTP server end-to-end test.
  • FTP is “urllib under shared limits,” not deep FTP support — fine for 0.1 if not oversold.

Release readiness

GateStatus
Import / console scriptOK
Size limits on all fetch pathsOK
Robots allow-when-unreachableOK
Sitemap boundsOK
Private/link-local block + file:// redirect rejectOK
CLI exit codesOK
PyPI nameflossware-scraping
CI 3.10–3.14Green
Residual SSRF TOCTOU / DNS hangDocument or harden before claiming SSRF-safe
PyPI Trusted Publishing envMust be configured in repo settings before first tag

Recommended before tagging 0.1

  1. Document residual DNS-rebinding risk and DNS hang behavior, or harden the resolver path.
  2. Forward full kwargs from crawl alias (or delete it).
  3. Non-zero exit when no targets were provided.
  4. Document 2× fetch cost of discover-then-acquire.
  5. Confirm GitHub pypi environment + OIDC is configured before tagging 0.1.

Nothing remaining looks like a hard correctness break for an Alpha 0.1 local-first CLI, provided the residual SSRF/DNS caveats are explicit. After those small fixes or documented acceptances, this is ready to merge and tag.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@sfloess