Repository navigation
fix(server): report a secret on a mirrored pane as entered only after herdr took it, and press Enter as a key - #263
Conversation
… herdr took it, and press Enter as a key
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. 🧰 Additional context used📚 Code guidelines (1)No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: devswha/herdr-web-ui/.coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (3)
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 8 remain after this review. 📝 SummarySummary by CodeRabbit
WalkthroughMirrored-pane secret entry now sends the secret text and Enter through separate herdr requests. The server reports success after both requests complete and reports failure if the text send is refused. Direct-attach panes continue to write the secret and carriage return to the PTY. ChangesMirrored secret delivery
Priority: ➖ Normal Estimated code review effort: 3 (Moderate) | ~20 minutes Change: Bug fix · Severity of issue fixed: Medium Suggested reviewers: Merge Risk: ⚪ Minimal · up to Mirrored secret submission now waits for text and Enter delivery before reporting success and safely reports failures. Direct-attached panes retain their existing behavior. The change is mergeable with normal checks; real-Windows validation remains unreported. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to The change improves success reporting and preserves existing access checks. No introduced secret disclosure was established. Recovery remains uncertain if the password reaches the terminal but Enter fails or its completion is unknown. Retained concerns Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
Resilience and Maintainability Implications
Hardening Proposals
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 75.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 2 files. (1 skipped: 1 unsupported.)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
Release v0.3.40: moves the Unreleased notes under 0.3.40 and bumps `package.json` and `herdr-plugin.toml`. What's in it: - Fixes from #249, #251, #252, #263, #264, #265, #267. - The iPhone CJK and text-symbol fix (#272). - Emoji cell widths matching herdr (#273). Every changelog entry already ends with its PR link. All these PRs are by @devswha, so no credits to add. No remote bundle change: `REMOTE_BUNDLE_VERSION` stays 9. The Windows PC part of #264 and #265 reaches a PC with the next remote bundle, as its changelog entry says.
What was wrong
On a terminal that is mirrored (a PC whose herdr cannot
terminal attach, as on Windows), entering a password from the secret prompt was reported as done the moment the server handed it over, before herdr had taken it. If herdr refused the send, the browser still showed success and cleared the field. Enter was also sent as a carriage return inside the text, not as a key.What changed
server/index.ts, thesecretcase: on a mirrored pane the secret goes out aspane.send_text(the secret only), thenpane.send_keys ["Enter"], both awaited inside the pane's existing serialized turn.secret-result ok:trueis sent only after both; a failure answersok:falsewithsecret_failed, and no herdr message is forwarded, so the secret cannot leak through an error text.pty.writeand no RPC payload.Tests and QA
server/secret.contract.test.tsgains two cases that run the real server withterminalAttach: falsebehind a pass-through proxy socket in front of the test herdr, which records every request and can refusepane.send_text:pane.send_text "fixture-value"thenpane.send_keys ["Enter"];secret-result ok:truearrives after herdr answered both; the no-echo stand-in records the line.secret-resultisok:false, code: secret_failed, no Enter is pressed, nothing reaches the stand-in.Before the fix both failed (one
send_textcarryingfixture-value\r, nosend_keys,ok:truebefore any answer, andok:trueon a refused send). After it: 3 pass, 0 fail, the existing direct-attach case included.bun run typecheck,bun run test:unit(681 pass) andbun run buildare green.Limits
createServer({ terminalAttach: false }), not on a real Windows PC.ok:falseand the typed text stays at the prompt.Closes #259