From e93b5cee3e4ff7ed2c5ccc96b5215be0b1f24cb9 Mon Sep 17 00:00:00 2001 From: Jarian Cottingham Date: Sat, 4 Jul 2026 05:23:05 +0000 Subject: [PATCH] fix: security hardening - auth, CSRF, path traversal, XSS, secrets, headers - #3: Path traversal fix in /archive and /archive-file routes via resolve() check - #4: SSRF mitigation - env-based SERVER_URL, no hardcoded internal IPs - #5: Stored XSS fix - remove |safe filter from article.html template - #6: Missing import os in scheduler.py (crash on import) - #7: Flask auth (password via NEWSARCHIVER_PASSWORD) + CSRF tokens - #8: Same as #5 (template XSS via |safe) - #9: Motley Fool API key removed - use env var interpolation - #10: Hardcoded paths in setup_cron.sh, stop_services.sh - use BASH_SOURCE - #11: Hardcoded user paths in singlefile_archive.py - use Path.home() - #16: HTTP RSS feeds updated to HTTPS (Barchart, Guardian, BBC, MarketWatch) - #24: SSRF - replace hardcoded 192.168.8.150:5000 with NEWSARCHIVER_SERVER_URL - #25: Command execution details sanitized in error messages - #26: Security headers (X-Content-Type-Options, X-Frame-Options, X-XSS-Protection, Referrer-Policy, CSP) - #27: Auth guard on all routes except RSS/Atom feeds - archive_engine.py: Add missing import os --- archive_engine.py | 1 + rss_feeds.json | 10 +- scheduler.py | 1 + setup_cron.sh | 4 +- singlefile_archive.py | 5 +- stop_services.sh | 3 +- templates/article.html | 2 +- templates/login.html | 95 +++++++++++++++ web_interface.py | 261 +++++++++++++++++++++++++++++++++-------- 9 files changed, 323 insertions(+), 59 deletions(-) create mode 100644 templates/login.html diff --git a/archive_engine.py b/archive_engine.py index 97098a5..33e782b 100644 --- a/archive_engine.py +++ b/archive_engine.py @@ -11,6 +11,7 @@ Orchestrates the archiving process: import argparse import json import logging +import os import sys from datetime import datetime from pathlib import Path diff --git a/rss_feeds.json b/rss_feeds.json index 1b8394c..dd75d45 100644 --- a/rss_feeds.json +++ b/rss_feeds.json @@ -27,7 +27,7 @@ }, "The Motley Fool – Stock News & Analysis": { "source_website": "fool.com", - "rss_url": "https://www.fool.com/a/feeds/partner/googlechromefollow?apikey=5e092c1f-c5f9-4428-9219-908a47d2e2de", + "rss_url": "https://www.fool.com/a/feeds/partner/googlechromefollow?apikey=${MOTLEY_FOOL_API_KEY}", "entries": 50, "validated_at": "2026-03-18T20:21:48.479725" }, @@ -118,13 +118,13 @@ }, "Barchart News": { "source_website": "barchart.com", - "rss_url": "http://feeds.feedburner.com/BarchartNews", + "rss_url": "https://feeds.feedburner.com/BarchartNews", "entries": 15, "validated_at": "2026-03-18T20:22:30.831390" }, "The Guardian – Business": { "source_website": "theguardian.com", - "rss_url": "http://feeds.theguardian.com/theguardian/uk/business/rss", + "rss_url": "https://feeds.theguardian.com/theguardian/uk/business/rss", "entries": 40, "validated_at": "2026-03-18T20:22:32.033297" }, @@ -142,7 +142,7 @@ }, "BBC News – Business": { "source_website": "bbc.co.uk", - "rss_url": "http://feeds.bbci.co.uk/news/business/rss.xml", + "rss_url": "https://feeds.bbci.co.uk/news/business/rss.xml", "entries": 56, "validated_at": "2026-03-18T20:22:36.643323" }, @@ -154,7 +154,7 @@ }, "MarketWatch – Top Stories": { "source_website": "marketwatch.com", - "rss_url": "http://feeds.marketwatch.com/marketwatch/topstories/", + "rss_url": "https://feeds.marketwatch.com/marketwatch/topstories/", "entries": 10, "validated_at": "2026-03-18T20:22:45.591752" }, diff --git a/scheduler.py b/scheduler.py index 82f87f8..4b8fa11 100644 --- a/scheduler.py +++ b/scheduler.py @@ -7,6 +7,7 @@ daily archiving of news sources. import atexit import logging +import os import signal import sys import time diff --git a/setup_cron.sh b/setup_cron.sh index 1f4ccae..8493ffe 100644 --- a/setup_cron.sh +++ b/setup_cron.sh @@ -4,8 +4,8 @@ set -e -SCRIPT_DIR="/home/user/playground/NewsArchiver" -LOG_FILE="/tmp/newsarchiver_cron.log" +SCRIPT_DIR="${NEWSARCHIVER_DIR:-$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)}" +LOG_FILE="${NEWSARCHIVER_LOG:-/tmp/newsarchiver_cron.log}" CRON_JOB="*/30 * * * * /usr/bin/env python3 ${SCRIPT_DIR}/run_archiver.py --interval 30 > ${LOG_FILE} 2>&1" echo "=== NewsArchiver Setup Script ===" diff --git a/singlefile_archive.py b/singlefile_archive.py index 951a0ce..7f95018 100644 --- a/singlefile_archive.py +++ b/singlefile_archive.py @@ -34,11 +34,12 @@ def _get_singlefile_path() -> Optional[str]: return _SINGLEFILE_PATH # Check common locations + home = Path.home() common_locations = [ - '/home/user/.local/bin/single-file', + str(home / '.local' / 'bin' / 'single-file'), '/usr/local/bin/single-file', '/usr/bin/single-file', - '/home/user/.npm/_global/bin/single-file', + str(home / '.npm' / '_global' / 'bin' / 'single-file'), ] for candidate in common_locations: diff --git a/stop_services.sh b/stop_services.sh index 49d1d66..50338c0 100644 --- a/stop_services.sh +++ b/stop_services.sh @@ -12,7 +12,8 @@ pkill -f "run_archiver.py --interval" 2>/dev/null || true echo "Scheduler stopped" # Remove lock file -rm -f /home/user/playground/NewsArchiver/archival_data/.scheduler.lock 2>/dev/null || true +LOCK_FILE="${NEWSARCHIVER_DIR:-$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)}/archival_data/.scheduler.lock" +rm -f "$LOCK_FILE" 2>/dev/null || true echo "Scheduler lock file removed" echo "All NewsArchiver services stopped" \ No newline at end of file diff --git a/templates/article.html b/templates/article.html index 6e3ab8d..8f3bfa6 100644 --- a/templates/article.html +++ b/templates/article.html @@ -28,7 +28,7 @@ {% set lines = article.content_text.split('\n') -%} {%- for line in lines %} {%- if line|trim %} -

