From 36ac487cbdb766a382904cbe30440c309983ea98 Mon Sep 17 00:00:00 2001 From: Codex_B Date: Sat, 28 Feb 2026 13:47:21 +0000 Subject: [PATCH] Resolve PR review gaps across comments, security, feeds, and UX --- .gitea/workflows/ci.yml | 18 +++++++ .github/workflows/ci.yml | 18 +++++++ apps/blog/feeds.py | 11 +++- apps/blog/models.py | 15 ++++-- apps/blog/tests/test_feeds.py | 26 ++++++++++ apps/blog/tests/test_views.py | 65 ++++++++++++++++++++++++ apps/comments/tests/test_views.py | 69 ++++++++++++++++++++++++++ apps/comments/views.py | 18 ++++++- apps/core/templatetags/seo_tags.py | 7 ++- apps/core/tests/test_security.py | 35 +++++++++++++ apps/core/views.py | 7 +++ config/settings/base.py | 1 + templates/blog/article_index_page.html | 16 ++++++ templates/blog/article_page.html | 25 ++++++++++ templates/components/nav.html | 1 + 15 files changed, 325 insertions(+), 7 deletions(-) diff --git a/.gitea/workflows/ci.yml b/.gitea/workflows/ci.yml index 8110b22..358dbeb 100644 --- a/.gitea/workflows/ci.yml +++ b/.gitea/workflows/ci.yml @@ -2,6 +2,8 @@ name: CI on: pull_request: + schedule: + - cron: "0 2 * * *" concurrency: group: ci-pr-${{ github.event.pull_request.number || github.ref }} @@ -9,6 +11,7 @@ concurrency: jobs: ci: + if: github.event_name == 'pull_request' runs-on: ubuntu-latest env: CI_IMAGE: nohype-ci:${{ github.run_id }} @@ -30,3 +33,18 @@ jobs: - name: Remove CI image if: always() run: docker image rm -f "$CI_IMAGE" || true + + nightly-e2e: + if: github.event_name == 'schedule' + runs-on: ubuntu-latest + env: + CI_IMAGE: nohype-ci-nightly:${{ github.run_id }} + steps: + - uses: actions/checkout@v4 + - name: Build + run: docker build -t "$CI_IMAGE" . + - name: Nightly smoke journey + run: docker run --rm -e DATABASE_URL=sqlite:////tmp/ci.sqlite3 "$CI_IMAGE" pytest apps/core/tests/test_smoke.py + - name: Remove CI image + if: always() + run: docker image rm -f "$CI_IMAGE" || true diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 8110b22..358dbeb 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -2,6 +2,8 @@ name: CI on: pull_request: + schedule: + - cron: "0 2 * * *" concurrency: group: ci-pr-${{ github.event.pull_request.number || github.ref }} @@ -9,6 +11,7 @@ concurrency: jobs: ci: + if: github.event_name == 'pull_request' runs-on: ubuntu-latest env: CI_IMAGE: nohype-ci:${{ github.run_id }} @@ -30,3 +33,18 @@ jobs: - name: Remove CI image if: always() run: docker image rm -f "$CI_IMAGE" || true + + nightly-e2e: + if: github.event_name == 'schedule' + runs-on: ubuntu-latest + env: + CI_IMAGE: nohype-ci-nightly:${{ github.run_id }} + steps: + - uses: actions/checkout@v4 + - name: Build + run: docker build -t "$CI_IMAGE" . + - name: Nightly smoke journey + run: docker run --rm -e DATABASE_URL=sqlite:////tmp/ci.sqlite3 "$CI_IMAGE" pytest apps/core/tests/test_smoke.py + - name: Remove CI image + if: always() + run: docker image rm -f "$CI_IMAGE" || true diff --git a/apps/blog/feeds.py b/apps/blog/feeds.py index b7ecf59..efd8842 100644 --- a/apps/blog/feeds.py +++ b/apps/blog/feeds.py @@ -11,6 +11,10 @@ class AllArticlesFeed(Feed): link = "/articles/" description = "Honest AI coding tool reviews for developers." + def get_object(self, request): + self.request = request + return None + def items(self): return ArticlePage.objects.live().order_by("-first_published_at")[:20] @@ -27,11 +31,16 @@ class AllArticlesFeed(Feed): return item.author.name def item_link(self, item: ArticlePage): - return f"{settings.WAGTAILADMIN_BASE_URL}{item.url}" + if hasattr(self, "request") and self.request is not None: + full_url = item.get_full_url(self.request) + if full_url: + return full_url + return f"{settings.WAGTAILADMIN_BASE_URL.rstrip('/')}{item.url}" class TagArticlesFeed(AllArticlesFeed): def get_object(self, request, tag_slug: str): + self.request = request return get_object_or_404(Tag, slug=tag_slug) def title(self, obj): diff --git a/apps/blog/models.py b/apps/blog/models.py index c6e22d9..e6c8535 100644 --- a/apps/blog/models.py +++ b/apps/blog/models.py @@ -6,10 +6,10 @@ from typing import Any from django.core.paginator import EmptyPage, PageNotAnInteger, Paginator from django.db import models -from django.db.models import CASCADE, PROTECT, SET_NULL +from django.db.models import CASCADE, PROTECT, SET_NULL, Prefetch from modelcluster.contrib.taggit import ClusterTaggableManager from modelcluster.fields import ParentalKey -from taggit.models import TaggedItemBase +from taggit.models import Tag, TaggedItemBase from wagtail.admin.panels import FieldPanel, PageChooserPanel from wagtail.fields import RichTextField, StreamField from wagtail.models import Page @@ -62,6 +62,9 @@ class ArticleIndexPage(Page): ctx = super().get_context(request, *args, **kwargs) tag_slug = request.GET.get("tag") articles = self.get_articles() + available_tags = ( + Tag.objects.filter(id__in=articles.values_list("tags__id", flat=True)).distinct().order_by("name") + ) if tag_slug: articles = articles.filter(tags__slug=tag_slug) paginator = Paginator(articles, self.ARTICLES_PER_PAGE) @@ -75,6 +78,7 @@ class ArticleIndexPage(Page): ctx["articles"] = page_obj ctx["paginator"] = paginator ctx["active_tag"] = tag_slug + ctx["available_tags"] = available_tags return ctx @@ -168,8 +172,11 @@ class ArticlePage(SeoMixin, Page): def get_context(self, request, *args, **kwargs): ctx = super().get_context(request, *args, **kwargs) ctx["related_articles"] = self.get_related_articles() - ctx["approved_comments"] = self.comments.filter(is_approved=True, parent__isnull=True).select_related( - "parent" + from apps.comments.models import Comment + + approved_replies = Comment.objects.filter(is_approved=True).select_related("parent") + ctx["approved_comments"] = self.comments.filter(is_approved=True, parent__isnull=True).prefetch_related( + Prefetch("replies", queryset=approved_replies) ) return ctx diff --git a/apps/blog/tests/test_feeds.py b/apps/blog/tests/test_feeds.py index 0fbfd76..b1112f0 100644 --- a/apps/blog/tests/test_feeds.py +++ b/apps/blog/tests/test_feeds.py @@ -1,4 +1,8 @@ import pytest +from django.test import override_settings + +from apps.blog.models import ArticleIndexPage, ArticlePage +from apps.blog.tests.factories import AuthorFactory @pytest.mark.django_db @@ -6,3 +10,25 @@ def test_feed_endpoint(client): resp = client.get("/feed/") assert resp.status_code == 200 assert resp["Content-Type"].startswith("application/rss+xml") + + +@pytest.mark.django_db +@override_settings(WAGTAILADMIN_BASE_URL="http://wrong-host.example") +def test_feed_uses_request_host_for_item_links(client, home_page): + index = ArticleIndexPage(title="Articles", slug="articles") + home_page.add_child(instance=index) + author = AuthorFactory() + article = ArticlePage( + title="Feed Article", + slug="feed-article", + author=author, + summary="summary", + body=[("rich_text", "

