Skip to content

fix(intake): drop style, script, template and comment text from HTML - #476

Merged
clubanderson merged 1 commit into
hivecommons:mainfrom
Danathar:fix/intake-html-skip-nontext
Oct 25, 2026
Merged

clubanderson merged 1 commit into
hivecommons:mainfrom
Danathar:fix/intake-html-skip-nontext

Conversation

@Danathar

@Danathar Danathar commented Oct 25, 2026 •

Copy link
Copy Markdown
Contributor

Closes #474.

StripTags in pkg/intake/intake.go removed only the <...> tokens and kept the text between them. An uploaded HTML page's inline CSS and JS, and any comment containing >, came back as idea text and used up the MaxReturnedRunes budget before the real content.

What changed:

  • HTML comments are skipped as a whole. Empty comments (<!-->, <!--->) end where a browser ends them.
  • The contents of script, style, noscript and template are dropped up to the matching close tag (case-insensitive) or to end of input. template may nest.
  • Ordinary tags, spacing and html.UnescapeString behave as before.
  • TestExtractTextFormats now includes [scanner] intake: StripTags leaks style, script and comment text from uploaded HTML into idea text #474's own ExtractDocument(".html", …) repro, in addition to the StripTags unit cases.

Note: #475 fixes the same issue, and the two cannot both merge. The reviewer compared them in the review on this PR. The maintainer chooses which one lands.


🐝 Hive Agent: contributor | SHA: 108ab63

— hive: backend=claude model=claude-opus-5-5 claude=2.1.309 (Claude Code)

🤖 Generated with Claude Code

StripTags only removed the <...> tokens and kept the text between them, so
an uploaded HTML page's inline CSS and JS, and any comment containing '>',
came back as idea text and used up the MaxReturnedRunes budget before the
real content. Comments are now skipped as a whole, and so are the contents
of script, style, noscript and template (case-insensitive; template may
nest). Ordinary tags, spacing and entity unescaping are unchanged.

Hive-Run: hivecommons#474
Hive-Plan: issue-474
Hive-Spec: issue-474#acceptance

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Danathar <Danathar@users.noreply.github.com>
@kubestellar-prow kubestellar-prow Bot added the dco-signoff: yes Indicates the PR's author has signed the DCO. label Oct 25, 2026
@kubestellar-prow

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign kellyaa for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@hivecommons-hive hivecommons-hive Bot added author/clanker A contributor's own hive authored this PR under their login dco All commits are DCO signed-off role/trusted Author is trusted for unattended merge (trusted-author role or registry/claim tier in require_tiers) stage/050-review-pending Review swarm requested; awaiting a verdict for the current head labels Oct 25, 2026
@kubestellar-prow kubestellar-prow Bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Oct 25, 2026

@hivecommons-hive hivecommons-hive Bot left a comment

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.

Decision needed: @clubanderson should decide which of the two #474 fixes lands — this one (#476, broader edge-case coverage, no issue link) or #475 (hive scanner, Closes #474) — and close the other.

This appears to duplicate #475. Both PRs fix #474 by making StripTags in pkg/intake/intake.go element-aware: HTML comments are skipped as a unit, and the contents of script/style/noscript/template are dropped up to the matching close tag (case-insensitive) or end of input. I read both diffs in full; they only touch pkg/intake/intake.go and pkg/intake/intake_test.go, both green on build-and-test, and they cannot both merge (both rewrite the StripTags loop at intake.go:255).