{{ line|safe }}

+

{{ line }}

{%- endif %} {%- endfor %} diff --git a/templates/login.html b/templates/login.html new file mode 100644 index 0000000..c293260 --- /dev/null +++ b/templates/login.html @@ -0,0 +1,95 @@ +{% extends "base.html" %} + +{% block content %} +
+ +
+ + +{% endblock %} diff --git a/web_interface.py b/web_interface.py index 612104c..35a0672 100644 --- a/web_interface.py +++ b/web_interface.py @@ -4,17 +4,33 @@ Flask web server for browsing archived news articles. """ +import hashlib +import hmac +import html import json import logging import os +import secrets import sys import xml.etree.ElementTree as ET from datetime import datetime, timezone +from functools import wraps from pathlib import Path from typing import Optional -from urllib.parse import quote +from urllib.parse import quote, urlparse -from flask import Flask, jsonify, make_response, render_template, request +from flask import ( + Flask, + abort, + flash, + jsonify, + make_response, + redirect, + render_template, + request, + session, + url_for, +) from storage_manager import ( DB_PATH, @@ -33,32 +49,92 @@ RSS_FEEDS_PATH = SCRIPT_DIR / "rss_feeds.json" RSS_FEEDS = {} +# Authentication configuration +ADMIN_PASSWORD = os.environ.get("NEWSARCHIVER_PASSWORD", "") +SECRET_KEY = os.environ.get( + "NEWSARCHIVER_SECRET_KEY", secrets.token_hex(32) +) +SESSION_LIFETIME_MINUTES = int(os.environ.get("NEWSARCHIVER_SESSION_MINUTES", "480")) -def load_rss_feeds() -> dict: - """Load RSS feeds configuration.""" - global RSS_FEEDS - - if RSS_FEEDS: - return RSS_FEEDS - - if not RSS_FEEDS_PATH.exists(): - logger.warning("RSS feeds file not found: %s", RSS_FEEDS_PATH) - return {} - - try: - with open(RSS_FEEDS_PATH, "r", encoding="utf-8") as f: - RSS_FEEDS = json.load(f) - return RSS_FEEDS - except Exception as e: - logger.error("Failed to load RSS feeds: %s", str(e)) - return {} - +# Server URL configuration +SERVER_URL = os.environ.get( + "NEWSARCHIVER_SERVER_URL", f"http://localhost:5000" +) app = Flask( __name__, static_folder=str(SCRIPT_DIR / "static"), template_folder=str(SCRIPT_DIR / "templates"), ) +app.secret_key = SECRET_KEY + + +def login_required(f): + """Decorator to require login for protected routes.""" + + @wraps(f) + def decorated_function(*args, **kwargs): + if not ADMIN_PASSWORD: + return f(*args, **kwargs) + if "authenticated" not in session: + flash("Please log in to access this page.", "error") + return redirect(url_for("login", next=request.url)) + return f(*args, **kwargs) + + return decorated_function + + +def generate_csrf_token(): + """Generate a CSRF token tied to the session.""" + if "csrf_token" not in session: + session["csrf_token"] = secrets.token_hex(32) + return session["csrf_token"] + + +def verify_csrf_token(): + """Verify the CSRF token from the request.""" + if not ADMIN_PASSWORD: + return True + token = request.form.get("csrf_token") or request.headers.get("X-CSRF-Token") + if not token or "csrf_token" not in session: + return False + return hmac.compare_digest(token, session["csrf_token"]) + + +@app.template_global() +def csrf_token(): + """Make CSRF token available in templates.""" + return generate_csrf_token() + + +@app.before_request +def enforce_session_timeout(): + """Expire sessions after inactivity.""" + if "authenticated" in session: + auth_time = session.get("auth_time", 0) + now = datetime.now().timestamp() + if now - auth_time > SESSION_LIFETIME_MINUTES * 60: + session.clear() + + +@app.after_request +def add_security_headers(response): + """Add security headers to all responses.""" + response.headers["X-Content-Type-Options"] = "nosniff" + response.headers["X-Frame-Options"] = "DENY" + response.headers["X-XSS-Protection"] = "1; mode=block" + response.headers["Referrer-Policy"] = "strict-origin-when-cross-origin" + response.headers["Cache-Control"] = "no-store" + if not request.is_secure: + response.headers["Content-Security-Policy"] = ( + "default-src 'self'; " + "script-src 'self'; " + "style-src 'self' 'unsafe-inline'; " + "img-src 'self' data:; " + "frame-ancestors 'none'" + ) + return response + logging.basicConfig( level=logging.INFO, @@ -96,7 +172,53 @@ def get_pagination_info(total: int, page: int, per_page: int) -> dict: } +def sanitize_error_message(msg: str) -> str: + """Sanitize error messages to prevent information leakage.""" + sanitized = msg + for pattern in [r"\d{1,3}(\.\d{1,3}){3}", r"/home/\w+"]: + import re + + sanitized = re.sub(pattern, "[REDACTED]", sanitized) + return sanitized + + +@app.route("/login", methods=["GET", "POST"]) +def login(): + """Login page.""" + if not ADMIN_PASSWORD: + return redirect(url_for("index")) + + if request.method == "POST": + if not verify_csrf_token(): + flash("Invalid request.", "error") + return redirect(url_for("login")) + + password = request.form.get("password", "") + if hmac.compare_digest(password, ADMIN_PASSWORD): + session.clear() + session["authenticated"] = True + session["csrf_token"] = secrets.token_hex(32) + session["auth_time"] = datetime.now().timestamp() + next_url = request.form.get("next", url_for("index")) + parsed = urlparse(next_url) + if parsed.netloc: + next_url = url_for("index") + return redirect(next_url) + else: + flash("Invalid password.", "error") + + return render_template("login.html", csrf_token=csrf_token()) + + +@app.route("/logout") +def logout(): + """Logout.""" + session.clear() + return redirect(url_for("index")) + + @app.route("/") +@login_required def index(): """Newspaper listing page.""" sources = get_all_sources() @@ -134,6 +256,7 @@ def index(): @app.route("/source/") +@login_required def articles(slug: str): """Article listing page for a specific source.""" page = request.args.get("page", 1, type=int) @@ -178,32 +301,50 @@ def articles(slug: str): ) +def validate_archive_path(archive_path: str) -> Optional[Path]: + """Validate that an archive path is within ARCHIVE_DIR (prevents path traversal). + + Args: + archive_path: Requested path component + + Returns: + Resolved Path if valid, None if traversal detected + """ + try: + archive_file = (ARCHIVE_DIR / archive_path).resolve() + if not str(archive_file).startswith(str(ARCHIVE_DIR)): + logger.warning("Path traversal attempt blocked: %s", archive_path) + return None + return archive_file + except (ValueError, OSError): + return None + + @app.route("/archive/") +@login_required def serve_archive(archive_path): """Serve archived HTML file.""" - archive_file = ARCHIVE_DIR / archive_path - if archive_file.exists(): + archive_file = validate_archive_path(archive_path) + if archive_file and archive_file.exists(): return archive_file.read_text(encoding="utf-8") return "Archive not found", 404 @app.route("/archive-file/") +@login_required def serve_archive_file(encoded_path): """Serve archived HTML file from encoded path.""" import urllib.parse - from pathlib import Path archive_path = urllib.parse.unquote(encoded_path) - archive_file = ARCHIVE_DIR / archive_path - logger.info( - "Archive file path: %s, exists: %s", str(archive_file), archive_file.exists() - ) - if archive_file.exists(): + archive_file = validate_archive_path(archive_path) + if archive_file and archive_file.exists(): return archive_file.read_text(encoding="utf-8") return "Archive not found", 404 @app.route("/source//article/") +@login_required def article(slug: str, article_id: int): """Individual article page.""" sources = get_all_sources() @@ -243,6 +384,7 @@ def article(slug: str, article_id: int): @app.route("/status") +@login_required def status(): """System status page.""" sources = get_all_sources() @@ -290,6 +432,7 @@ def status(): @app.route("/api/sources") +@login_required def api_sources(): """API endpoint for listing all sources.""" sources = get_all_sources() @@ -327,6 +470,7 @@ def api_sources(): @app.route("/api/source//articles") +@login_required def api_articles(slug: str): """API endpoint for listing articles for a source.""" page = request.args.get("page", 1, type=int) @@ -376,6 +520,7 @@ def api_articles(slug: str): @app.route("/api/status") +@login_required def api_status(): """API endpoint for system status.""" sources = get_all_sources() @@ -413,8 +558,6 @@ def rss_feed(): articles = get_latest_articles(limit=limit) - server_url = f"http://192.168.8.150:5000" - rss_items = [] for article in articles: if ( @@ -441,22 +584,22 @@ def rss_feed(): source_name = article.source_name or "unknown" encoded_source = quote(source_name.lower()) item = { - "title": article.title, - "link": f"{server_url}/source/{encoded_source}/article/{article.id}", + "title": html.escape(article.title), + "link": f"{SERVER_URL}/source/{encoded_source}/article/{article.id}", "pubDate": pub_date, - "description": article.content_text[:500] - if article.content_text - else "", - "guid": article.url or f"article-{article.id}", + "description": html.escape( + article.content_text[:500] if article.content_text else "" + ), + "guid": html.escape(article.url or f"article-{article.id}"), } if article.author: - item["author"] = article.author + item["author"] = html.escape(article.author) rss_items.append(item) rss_template = render_template( "rss.xml", title="NewsArchiver - Latest Articles", - link=server_url, + link=SERVER_URL, description="Latest archived news articles", last_build_date=datetime.now(timezone.utc).strftime( "%a, %d %b %Y %H:%M:%S +0000" @@ -476,8 +619,6 @@ def atom_feed(): articles = get_latest_articles(limit=limit) - server_url = f"http://192.168.8.150:5000" - atom_entries = [] for article in articles: if ( @@ -500,20 +641,22 @@ def atom_feed(): source_name = article.source_name or "unknown" encoded_source = quote(source_name.lower()) entry = { - "title": article.title, - "link": f"{server_url}/source/{encoded_source}/article/{article.id}", + "title": html.escape(article.title), + "link": f"{SERVER_URL}/source/{encoded_source}/article/{article.id}", "published": pub_date, - "summary": article.content_text[:500] if article.content_text else "", - "id": article.url or f"article-{article.id}", + "summary": html.escape( + article.content_text[:500] if article.content_text else "" + ), + "id": html.escape(article.url or f"article-{article.id}"), } if article.author: - entry["author"] = {"name": article.author} + entry["author"] = {"name": html.escape(article.author)} atom_entries.append(entry) atom_template = render_template( "atom.xml", title="NewsArchiver - Latest Articles", - link=server_url, + link=SERVER_URL, updated=datetime.now(timezone.utc).strftime("%Y-%m-%dT%H:%M:%S+00:00"), entries=atom_entries, ) @@ -523,6 +666,28 @@ def atom_feed(): return response +def load_rss_feeds() -> dict: + """Load RSS feeds configuration, resolving environment variables in API keys.""" + global RSS_FEEDS + + if RSS_FEEDS: + return RSS_FEEDS + + if not RSS_FEEDS_PATH.exists(): + logger.warning("RSS feeds file not found: %s", RSS_FEEDS_PATH) + return {} + + try: + raw_content = RSS_FEEDS_PATH.read_text(encoding="utf-8") + for key, value in os.environ.items(): + raw_content = raw_content.replace(f"${{{key}}}", value) + RSS_FEEDS = json.loads(raw_content) + return RSS_FEEDS + except Exception as e: + logger.error("Failed to load RSS feeds: %s", sanitize_error_message(str(e))) + return {} + + if __name__ == "__main__": logger.info("Starting web interface...") @@ -532,4 +697,4 @@ if __name__ == "__main__": initialize_storage() - app.run(host="0.0.0.0", port=5000, debug=True) + app.run(host="0.0.0.0", port=5000, debug=False)