Body

")], + ) + index.add_child(instance=article) + article.save_revision().publish() + + resp = client.get("/feed/") + body = resp.content.decode() + assert resp.status_code == 200 + assert "http://localhost/articles/feed-article/" in body diff --git a/apps/blog/tests/test_views.py b/apps/blog/tests/test_views.py index 138531f..d30a212 100644 --- a/apps/blog/tests/test_views.py +++ b/apps/blog/tests/test_views.py @@ -1,7 +1,9 @@ import pytest +from taggit.models import Tag from apps.blog.models import ArticleIndexPage, ArticlePage from apps.blog.tests.factories import AuthorFactory +from apps.comments.models import Comment @pytest.mark.django_db @@ -29,6 +31,7 @@ def test_article_index_pagination_and_tag_filter(client, home_page): resp = client.get("/articles/?page=2") assert resp.status_code == 200 assert resp.context["articles"].number == 2 + assert "Pagination" in resp.content.decode() @pytest.mark.django_db @@ -92,3 +95,65 @@ def test_article_page_renders_share_links_and_newsletter_form(client, home_page) assert "Share on LinkedIn" in html assert 'data-copy-link' in html assert 'name="source" value="article"' in html + + +@pytest.mark.django_db +def test_article_page_renders_approved_comments_and_reply_form(client, home_page): + index = ArticleIndexPage(title="Articles", slug="articles") + home_page.add_child(instance=index) + author = AuthorFactory() + article = ArticlePage( + title="Main", + slug="main", + author=author, + summary="summary", + body=[("rich_text", "

