Skip to content

Improve WebUI: add heading for scan results section#1484

Open
Vinubr wants to merge 3 commits intoOWASP:masterfrom
Vinubr:improve-webui-results-heading
Open

Improve WebUI: add heading for scan results section#1484
Vinubr wants to merge 3 commits intoOWASP:masterfrom
Vinubr:improve-webui-results-heading

Conversation

@Vinubr
Copy link
Copy Markdown

@Vinubr Vinubr commented Mar 31, 2026

This PR improves the WebUI by adding a clear "Scan Results" heading to the results section.

Currently, the results section lacks a visible title, which can make it less intuitive for users. This change enhances readability and improves user experience without affecting existing functionality.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is not necessary for this PR. The usage document explains quick start nicely

@securestep9
Copy link
Copy Markdown
Collaborator

@coderabbitai remember to check PRs for following the PR template checks and flag low-value contribution which appear to be "contribution farming"

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Apr 3, 2026

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check commit signatures and author info
gh api repos/OWASP/Nettacker/pulls/1484/commits --jq '.[] | {sha: .sha, author: .commit.author.name, email: .commit.author.email, verified: .commit.verification.verified, reason: .commit.verification.reason}'

echo "---PR DETAILS---"
gh pr view 1484 --repo OWASP/Nettacker --json title,body,author,labels,linkedIssues,changedFiles,additions,deletions

echo "---CHANGED FILES---"
gh api repos/OWASP/Nettacker/pulls/1484/files --jq '.[] | {filename: .filename, additions: .additions, deletions: .deletions, patch: .patch}'

echo "---PR TEMPLATE---"
gh api repos/OWASP/Nettacker/contents/.github/PULL_REQUEST_TEMPLATE.md --jq '.content' | base64 -d

Length of output: 4253


@securestep9: I've reviewed PR #1484 in full. Here is a comprehensive assessment:


🚨 Pre-Merge Check Failures

1. Unsigned Commits & Unknown Git Identity

All 3 commits fail pre-merge checks:

SHA Author Verified Reason
7158bbcd unknown unsigned
c64b9a10 unknown unsigned
b76ad7c8 unknown unsigned

Two separate issues here:

  • Commits are not GPG/SSH signed — violates the mandatory checklist item "I have digitally signed all my commits in this PR."
  • Author name is "unknown" — indicates a misconfigured Git environment (git config user.name has not been set).

2. PR Template Not Filled Out

The PR body contains the raw template placeholder "Your PR description goes here." — the author did not fill in any section of the PR template:

  • ❌ No description of changes in "Proposed change"
  • ❌ No "Type of change" checkbox selected
  • ❌ Zero checklist items checked (all 11 unchecked)
  • ❌ No linked issue

⚠️ Low-Value / Contribution Farming Indicators

The PR bundles two unrelated changes without disclosure:

  1. nettacker/web/static/index.html (+1 line): Adds a single <h2>Scan Results</h2> heading — a purely cosmetic, trivial change with no functional impact and no linked issue justifying it.

  2. README.md (+17 lines): Silently adds a "Quick Start Example" section with basic CLI examples (-i example.com, -m port_scan, -v). This content almost certainly duplicates existing documentation and was not mentioned in the PR title or description at all.

Combined signals:

  • ✅ Trivial/cosmetic changes
  • ✅ No linked issue
  • ✅ No functional impact
  • ✅ Template entirely unfilled
  • ✅ Unsigned commits with unknown Git identity
  • ✅ README content likely duplicating existing docs
  • ✅ Unrelated changes bundled together without disclosure

Recommendation

This PR should not be merged in its current state. The author (@vinuthabr2020, based on the commit email) should be asked to:

  1. Close this PR and reopen after fixing Git identity (git config user.name)
  2. Sign all commits with GPG/SSH
  3. Fill out the complete PR template, including linking to a relevant open issue
  4. Separate the README change from the HTML change (if both are even warranted)
  5. Justify why both changes are needed with links to open issues

