Skip to content

add honeypot to snap reports #5589

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
M7mdisk merged 6 commits into from
Feb 17, 2026
Merged

add honeypot to snap reports #5589

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
6103e87
add honeypot to snap reports
M7mdisk Feb 13, 2026
6ba1819
Add aria-hidden attribute to honeypot field for screen reader accessi…
Copilot Feb 13, 2026
4310bd3
Apply suggestion from @Copilot
M7mdisk Feb 13, 2026
b869b51
Merge branch 'main' into honey-pot
M7mdisk Feb 17, 2026
770342b
allow urls in comments
M7mdisk Feb 17, 2026
3b5c4f2
fix: remove inline styling
M7mdisk Feb 17, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions templates/store/snap-details/_report_snap_modal.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,8 @@ <h5 class="p-notification__title">

<label for="report-snap-email">Your email (optional)</label>
<input id="report-snap-email" type="email" name="email" placeholder="email@example.com" />
<label for="report-snap-confirm" style="position: absolute; top: -9999999px;">I agree</label>
<input id="report-snap-confirm" type="checkbox" name="confirm" style="position: absolute; top: -9999999px;" />
Copy link

Copilot AI Feb 13, 2026

Choose a reason for hiding this comment

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

The honeypot field lacks accessibility attributes that are important for screen reader users. While the field is intentionally hidden from visual users, it should include an aria-hidden="true" attribute to explicitly signal to assistive technologies that this field should be ignored. This ensures that screen reader users don't encounter confusion from a hidden "I agree" checkbox that appears unrelated to the form's stated purpose.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor Author

@M7mdisk M7mdisk Feb 13, 2026

Choose a reason for hiding this comment

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

@copilot open a new pull request to apply changes based on this feedback

Comment thread
M7mdisk marked this conversation as resolved.
Outdated
<p>In submitting this form, I confirm that I have read and agree to <a href="https://ubuntu.com/legal/data-privacy/contact">Canonical’s Privacy Notice</a> and <a href="https://ubuntu.com/legal/data-privacy">Privacy Policy</a>.</p>
<div class="u-align--right">
<button type="button" class="js-modal-close u-no-margin--bottom">Cancel</button>
Expand Down
7 changes: 7 additions & 0 deletions webapp/store/snap_details_views.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -542,6 +542,13 @@ def report_snap():

fields = flask.request.form

# If the honeypot is activated or a URL is included in the comment,
Copy link

Copilot AI Feb 13, 2026

Choose a reason for hiding this comment

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

The PR description is incomplete and does not meet the repository's required standards. According to the custom coding guidelines, every PR must include:

  1. Clear "How to QA" instructions with specific, actionable steps
  2. Link to the issue or card being addressed (currently shows "Fixes #" with no issue number)
  3. Completion of security considerations checklist (currently all unchecked)
  4. Brief explanation of what changed and why

These sections are currently empty or incomplete, making it difficult for reviewers to understand the context and properly test the changes.

Copilot generated this review using guidance from repository custom instructions.
# silently reject to avoid spam
if (
"confirm" in fields and fields["confirm"] == "on"
) or "http" in fields.get("comment", ""):
return flask.jsonify({"ok": True}), 200
Copy link

Copilot AI Feb 13, 2026

Choose a reason for hiding this comment

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

The spam detection implementation lacks test coverage. The repository has comprehensive Python test coverage for store endpoints (see tests/store/tests_details.py with 675 lines of tests), but this new security-critical functionality has no tests. Tests should verify:

  1. Legitimate reports without URLs are accepted
  2. Reports with the honeypot field checked are silently rejected
  3. Reports containing "http" in comments are rejected (though this behavior should be reconsidered as noted in another comment)
  4. The endpoint returns the correct response codes and JSON payloads in each scenario

Testing is particularly important for spam prevention logic to ensure it works as intended and doesn't create false positives.

Copilot uses AI. Check for mistakes.
Copy link

Copilot AI Feb 13, 2026

Choose a reason for hiding this comment

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

The URL detection pattern is overly restrictive and will block legitimate reports. The check for "http" in the comment field will reject valid reports that include legitimate URLs (e.g., "This snap violates copyright of https://example.com"). Consider:

  1. Only blocking if the comment contains multiple URLs (spam pattern)
  2. Using a more sophisticated pattern to detect spam URLs vs. legitimate references
  3. Checking for "http://" or "https://" at word boundaries rather than just "http" as a substring

Additionally, "https" URLs will also be blocked since they contain "http" as a substring, making this check even more restrictive than likely intended.

Copilot uses AI. Check for mistakes.

Comment thread
M7mdisk marked this conversation as resolved.
Outdated
payload = {
"snap_name": fields.get("snap_name", ""),
"reason": fields.get("reason", ""),
Expand Down
Loading
⚡