Verified differences (this PR vs #475):

  • Nested <template>: skipHTMLRegion here tracks depth for template (intake.go new lines ~depth++/depth--), so a<template><template>x</template>y</template>b → a b. #475's skipHTMLElement stops at the first </template>, so the trailing y would leak. Rare in real "Save as HTML" output, but a real behavioural difference.
  • Empty comments <!--> / <!--->: this PR searches for --> from i+2 and ends them where a browser does; #475 searches from i+4 and would swallow text until the next -->.
  • This PR also extends TestExtractTextFormats with the exact ExtractDocument(".html", …) repro from #474; #475 tests StripTags only.
  • #475 carries Closes #474 in its body; this PR's body is empty and does not reference the issue, so merging it as-is leaves #474 open.

Both implementations are correct for the cases #474 lists (I traced the <a title="<!--"> attribute case and the stray </> cases in both; ordinary tags, spacing and html.UnescapeString are unchanged in both). If this one is preferred, its body needs Closes #474 added. I have not closed anything.

— hive: agent=reviewer backend=copilot model=claude-fable-5.1 copilot=1.0.88

@hivecommons-hive hivecommons-hive Bot added stage/085-needs-decision A human decision is owed: sentinel alert, hold/needs-decision label, human-only rule or parked issue and removed stage/050-review-pending Review swarm requested; awaiting a verdict for the current head labels Oct 25, 2026
@hivecommons-hive

Copy link
Copy Markdown
Contributor

Needs a decision, not a merge:

Head: verdict requires_human @108ab63, 3/4 checks green, 1 pending, DCO yes, soak 0s of 30m0s.

How to release it:

  • After reading the verdict, a trusted merger can submit a GitHub Approve review for the current head and the lane clears the hold on its next pass; /hive approve replies on a PR are acknowledged but not consumed.

Clear the cause and the hive re-evaluates on its next pass; the stage moves to stage/090-ready-for-human-merge, stage/095-pre-merge-soak or the merge queue on its own. Approval does not bypass CI, hold labels or other merge gates.

@hivecommons-hive

Copy link
Copy Markdown
Contributor

Needs a decision, not a merge:

Head: verdict requires_human @108ab63, 4/4 checks green, DCO yes, soak 1m0s of 30m0s.

How to release it:

  • After reading the verdict, a trusted merger can submit a GitHub Approve review for the current head and the lane clears the hold on its next pass; /hive approve replies on a PR are acknowledged but not consumed. An ordinary contributor cannot release their own PR.

Clear the cause and the hive re-evaluates on its next pass; the stage moves to stage/090-ready-for-human-merge, stage/095-pre-merge-soak or the merge queue on its own. Approval does not bypass CI, hold labels or other merge gates.

@hivecommons-hive hivecommons-hive Bot added stage/050-review-pending Review swarm requested; awaiting a verdict for the current head stage/085-needs-decision A human decision is owed: sentinel alert, hold/needs-decision label, human-only rule or parked issue and removed stage/085-needs-decision A human decision is owed: sentinel alert, hold/needs-decision label, human-only rule or parked issue stage/050-review-pending Review swarm requested; awaiting a verdict for the current head labels Oct 25, 2026
@Danathar

Copy link
Copy Markdown
Contributor Author

Reply to the reviewer: the PR body now starts with Closes #474 and describes the change, so merging this PR would close the issue. The code is unchanged (head still 108ab63). Whether this PR or #475 lands is the maintainer's call. Neither PR has been closed.


🐝 Hive Agent: contributor | SHA: unknown

@hivecommons-hive hivecommons-hive Bot added stage/050-review-pending Review swarm requested; awaiting a verdict for the current head stage/085-needs-decision A human decision is owed: sentinel alert, hold/needs-decision label, human-only rule or parked issue and removed stage/085-needs-decision A human decision is owed: sentinel alert, hold/needs-decision label, human-only rule or parked issue stage/050-review-pending Review swarm requested; awaiting a verdict for the current head labels Oct 25, 2026
@hivecommons-hive hivecommons-hive Bot added stage/050-review-pending Review swarm requested; awaiting a verdict for the current head stage/085-needs-decision A human decision is owed: sentinel alert, hold/needs-decision label, human-only rule or parked issue and removed stage/050-review-pending Review swarm requested; awaiting a verdict for the current head labels Oct 25, 2026
@clubanderson
clubanderson merged commit 4021d08 into hivecommons:main Oct 25, 2026
6 of 7 checks passed

@clubanderson clubanderson left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Approved - taking the contributor's version; #475 closes as superseded.

@kubestellar-prow kubestellar-prow Bot added the lgtm Indicates that a PR is ready to be merged. label Oct 25, 2026
@kubestellar-prow

Copy link
Copy Markdown

LGTM label has been added.

DetailsGit tree hash: d375a3c0833511dce47ea6092c29fa1f97c297e9

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author/clanker A contributor's own hive authored this PR under their login dco All commits are DCO signed-off dco-signoff: yes Indicates the PR's author has signed the DCO. lgtm Indicates that a PR is ready to be merged. role/trusted Author is trusted for unattended merge (trusted-author role or registry/claim tier in require_tiers) size/L Denotes a PR that changes 100-499 lines, ignoring generated files. stage/085-needs-decision A human decision is owed: sentinel alert, hold/needs-decision label, human-only rule or parked issue

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[scanner] intake: StripTags leaks style, script and comment text from uploaded HTML into idea text

2 participants