Skip to content

[codex] Mark watch viewed when opening page link#4253

Open
archit-goyal wants to merge 3 commits into
dgtlmoon:masterfrom
archit-goyal:fix/open-watch-marks-viewed
Open

[codex] Mark watch viewed when opening page link#4253
archit-goyal wants to merge 3 commits into
dgtlmoon:masterfrom
archit-goyal:fix/open-watch-marks-viewed

Conversation

@archit-goyal

Copy link
Copy Markdown

Fixes #4218

What changed

  • Route the watch-list external-link button through an internal endpoint that marks the watch as viewed before redirecting to the monitored page.
  • Keep the existing target="_blank" and rel="noopener" behavior, so normal clicks, modifier-clicks, middle-clicks, and keyboard activation retain native browser semantics.
  • Treat the viewed-state update as best effort so a persistence failure does not prevent the monitored page from opening.

Root cause

The button linked directly to the monitored URL, so changedetection.io never received a request and could not update the watch's last_viewed timestamp. The new endpoint reuses the same set_last_viewed path used by existing viewed actions, including the realtime watch update signal.

Testing

  • FLASK_SERVER_NAME=localhost PYTHONPATH=. pytest -q changedetectionio/tests/test_ui.py --tb=short — 6 passed
  • ruff check . --select E9,F63,F7,F82,INT — passed
  • git diff --check — passed

@dgtlmoon dgtlmoon left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice fix for the last_viewed problem — routing through an internal endpoint is the right call. One security note on the redirect, though.

rel="noopener" does nothing for the Referer header. It only severs window.opener (reverse-tabnabbing). The token that strips referer is rel="noreferrer". So the retained noopener doesn't protect anything here.

The redirect makes the watch UUID the referer source. Before this PR the link went straight to watch.link, so the Referer the monitored site saw was the overview page (https://cd-host/) — no UUID. Now the browser first hits /form/watch/<uuid>/open and that URL becomes the referer source for the 302 to the external site. So the per-watch UUID can now leak to the monitored (potentially adversarial) site.

Whether it actually leaks depends on the effective Referrer-Policy:

  • The app only sets Referrer-Policy: same-origin when HIDE_REFERER=true, which is off by default (changedetectionio/__init__.py).
  • Modern browser default (strict-origin-when-cross-origin): only the origin leaks (path/UUID stripped) — same as before.
  • Legacy default (no-referrer-when-downgrade) or unsafe-url (older browsers / some proxies): the full URL incl. the UUID leaks on HTTPS→HTTPS.

Leaking the UUID tells the monitored site it's being watched and hands over the stable internal watch ID (used in edit/diff/preview/API/RSS URLs) — an info-disclosure the pre-PR direct link didn't have.

Suggested fix — set the policy on the redirect response itself, so it's independent of global config and browser defaults (a redirect's Referrer-Policy governs the follow-up request):

        target_url = watch.link
        try:
            datastore.set_last_viewed(uuid, int(time.time()))
        except Exception as e:
            # Opening the monitored page should still work if viewed state cannot be saved.
            logger.error(f"Error marking watch {uuid} as viewed: {e}")

        resp = redirect(target_url)
        resp.headers["Referrer-Policy"] = "no-referrer"
        return resp

Optionally also make the anchor rel="noopener noreferrer" as defense-in-depth, but the server-side header is the authoritative fix and doesn't rely on the template.

@archit-goyal

Copy link
Copy Markdown
Author

Addressed in f1917a8: the redirect now sets Referrer-Policy: no-referrer, with noreferrer on the link as defense-in-depth. I also added regression coverage for both protections.

Sign up for free to 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.

[feature] Opening the page link should mark it as viewed

2 participants