-
-
Notifications
You must be signed in to change notification settings - Fork 466
fix: synthesize hardware backspace repeat #940
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.
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
Conversation
WalkthroughThis PR modifies SSH server host key verification, adjusts AI prompt construction logic, updates localization strings across multiple languages with new AI-related UI keys while removing server stats entries, refreshes the AI page UI with a disclaimer display, and updates two git-based dependencies (dartssh2 and xterm). Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes The diversity of changes across multiple domains (SSH security, AI logic, extensive localization consistency across 13 locales with varying patterns, UI modification) combined with the security implications of host key verification bypass and the need to verify localization correctness across files elevates review complexity. Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (3 warnings)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
♻️ Duplicate comments (4)
lib/l10n/app_uk.arb (1)
46-47: Inconsistency: AI summary mentions removal but keys are still present.Same as in app_id.arb - the AI summary states these keys were removed, but they remain in the file.
lib/l10n/app_nl.arb (1)
46-47: Inconsistency: AI summary mentions removal but keys are still present.Same as other locale files - the AI summary states these keys were removed, but they remain present.
lib/l10n/app_de.arb (1)
46-47: Inconsistency: AI summary mentions removal but keys are still present.Same as other locale files - the AI summary states these keys were removed, but they remain present.
lib/core/utils/server.dart (1)
113-113: CRITICAL: Unconditional host key acceptance creates a security vulnerability.Same issue as Line 99 - this unconditionally accepts all SSH host keys for key-based authentication, creating the same security vulnerability.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (14)
lib/generated/l10n/l10n.dartis excluded by!**/generated/**lib/generated/l10n/l10n_de.dartis excluded by!**/generated/**lib/generated/l10n/l10n_en.dartis excluded by!**/generated/**lib/generated/l10n/l10n_es.dartis excluded by!**/generated/**lib/generated/l10n/l10n_fr.dartis excluded by!**/generated/**lib/generated/l10n/l10n_id.dartis excluded by!**/generated/**lib/generated/l10n/l10n_ja.dartis excluded by!**/generated/**lib/generated/l10n/l10n_nl.dartis excluded by!**/generated/**lib/generated/l10n/l10n_pt.dartis excluded by!**/generated/**lib/generated/l10n/l10n_ru.dartis excluded by!**/generated/**lib/generated/l10n/l10n_tr.dartis excluded by!**/generated/**lib/generated/l10n/l10n_uk.dartis excluded by!**/generated/**lib/generated/l10n/l10n_zh.dartis excluded by!**/generated/**pubspec.lockis excluded by!**/*.lock
📒 Files selected for processing (17)
lib/core/utils/server.dart(2 hunks)lib/data/provider/ai/ask_ai.dart(1 hunks)lib/l10n/app_de.arb(2 hunks)lib/l10n/app_en.arb(2 hunks)lib/l10n/app_es.arb(2 hunks)lib/l10n/app_fr.arb(2 hunks)lib/l10n/app_id.arb(2 hunks)lib/l10n/app_ja.arb(2 hunks)lib/l10n/app_nl.arb(2 hunks)lib/l10n/app_pt.arb(2 hunks)lib/l10n/app_ru.arb(2 hunks)lib/l10n/app_tr.arb(2 hunks)lib/l10n/app_uk.arb(2 hunks)lib/l10n/app_zh.arb(2 hunks)lib/l10n/app_zh_tw.arb(2 hunks)lib/view/page/ssh/page/ask_ai.dart(1 hunks)pubspec.yaml(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
lib/view/**/*.dart
📄 CodeRabbit inference engine (CLAUDE.md)
lib/view/**/*.dart: Prefer widgets/utilities from fl_lib for common UI (e.g., CustomAppBar, context.showRoundDialog, Input, Btnx.cancelOk)
Prefer using libL10n strings before adding new ones to project l10n
Split UI into build, actions, and utils; use extension on to separate concerns
Files:
lib/view/page/ssh/page/ask_ai.dart
lib/data/provider/**/*.dart
📄 CodeRabbit inference engine (CLAUDE.md)
Organize Riverpod providers by feature under lib/data/provider/
Files:
lib/data/provider/ai/ask_ai.dart
lib/l10n/**/*.arb
📄 CodeRabbit inference engine (CLAUDE.md)
lib/l10n/**/*.arb: After modifying ARB files, run: flutter gen-l10n to regenerate localization
Keep localized strings in ARB files under lib/l10n/ (Flutter i18n)
Files:
lib/l10n/app_pt.arblib/l10n/app_uk.arblib/l10n/app_es.arblib/l10n/app_ru.arblib/l10n/app_ja.arblib/l10n/app_fr.arblib/l10n/app_id.arblib/l10n/app_de.arblib/l10n/app_zh.arblib/l10n/app_zh_tw.arblib/l10n/app_en.arblib/l10n/app_tr.arblib/l10n/app_nl.arb
pubspec.yaml
📄 CodeRabbit inference engine (CLAUDE.md)
Use hive_ce instead of hive for Hive integration
Files:
pubspec.yaml
🔇 Additional comments (8)
lib/l10n/app_id.arb (2)
46-47: Inconsistency: AI summary mentions removal but keys are still present.The AI summary states that
clearServerStatsContentandclearServerStatsTitlewere removed, but these keys are still present in the file at lines 46-47.
289-290: No repositioning concerns; verify flutter gen-l10n was run.The verification confirms
writeScriptTipis properly referenced in the codebase (lib/view/page/server/edit/widget.dartasl10n.writeScriptTip). Repositioning a key in an ARB file does not break references since keys are accessed by name through generated l10n properties, not by file position. Per the coding guidelines, ensureflutter gen-l10nwas executed after modifying the ARB file to regenerate localization.lib/view/page/ssh/page/ask_ai.dart (1)
423-455: LGTM! Improved disclaimer visibility.The UI restructuring places the AI disclaimer prominently above the input area with appropriate error color and bold styling, making users aware of AI limitations before interacting. The input functionality remains unchanged.
pubspec.yaml (1)
44-44: Dependency versions verified; ensure SSH auth changes in dartssh2 v1.0.293 are compatible.Both version tags confirmed:
- dartssh2 v1.0.293: Tag exists ✓
- xterm v4.0.12: Tag exists ✓
Changelog review shows dartssh2 includes notable changes: SSH auth fix with hostkey verify tightening and cipher defaults, plus RFC compliance updates. The xterm update is a minor backspace repeat fix.
Verify that the SSH auth changes in dartssh2 v1.0.293 don't break your SSH connection flows.
lib/data/provider/ai/ask_ai.dart (1)
215-218: Verify localeHint fallback behavior change.The prompt previously always included a Chinese fallback instruction ("如果无法判断语言,请使用简体中文。"), which is now removed when
localeHintis null or empty. This means the AI receives no language guidance in these cases, changing its response behavior significantly.Confirm that omitting the fallback instruction aligns with the international app direction and produces acceptable AI responses when locale detection fails.
lib/l10n/app_tr.arb (1)
9-24: Correct locale count and confirm all changes are valid.The verification confirms 16 AI-related keys are correctly added and consistently present across all 13 locale files (not 8 as mentioned), including all locales: de, en, es, fr, id, ja, nl, pt, ru, tr, uk, zh, and zh_tw. All ARB files have valid JSON structure.
Important: Run localization regeneration command in your development environment.
As per coding guidelines, after modifying ARB files, you must run:
flutter gen-l10nThis regenerates the localization classes based on the updated ARB files.
lib/l10n/app_zh_tw.arb (2)
289-290: LGTM!The
writeScriptTipmodification properly formats the multi-line string with correct JSON escaping and preserves the technical path information.
9-24: Ensure localization files are regenerated after this change.The new AI assistant localization keys follow consistent naming conventions with appropriate Traditional Chinese translations and properly preserved placeholders. However, per the coding guidelines, you must manually run the following command to regenerate the localization files:
flutter gen-l10nThis is a mandatory step whenever ARB files are modified to ensure all localization strings are properly compiled.
| username: alterUser ?? spi.user, | ||
| onPasswordRequest: () => spi.pwd, | ||
| onUserInfoRequest: onKeyboardInteractive, | ||
| onVerifyHostKey: (type, fingerprint) => true, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CRITICAL: Unconditional host key acceptance creates a security vulnerability.
This callback unconditionally accepts all SSH host keys without verification, completely bypassing a fundamental security mechanism. This exposes users to man-in-the-middle attacks, DNS spoofing, and server impersonation.
Security implications:
- Attackers can intercept SSH connections without detection
- Changed host keys (potential compromise indicator) are silently accepted
- No user warning or consent for this security downgrade
Recommended approach:
- Store and verify host keys properly (persist fingerprints in secure storage)
- Prompt users to verify fingerprints on first connection
- Warn users when host keys change
- If bypassing is needed for testing, gate it behind a developer setting with clear warnings
- onVerifyHostKey: (type, fingerprint) => true,
+ onVerifyHostKey: (type, fingerprint) {
+ // TODO: Implement proper host key verification
+ // 1. Check if fingerprint is stored for this host
+ // 2. If first connection, prompt user to verify fingerprint
+ // 3. If fingerprint changed, warn user of potential MITM attack
+ // 4. Store accepted fingerprints securely
+ return _verifyHostKeyWithUserConsent(spi.id, type, fingerprint);
+ },Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In lib/core/utils/server.dart around line 99 the onVerifyHostKey callback
currently returns true unconditionally, which accepts any SSH host key and
creates a critical MITM risk; replace this with a proper verification flow: load
stored host key fingerprints from secure persistent storage, if none exist
prompt or surface the fingerprint for first-time user confirmation and then
persist it; on subsequent connections compare the presented fingerprint to the
stored one and reject the connection (or surface a clear warning and require
explicit user confirmation) if it differs; provide a developer-only bypass flag
(explicitly gated and logged) for testing only.
| "askAi": "Tanya AI", | ||
| "askAiApiKey": "Kunci API", | ||
| "askAiAwaitingResponse": "Menunggu respons AI...", | ||
| "askAiBaseUrl": "URL dasar", | ||
| "askAiCommandInserted": "Perintah dimasukkan ke terminal", | ||
| "askAiConfigMissing": "Harap konfigurasikan {fields} di Pengaturan.", | ||
| "askAiConfirmExecute": "Konfirmasi sebelum menjalankan", | ||
| "askAiConversation": "Percakapan AI", | ||
| "askAiDisclaimer": "AI bisa saja salah. Gunakan dengan hati-hati.", | ||
| "askAiFollowUpHint": "Ajukan pertanyaan lanjutan...", | ||
| "askAiInsertTerminal": "Masukkan ke terminal", | ||
| "askAiModel": "Model", | ||
| "askAiNoResponse": "Tidak ada respons", | ||
| "askAiRecommendedCommand": "Perintah yang disarankan AI", | ||
| "askAiSelectedContent": "Konten yang dipilih", | ||
| "askAiUsageHint": "Digunakan di Terminal SSH", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
Verify localization keys are used in the codebase.
New AI-related localization keys have been added. Please ensure these keys are actually used in the UI code and that the flutter gen-l10n command is run to regenerate localization files.
As per coding guidelines
🏁 Script executed:
#!/bin/bash
# Verify AI localization keys are referenced in the codebase
echo "=== Checking usage of new AI localization keys ==="
for key in askAi askAiApiKey askAiAwaitingResponse askAiBaseUrl askAiCommandInserted askAiConfigMissing askAiConfirmExecute askAiConversation askAiDisclaimer askAiFollowUpHint askAiInsertTerminal askAiModel askAiNoResponse askAiRecommendedCommand askAiSelectedContent askAiUsageHint; do
echo -n "$key: "
rg -l "l10n\.$key\b" --type=dart | wc -l | xargs echo "used in files"
done
echo -e "\n=== Checking if l10n was regenerated ==="
if [ -f "lib/generated/l10n.dart" ] || [ -f ".dart_tool/flutter_gen/gen_l10n/app_localizations.dart" ]; then
echo "✓ Generated l10n files exist"
else
echo "⚠ Generated l10n files not found - run 'flutter gen-l10n'"
fiLength of output: 694
Run flutter gen-l10n before merging.
All 16 new AI-related localization keys are properly used in the codebase. However, the generated l10n files are missing and have not been regenerated. Per coding guidelines, you must run flutter gen-l10n to regenerate localization files after modifying ARB files.
🤖 Prompt for AI Agents
In lib/l10n/app_id.arb lines 9-24 the ARB has added 16 AI-related keys but the
generated localization files were not updated; run `flutter gen-l10n` from the
project root to regenerate the Dart localization artifacts, verify the new keys
appear in the generated localization classes, add and commit the updated files
(and any changed .dart files under lib/l10n or generated output) so the build
includes the new translations.
A ja bi ka ovo trebala izvrsavati jel 🤣😂🤣😂🤣 .Stojite u mom uredaju bez ikakve ovlasti i jos unutra drkate svoje stvari koje bi ja morala ucinit da zadovoljim vase komentare ??? Daj te i meni toga na cemu ste molim vas sve ulazem samo dajte da me tako puca . |
And I'm supposed to be doing this right 🤣😂🤣😂🤣 . You are standing in my device without any authority and you are still inside jerking off your things that I would have to do to satisfy your comments ??? Give me what you're up to, please, I'm going in, just let me shoot like that. |
Fixes #939
Summary by CodeRabbit
New Features
Bug Fixes
Localization
Chores