body

")], + ) + index.add_child(instance=article) + article.save_revision().publish() + comment = Comment.objects.create( + article=article, + author_name="A", + author_email="a@example.com", + body="Top level", + is_approved=True, + ) + Comment.objects.create( + article=article, + parent=comment, + author_name="B", + author_email="b@example.com", + body="Reply", + is_approved=True, + ) + + resp = client.get("/articles/main/") + html = resp.content.decode() + assert resp.status_code == 200 + assert "Top level" in html + assert "Reply" in html + assert f'name="parent_id" value="{comment.id}"' in html + + +@pytest.mark.django_db +def test_article_index_renders_tag_filter_controls(client, home_page): + index = ArticleIndexPage(title="Articles", slug="articles") + home_page.add_child(instance=index) + author = AuthorFactory() + article = ArticlePage( + title="Main", + slug="main", + author=author, + summary="summary", + body=[("rich_text", "

body

")], + ) + index.add_child(instance=article) + article.save_revision().publish() + tag = Tag.objects.create(name="TagOne", slug="tag-one") + article.tags.add(tag) + article.save_revision().publish() + + resp = client.get("/articles/") + html = resp.content.decode() + assert resp.status_code == 200 + assert "/articles/?tag=tag-one" in html diff --git a/apps/comments/tests/test_views.py b/apps/comments/tests/test_views.py index 74a20d8..189e328 100644 --- a/apps/comments/tests/test_views.py +++ b/apps/comments/tests/test_views.py @@ -1,5 +1,6 @@ import pytest from django.core.cache import cache +from django.test import override_settings from apps.blog.models import ArticleIndexPage, ArticlePage from apps.blog.tests.factories import AuthorFactory @@ -60,3 +61,71 @@ def test_comment_post_rejected_when_comments_disabled(client, home_page): ) assert resp.status_code == 404 assert Comment.objects.count() == 0 + + +@pytest.mark.django_db +def test_comment_reply_depth_is_enforced(client, home_page): + cache.clear() + index = ArticleIndexPage(title="Articles", slug="articles") + home_page.add_child(instance=index) + author = AuthorFactory() + article = ArticlePage(title="A", slug="a", author=author, summary="s", body=[("rich_text", "

body

