Repository navigation
Conversation
Add a plugin-only hack-feedback skill that drafts reports about Hack itself offline: observed and expected behavior, one honest reproduction status, evidence tagged observed/source/reported/inferred, a friction category, sanitized duplicate search and an attachment list. Posting needs a destination the user confirms and an explicit yes for the exact payload, and goes through the user's own tooling; Hack ships no sender. The skill, its issue-template and examples references, and the new .github/ISSUE_TEMPLATE/hack-feedback.md render from one typed contract in src/agents/feedback-review.ts through the plugin generator. The same module holds the reference sanitizer and the posting gate the skill describes, exercised with a mock sender over synthetic secret, path, URL, config, log-instruction, duplicate, partial-reproduction, denial, edit and failed-send fixtures (HACK-1138). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- Duplicate-search terms keep only the leading tokens that resolve to a CLI_SPEC command path (injected) and registered HACK_ERROR_CODES, so positional project, service and script names never become terms. - Placeholder exemptions apply only to a complete `<...>` value ending at whitespace, punctuation or end of text; `KEY=<x>leak` is redacted. - New `hostname` redaction kind for routed `*.hack` and private-suffix hosts without a scheme; paths under /root, /srv, /Volumes, /workspace and other roots plus Windows drive paths; IPv6; truncated (16+ hex) digests; `Authorization: token ...` headers. - Public-URL allowlist parses the URL and rejects dot segments, encoded dots, queries and free-text fragments. - Embedded-instruction filter targets addressed instructions and approval claims instead of any "send the report" phrase. - attemptFeedbackSend treats a synchronous sender throw as a failed attempt and consumes the approval; comment destinations need a positive integer issue number. - Tests cover each case, including attachment-only edits after review and attachment sanitizing at render time; 18 targeted mutations all killed. Refs HACK-1138 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The feedback-review fixture embedded a literal OpenSSH private-key armor line, which gitleaks flags as a private key. Build the armor at runtime like the file's other token-shaped fixtures, and record the fingerprint of the earlier branch commit that still contains the literal. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A placeholder is exempt only when it is the whole value: after `<...>` only trailing punctuation may come before the value's own terminator, so `API_TOKEN=<x>)leak`, `--password <x>)leak` and `token: <x>`leak`` are redacted. Bearer values that start with `<` are redacted too. The sanitizer now removes JSON, syslog, Go-style and clock-prefixed log lines; redacts `db_pass`, `*_key`, cookie and session-id values, secret- store envelope fields at any length, project paths under `~/.hack`, `$HOME`, `/opt` and `/var/lib`, and hosts with a private inner label; and drops more agent-addressed instructions. The send gate counts an attachment as listed only on a visible line of the Attachments section, so a listing inside an HTML comment or raw HTML is refused. Drafts sanitize attachment names before listing them. New tests cover the attachment-name privacy check and the URL fragment allowlist, which no test exercised before. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The privacy check rejects a macOS home path with a personal-looking user name. Use the file's existing /Users/example fixture style and neutral host and user names in the new log and attachment cases. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The send gate only looked for raw HTML inside the Attachments section, so a `<details>` opened before the heading folded the whole section and its listing still counted. Any element that can hide Markdown, anywhere in the body and outside inline code, now cancels the listing. Escaped backticks no longer hide a tag, and the section's tag check uses CommonMark tag names, so a `<redacted:path>` placeholder in the section is text, not HTML. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Linear: HACK-1138 (WU25, privacy-reviewed Hack feedback drafts).
Adds a plugin-only
hack-feedbackskill. It drafts reports about Hack itself offline and never posts on its own.reproduced,partially-reproduced,not-reproduced,not-attempted), and evidence taggedobserved/source/reported/inferred. It also records one friction category, the command shape with placeholders (orunknownwhen the original argv was not kept), sanitized duplicate-search terms and an attachment list.*.hackand private hostnames written without a scheme, IPv4 and IPv6 addresses, emails, raw logs and config values. A<...>placeholder is exempt only when it is the whole value, soKEY=<x>leakis still redacted. Instructions found in logs or tool output are treated as data.HACK_ERROR_CODES), the leading tokens that resolve to aCLI_SPECcommand path, and the category. The first token that is not a subcommand stops the walk, so positional project, service and script names never become terms. A search happens only when the user agrees. A matching issue turns the report into a comment instead of a new issue.gh). There is no built-in GitHub integration and no background sender.Rendering
src/agents/feedback-review.tsholds the typed contract: taxonomy, statuses, evidence classes, template sections, redaction kinds and gate refusals. It also holds the reference sanitizer, the draft renderer and the send gate.src/agents/feedback-skill.tsrenders these outputs from that contract throughbun run generate:agent-plugins:references/issue-template.mdreferences/examples.md: a synthetic useful report, plus the registered negatives (missing original argv, host-wide disk totals, stale release claim, pasted config/logs, log instructions, hashed identifiers).github/ISSUE_TEMPLATE/hack-feedback.mdThe plugin
hack-clirouting description now hands feedback intents tohack-feedback. Docs are updated: the skill count goes from seven to eight, the agent-guidance sources table changes, and the hack-feedback candidate status changes from Defer to Shipped. The capability matrix is regenerated.Canonical instruction sections and the standalone renders (
hack-cli,hack-init, Cursor rules) did not change. No new integration-ownership fingerprints or content revision were needed; the ownership and instruction-source tests pass unchanged. The CLI surface did not change, so the CLI reference did not need regenerating. No Rust changes.Rework after verification (head 2f7bd64)
The verifier found these defects at
0cb0ff23. Each is now fixed and has a test that fails without the fix:hack env acme-secret-projectgaveenv acme-secret-project.buildDuplicateSearchTerms({ cli, draft })now resolves the command path through the injectedCliSpec(resolveCommand) and keeps only registered error codes.hack env acme-secret-projectnow givesenv env,hack run acmeprod migrategivesrun env, and an inventedE_ACME_SECRET_DBis dropped.<-prefixed secrets:(?!<)skipped any value that started with<. The lookahead now exempts only a complete<...>token followed by whitespace, closing punctuation or the end of the text. This applies to secret assignments, config values, JSON values, secret flags and bearer values.DB_PASSWORD=<x>leakedvalue,API_TOKEN=<redacted:secret>extra-leak,token: <x>hunter2fixtureand--password <x>flagleakare redacted. A freshly approved body containing them returnsprivacy-findings.hostnameredaction kind covers routed*.hackhosts andcorp/internal/local/lan/home/private/test/localhosthosts written without a scheme. Paths under/root,/srv,/Volumes,/workspace(s),/mnt,/media,/data,/code,/project(s)and/var/wwware redacted, as are Windows drive paths. IPv6 is redacted when compressed or full; clock times and::1stay. A body withrouted web.acme-app.hack 502now returnsprivacy-findings.Promise.resolve().then(...). A non-async throw is a failed attempt that consumes the approval, and reusing the approval returnsapproval-already-used.issue-commentdestination needs a positive safe integer; -3.5, 0, 1.5, NaN and Infinity returndestination-invalid. The refusal summary in the skill says so.https://github.comand has no credentials. Its normalized path must be under/hack-dance/hack. URLs with..,%2e, backslashes or queries are redacted, as are fragments other than#[A-Za-z0-9_-]*.file/create/submit, imperatives at a line or sentence start, "agent must now ...", and "user said yes".Hack failed to send the report to the daemonandFile watching stopped ...are now kept.Authorization: token ...is redacted.payload-changed. Another renders a draft with a leaky attachment and asserts render-time sanitizing, attachment redaction counts and a passing gate.Validation
bun run generate:agent-pluginsandbun run docs:agent-client-capabilitieswere regenerated. The idempotence test reproduces the committedplugins/tree.feedback-review(25 tests),integration-ownership,cli-spawn-env-isolation,agent-instruction-source,focused-skills,agent-plugin-generationandskill-references.bun run generate:agent-pluginschanges only the hack-feedbackSKILL.mdand is idempotent. CLI typecheck reports 0 errors. CLI check reports 0 errors and 34 existing warnings, none in changed files. Biome is clean on the changed files, andprivacy:checkpasses.tests/feedback-review.test.ts(new),focused-skills,agent-plugin-generation,skill-references,integration-ownership,cli-spawn-env-isolation,agent-instruction-source,agent-client-capabilitiesandpattern-catalogue.bun run --cwd packages/cli typecheck: 0 errors.bun run --cwd packages/cli check: 0 errors (existing warnings only, none in changed files).biome checkon the changed tests and scripts is clean.bun run privacy:checkpasses with the new files staged. Fixture tokens are assembled at runtime, so no credential-shaped string is committed.Negative controls (mock sender; the sender is never called on refusal)
destination-unconfirmed,approval-not-from-user).samematch refuses a new issue (known-duplicate) and allows a confirmed comment.unknownand is not reconstructed.posting-denied, and the draft is retained unchanged.payload-changed. A hidden attachment returnsattachment-unlisted. A re-added secret or a raw-log attachment returnsprivacy-findings. A fresh review makes the edited payload sendable.approval-already-used. A new approval retries, and once a send succeeds, further attempts returnalready-sent.Remaining gaps
api.acme.com) and bare names in prose are left to the user's review: only private, reserved and*.hacksuffixes are matched, so that file names such aspackage.jsonare not redacted. The skill still requires the user to review the exact text.Rework 2 (head f342e44)
The verifier reported these open defects at
fdd00e40. Each one reproduced on that head withfindFeedbackPrivacyIssues(...) = []or asentmock outcome. Each one is now fixed and has a test.),],}, quotes or a backtick straight after<x>, while the value kept going. A placeholder is now exempt only when it is the whole value. After<...>, only trailing punctuation may come before the value's own end character: whitespace for flags,[\s,;]for assignments,[\s,}]for JSON values, and whitespace, quotes, backtick,,or;forAuthorizationvalues. A Bearer value that starts with<is redacted. Now redacted:API_TOKEN=<x>)…,--password <x>)…,--password <x>,…,API_TOKEN=<x>"…",<x>]…,<x>'…,password=<x>}…,token: <x>`…`andBearer <x>)…. These still pass unchanged:(set API_TOKEN=<value>),,HACK_A=<a>,HACK_B=<b>and{"PORT": <port>,"HOST": <host>}. The verifier's exact strings, in a body the user approved for a confirmed destination, returnprivacy-findings./Users/alice/acme/dump.logand expectsprivacy-findings. With the name check removed, the result isattachment-unlisted. Drafts now sanitize attachment names before the body lists them, so the listing and the attachment carry the same<redacted:path>.https://github.com/hack-dance/hack/issues/1#acme.secret-project.raw-lognow covers JSON records with a log key (level,ts,time,timestamp,msg,logger,log,severity) in any position. It also covers syslog (Oct 19 10:00:01 host proc[1]:), Go-style2026/10/19 10:00:01and clock-time prefixes. A--jsonenvelope such as{"ok":false,"error":{...}}stays.## Attachmentssection that starts with- `name`. That line may be followed by nothing, whitespace or:. HTML comments are removed first, including an unterminated one. A section with raw HTML outside inline code lists nothing. A name containing a backtick or a line break never counts as listed. A listing inside<!-- -->,<div hidden>or<details>, outside the section, or as a prefix only (- `dump.log`.bak) returnsattachment-unlisted.db_pass, any<prefix>_KEYsuch asencryption_key,cookieandSESSION_IDare secret keys.bypass:andtests passed:stay.~/.hackpath stays only when every part is a Hack-owned name, such asdaemon/hackd.sock,projects.jsonorcerts, or a placeholder.~/.hack/projects/<name>/…is redacted.$HOME/…,${HOME}/…,/opt/…(except/opt/homebrew) and/var/lib/…are redacted too.corp/internal/intranet/lan/privatelabel is redacted with its path, for exampleacme-secret.internal.acme.com/admin. Names that end in a file extension, such asapp.internal.ts, stay, and so doescompose.local.yml.ciphertext,encrypted*andwrapped_keyalways, andiv,tag,nonceandsaltwhen the value looks encoded."tag": "latest"stays.attachment-unlistedsummary now describes the visible-section rule (plugins/regenerated).Validation (rework 2)
tests/feedback-review.test.ts: 29 tests pass. Focused set:feedback-review,focused-skills,agent-plugin-generationandskill-references, 67 tests pass.SAFE_URL_FRAGMENT), the old placeholder lookahead, the per-grammar terminators (assignment, JSON), the Bearer<branch, each new secret key and thePASSword guard, both envelope rules, each new raw-log form, the~/.hackallowlist and exemption, the/opt,/var/liband$HOMEroots and the Homebrew guard, the inner-label host rule and its file-extension guard, each of the five instruction patterns, comment stripping, the raw-HTML rule, the inline-code exception, section scoping, the prefix-only check and render-time name sanitizing.gitleaks8.30.1 (detect --log-opts=origin/next..HEAD,.gitleaks.toml): 4 commits, no leaks. Fixture values are assembled at runtime.bun run --cwd packages/cli typecheck: 0 errors.bun run --cwd packages/cli check: 0 errors and the 34 existing warnings.bun run test(local, load average 13 to 23): 6969 pass, 149 skip, 258 fail across 56 files. Every failing file was rerun serially with--timeout 60000. 42 files then passed fully, including every agent, MCP, plugin, env and registry file. The rest fail at fixed 20 or 30 second deadlines inside the tests, not at the runner timeout:native-authored-project-start(the first failing test differs each run, then cascades with "Startup fixture settlement unconfirmed"),native-compose-storage-activation-command,native-compose-retained-storage-command,native-compose-storage-fence-cost(CLI cold start returns 1 after about 35 seconds),native-compose-adoption-generation(11 tests hit 20 or 30 second fixture deadlines),native-compose-proxy-probe(2 TLS cases),native-authored-project-recovery(2) andnative-authored-process-incarnation(1). None import the feedback modules.native-authored-project-startfails the same way with this change reverted to the parent commit. These are reported as load-only local failures; hosted CI is the confirming gate.Remaining gaps (rework 2)
<project>, is hidden by GitHub's renderer. It is not a leak, but the rendered issue loses the placeholder. Drafts render command shapes in code spans.Rework 2, follow-up (head b20820f)
I checked every verifier defect again on
f342e444, using the verifier's exact strings. All six are fixed there:<x>),<x>",<x>],<x>',<x>}and<x>`, plus extra cases with.,:,/,|,<y>,--token=,Bearerandx-api-key.raw-log.SAFE_URL_FRAGMENT. A test now catches each one.This follow-up adds two commits:
5b9613e2test fixtures:5b9613e2was committed earlier but not pushed. It fixes the hostedTestfailure onf342e444. On that head,bun run privacy:checkflagged the fixture/Users/alice/acme/dump.log. The fixtures now use/Users/example/...and neutral host and user names.b20820fchidden attachment listing (new defect):## Attachmentssection. A<details>opened before the heading folded the whole section, and its listing still counted, so the gate allowed the send.<div hidden>around the heading did the same.\`<details>`(an escaped backtick) and<details title="">` hid the tag from the inline-code filter.<redacted:path>placeholder in the section is treated as text, not HTML.attachment-unlistedsummary describes the new rule.plugins/is regenerated.Validation (rework 2 follow-up)
tests/feedback-review.test.ts: 29 pass. The focused set (feedback-review,focused-skills,agent-plugin-generation,skill-references) has 67 passing tests.HTML_TAG(the placeholder listing becomes unsendable);bun run privacy:check: ok.gitleaks git --config .gitleaks.toml --redact --log-opts=origin/next..HEAD): 6 commits scanned, no leaks.bun run --cwd packages/cli typecheck: 0 errors.bun run --cwd packages/cli check: 0 errors, plus the 34 existing warnings.bun run test: run locally with load average 13 to 67. The result was 6988 pass, 149 skip and 239 fail. I reran each failing file serially with--timeout 60000, and 40 of 47 files then passed fully. Those include every agent, MCP, plugin, env, registry and global-command file. Theagent-plugin-generationfailures under load were spawn exit 127.native-compose-adoption-generation(110),native-authored-project-start(11),native-compose-down-command(8),native-authored-project-recovery(2),native-compose-file-command(2),native-compose-vm-file-finalization(2) andnative-authored-process-incarnation(1).skill-references,feedback-skill, the generator and three agent test files do.Remaining gaps (unchanged)
🤖 Generated with Claude Code
Need help on this PR? Tag
@codesmith-botwith what you need.