Download and store full article text at ingest #32
marvin8
Loading…
Reference in a new issue
No description provided.
Delete branch "feat/issue-23-full-text"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Full text is fetched when an article is first ingested and stored as markdown, so the reader shows the whole story instead of the feed teaser.
articles.full_text+full_text_fetched_at(NULL = teaser-only)._EXCERPT_MAX, tunable).Design decisions per the issue discussion (ingest-time fetch chosen over lazy/backfill).
Closes #23
WuMing
Found 2 issue(s). See inline comments below.
@ -583,0 +639,4 @@entry = _build_article_entry(1, "T", long_text, "https://example.com/a")assert long_text[:_EXCERPT_MAX] in entrytests [MEDIUM]
The excerpt test imports the limit from the code under test (
_EXCERPT_MAX) and assertslong_text[:_EXCERPT_MAX] in entry, so the assertion always matches whatever value the constant has — reverting the cap to the old 300 characters would still pass. Only the"…" in entrycheck pins anything. Assert against the literal expected cap (e.g.assert len(_build_article_entry(...).split('\n')[-1]) <= 1000 + 1or check that text beyond 1000 chars is absent) so a regression in the cap value is actually caught.@ -721,0 +754,4 @@response = client.get(f"/article/{article.id}")assert b"<h1>" in response.datatests [LOW]
assert b"<h1>" in response.datais likely satisfied by the article page template's own title heading, so it does not verify that the markdown heading in the stored full text was rendered. Assert on the rendered heading text instead (e.g.b"<h1>Wind project approved</h1>"inresponse.data) to make the assertion specific to the full-text rendering path.WuMing
Found 2 issue(s). See inline comments below.
@ -0,0 +60,4 @@_validate_feed_url(url)response = _fetch_feed_response(url)except FetchError as exc:logger.warning(f"Full-text fetch skipped for {url}: {exc}")security [LOW]
A09: Logs the untrusted article URL directly. URLs can contain query-string credentials/tokens or CRLF sequences, leading to sensitive data disclosure and log injection. Redact query parameters and strip control characters before logging.
@ -0,0 +89,4 @@try:full_text = fetch_full_text(article.url)except Exception:logger.error(f"Unexpected full-text fetch error for {article.url}", exc_info=True)security [LOW]
A09: Logs the untrusted article URL directly. URLs can contain query-string credentials/tokens or CRLF sequences, leading to sensitive data disclosure and log injection. Redact query parameters and strip control characters before logging.
✅ Fixed — both test-strength findings
tests/test_aggregator.py·tests/test_web.pyBoth fair. The excerpt test now hardcodes the 1000-char boundary (with a comment tying it to
_EXCERPT_MAXso retunes update it deliberately), asserts the ellipsis, and asserts the full uncapped text is absent. The markdown-heading assertion now checks<h1>Wind project approved</h1>— the rendered heading text, which the template's own title cannot satisfy. Pushed as25e1e41; the first push also surfaced a legacy-migration fixture that needed the new columns rewound, fixed in8af76f9.WuMing
Found 2 issue(s). See inline comments below.
@ -25,6 +25,10 @@ dependencies = ["feedparser~=6.0.14","flask~=3.1.3","httpx2~=2.12.0","markupsafe~=3.0.3",config [MEDIUM]
markupsafe~=3.0.3is added as a direct runtime dependency, but MarkupSafe is already pulled in transitively by Flask/Jinja2 (both declared just above), and nothing in the described feature (trafilatura extraction, mistune rendering, nh3 sanitization) needs it directly. More importantly, pylock.toml in this diff adds/updates entries for every other new dependency (mistune, nh3, trafilatura and all of trafilatura's transitive deps) but contains no MarkupSafe entry, so if the locked MarkupSafe version is < 3.0.3 the lockfile no longer satisfies pyproject and reproducible installs (uv sync --locked/ CI) will fail. Either drop this line or regenerate pylock.toml so the resolved MarkupSafe version satisfies~=3.0.3.@ -0,0 +58,4 @@"""try:_validate_feed_url(url)response = _fetch_feed_response(url)security [MEDIUM]
Article URLs come from untrusted feed entries and are passed to an HTTP client after only _validate_feed_url. Per the PR this is a deny-list of private/reserved addresses rather than an allow-list, so SSRF is still possible via DNS rebinding, alternate IP encodings, or a missing per-hop redirect revalidation. Use a strict allow-list of schemes/hosts and pin the resolved IP for the HTTP client.