")]) + index.add_child(instance=article) + article.save_revision().publish() + + parent = Comment.objects.create( + article=article, + author_name="Parent", + author_email="p@example.com", + body="Parent", + is_approved=True, + ) + child = Comment.objects.create( + article=article, + parent=parent, + author_name="Child", + author_email="c@example.com", + body="Child", + is_approved=True, + ) + + resp = client.post( + "/comments/post/", + { + "article_id": article.id, + "parent_id": child.id, + "author_name": "TooDeep", + "author_email": "deep@example.com", + "body": "Nope", + "honeypot": "", + }, + ) + assert resp.status_code == 302 + assert Comment.objects.count() == 2 + + +@pytest.mark.django_db +@override_settings(TRUSTED_PROXY_IPS=[]) +def test_comment_uses_remote_addr_when_proxy_untrusted(client, home_page): + cache.clear() + index = ArticleIndexPage(title="Articles", slug="articles") + home_page.add_child(instance=index) + author = AuthorFactory() + article = ArticlePage(title="A", slug="a", author=author, summary="s", body=[("rich_text", "

body

")]) + index.add_child(instance=article) + article.save_revision().publish() + + client.post( + "/comments/post/", + { + "article_id": article.id, + "author_name": "Test", + "author_email": "test@example.com", + "body": "Hello", + "honeypot": "", + }, + REMOTE_ADDR="10.0.0.1", + HTTP_X_FORWARDED_FOR="203.0.113.7", + ) + comment = Comment.objects.get() + assert comment.ip_address == "10.0.0.1" diff --git a/apps/comments/views.py b/apps/comments/views.py index 243e16f..1c9a036 100644 --- a/apps/comments/views.py +++ b/apps/comments/views.py @@ -1,7 +1,9 @@ from __future__ import annotations +from django.conf import settings from django.contrib import messages from django.core.cache import cache +from django.core.exceptions import ValidationError from django.http import HttpResponse from django.shortcuts import get_object_or_404, redirect from django.views import View @@ -11,9 +13,18 @@ from apps.comments.forms import CommentForm from apps.comments.models import Comment +def client_ip_from_request(request) -> str: + remote_addr = request.META.get("REMOTE_ADDR", "").strip() + trusted_proxies = getattr(settings, "TRUSTED_PROXY_IPS", []) + x_forwarded_for = request.META.get("HTTP_X_FORWARDED_FOR", "") + if remote_addr in trusted_proxies and x_forwarded_for: + return x_forwarded_for.split(",")[0].strip() + return remote_addr + + class CommentCreateView(View): def post(self, request): - ip = (request.META.get("HTTP_X_FORWARDED_FOR") or request.META.get("REMOTE_ADDR", "")).split(",")[0].strip() + ip = client_ip_from_request(request) key = f"comment-rate:{ip}" count = cache.get(key, 0) if count >= 3: @@ -34,6 +45,11 @@ class CommentCreateView(View): if parent_id: comment.parent = Comment.objects.filter(pk=parent_id, article=article).first() comment.ip_address = ip or None + try: + comment.full_clean() + except ValidationError: + messages.error(request, "Reply depth exceeds the allowed limit") + return redirect(article.url) comment.save() messages.success(request, "Your comment is awaiting moderation") return redirect(f"{article.url}?commented=1") diff --git a/apps/core/templatetags/seo_tags.py b/apps/core/templatetags/seo_tags.py index fc71c59..c5ddb36 100644 --- a/apps/core/templatetags/seo_tags.py +++ b/apps/core/templatetags/seo_tags.py @@ -37,6 +37,7 @@ def article_og_image_url(context, article) -> str: @register.simple_tag(takes_context=True) def article_json_ld(context, article): request = context["request"] + nonce = getattr(request, "csp_nonce", "") data = { "@context": "https://schema.org", "@type": "Article", @@ -49,5 +50,9 @@ def article_json_ld(context, article): "image": _article_image_url(request, article), } return mark_safe( - '" + '" ) diff --git a/apps/core/tests/test_security.py b/apps/core/tests/test_security.py index 78836c6..3508317 100644 --- a/apps/core/tests/test_security.py +++ b/apps/core/tests/test_security.py @@ -64,3 +64,38 @@ def test_article_comment_form_contains_csrf_token(client, home_page): html = resp.content.decode() assert resp.status_code == 200 assert "csrfmiddlewaretoken" in html + + +@pytest.mark.django_db +def test_consent_rejects_open_redirect(client, home_page): + resp = client.post( + "/consent/", + {"reject_all": "1"}, + HTTP_REFERER="https://evil.example.com/phish", + ) + assert resp.status_code == 302 + assert resp["Location"] == "/" + + +@pytest.mark.django_db +def test_article_json_ld_script_has_csp_nonce(client, home_page): + index = ArticleIndexPage(title="Articles", slug="articles") + home_page.add_child(instance=index) + author = AuthorFactory() + article = ArticlePage( + title="Nonce Article", + slug="nonce-article", + author=author, + summary="summary", + body=[("rich_text", "