✏️ Learnings added
Learnt from: securestep9
URL: https://github.com/OWASP/Nettacker/pull/1484

Timestamp: 2026-04-03T12:38:44.049Z
Learning: In OWASP/Nettacker, when a PR body contains the raw template placeholder text (e.g., "Your PR description goes here."), treat the PR template as entirely unfilled and flag it as non-compliant — all 3 sections (Proposed change, Type of change, Checklist) must be properly completed.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.


🧠 Learnings used
Learnt from: securestep9
Repo: OWASP/Nettacker PR: 0
File: :0-0
Timestamp: 2026-04-01T22:22:07.355Z
Learning: Flag low-value or "contribution farming" PRs in OWASP/Nettacker. Indicators include: trivial/cosmetic changes (e.g., minor README edits), no linked issue, no functional impact, no tests, content that duplicates existing documentation, and first-time contributors making only superficial changes. Such PRs should be flagged to maintainers and the author asked to provide a linked issue and full PR template compliance before the PR is considered for merge.

Learnt from: securestep9
Repo: OWASP/Nettacker PR: 0
File: :0-0
Timestamp: 2026-04-01T22:22:07.355Z
Learning: Always check that PR submissions in OWASP/Nettacker follow the PR template at .github/PULL_REQUEST_TEMPLATE.md. The template requires three sections: "Proposed change" (with description and linked issue), "Type of change" (exactly one checkbox), and "Checklist" (11 items including: following contributing guidelines, digitally signing commits, running `make pre-commit` and `make test`, updating docs, linking an open issue, attaching screenshots, confirming no unreviewed AI output, and confirming sole authorship). Flag PRs that omit the template as non-compliant.

Learnt from: securestep9
Repo: OWASP/Nettacker PR: 0
File: :0-0
Timestamp: 2026-01-13T22:48:26.796Z
Learning: When reviewing any pull request in the OWASP/Nettacker repository, always remind the PR author to use signed commits and explicitly state that the PR fails pre-merge checks if any commits are unsigned. Tag the author by username and provide instructions on how to sign commits.

Learnt from: securestep9
Repo: OWASP/Nettacker PR: 0
File: :0-0
Timestamp: 2026-04-03T12:22:41.028Z
Learning: In OWASP/Nettacker, always verify that PR authors have a properly configured Git identity (author name should not be "unknown" in commit metadata). Flag PRs where commit author name is "unknown" as this indicates a misconfigured Git environment and should be flagged alongside unsigned commits as a pre-merge check failure.

Learnt from: securestep9
Repo: OWASP/Nettacker PR: 0
File: :0-0
Timestamp: 2026-01-10T15:29:31.759Z
Learning: When reviewing any pull request, always check that all commits are signed (GPG/SSH signatures verified). Use GitHub CLI to verify commit signatures with: `gh api repos/OWNER/REPO/pulls/PR_NUMBER/commits --jq '.[] | {sha: .sha, author: .commit.author.name, verified: .commit.verification.verified, reason: .commit.verification.reason}'`. Report any unsigned commits to maintainers.

Learnt from: securestep9
Repo: OWASP/Nettacker PR: 0
File: :0-0
Timestamp: 2026-01-10T14:58:54.638Z
Learning: When reviewing a pull request, always analyze other open PRs in the repository to identify potential conflicts, such as duplicate changes to the same files, overlapping features, or merge conflicts. Use GitHub CLI to search for related PRs by file changes, keywords, or linked issues.

Learnt from: securestep9
Repo: OWASP/Nettacker PR: 0
File: :0-0
Timestamp: 2026-01-15T17:07:23.769Z
Learning: When securestep9 requests to check commit signatures in open PRs, always verify all commits are signed using GitHub CLI and report unsigned commits immediately with explicit notification that the PR fails pre-merge checks.

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.

3 participants