Body

")], + ) + index.add_child(instance=article) + article.save_revision().publish() + + resp = client.get("/articles/nonce-article/") + csp = resp["Content-Security-Policy"] + match = re.search(r"nonce-([^' ;]+)", csp) + assert match + nonce = match.group(1) + html = resp.content.decode() + assert f'type="application/ld+json" nonce="{nonce}"' in html diff --git a/apps/core/views.py b/apps/core/views.py index d2b93e2..bbfeb4f 100644 --- a/apps/core/views.py +++ b/apps/core/views.py @@ -2,6 +2,7 @@ from __future__ import annotations from django.http import HttpRequest, HttpResponse, HttpResponseNotAllowed from django.shortcuts import redirect, render +from django.utils.http import url_has_allowed_host_and_scheme from apps.core.consent import ConsentService @@ -24,6 +25,12 @@ def consent_view(request: HttpRequest) -> HttpResponse: advertising = request.POST.get("advertising") in {"true", "1", "on"} target = request.META.get("HTTP_REFERER", "/") + if not url_has_allowed_host_and_scheme( + url=target, + allowed_hosts={request.get_host()}, + require_https=request.is_secure(), + ): + target = "/" response = redirect(target) ConsentService.set_consent(response, analytics=analytics, advertising=advertising) return response diff --git a/config/settings/base.py b/config/settings/base.py index 17bca2a..483e9b2 100644 --- a/config/settings/base.py +++ b/config/settings/base.py @@ -134,5 +134,6 @@ SECURE_CONTENT_TYPE_NOSNIFF = True X_CONTENT_TYPE_OPTIONS = "nosniff" CSRF_TRUSTED_ORIGINS = [u for u in os.getenv("CSRF_TRUSTED_ORIGINS", "http://localhost:8035").split(",") if u] +TRUSTED_PROXY_IPS = [ip.strip() for ip in os.getenv("TRUSTED_PROXY_IPS", "").split(",") if ip.strip()] STATICFILES_STORAGE = "whitenoise.storage.CompressedManifestStaticFilesStorage" diff --git a/templates/blog/article_index_page.html b/templates/blog/article_index_page.html index b39d72f..edeafc4 100644 --- a/templates/blog/article_index_page.html +++ b/templates/blog/article_index_page.html @@ -11,9 +11,25 @@ {% endblock %} {% block content %}

{{ page.title }}

+
+

Filter by tag

+ All + {% for tag in available_tags %} + {{ tag.name }} + {% endfor %} +
{% for article in articles %} {% include 'components/article_card.html' with article=article %} {% empty %}

No articles found.

{% endfor %} + {% endblock %} diff --git a/templates/blog/article_page.html b/templates/blog/article_page.html index 97a105c..3500be1 100644 --- a/templates/blog/article_page.html +++ b/templates/blog/article_page.html @@ -44,6 +44,31 @@ {% if page.comments_enabled %}
+

Comments

+ {% for comment in approved_comments %} +
+

{{ comment.author_name }}

+

{{ comment.body }}

+ {% for reply in comment.replies.all %} +
+

{{ reply.author_name }}

+

{{ reply.body }}

+
+ {% endfor %} +
+ {% csrf_token %} + + + + + + + +
+
+ {% empty %} +

No comments yet.

+ {% endfor %}
{% csrf_token %} diff --git a/templates/components/nav.html b/templates/components/nav.html index b6d12e2..9ab902a 100644 --- a/templates/components/nav.html +++ b/templates/components/nav.html @@ -2,5 +2,6 @@ Home Articles About + {% include 'components/newsletter_form.html' with source='nav' label='Get updates' %}