feat: full copy from apollo_nxt-trcaa with complete sanitization #69
No reviewers
Labels
No Label
Compat/Breaking
Kind/Bug
Kind/Documentation
Kind/Enhancement
Kind/Feature
Kind/Security
Kind/Testing
Priority
Critical
Priority
High
Priority
Low
Priority
Medium
Reviewed
Confirmed
Reviewed
Duplicate
Reviewed
Invalid
Reviewed
Won't Fix
Status
Abandoned
Status
Blocked
Status
Need More Info
No Milestone
No project
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: sarman/tftsr-devops_investigation#69
Loading…
Reference in New Issue
Block a user
No description provided.
Delete Branch "feature/full-copy-from-trcaa"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Complete backport of all features from apollo_nxt-trcaa repository:
Sanitization applied to all files:
Technical changes:
Version: 1.0.8
Co-Authored-By: Claude Sonnet 4.5 noreply@anthropic.com
Automated PR Review could not be completed - LLM analysis failed or produced no output.
Automated PR Review could not be completed - LLM analysis failed or produced no output.
Automated PR Review could not be completed - LLM analysis failed or produced no output.
Automated PR Review (qwen3-coder-next via liteLLM):\n\nSummary
This PR claims to perform a "full copy from apollo_nxt-trcaa with complete sanitization", but introduces critical security and correctness regressions: hardcoded corporate-internal URLs and headers remain in the CSP and code, and a 32 KB input cap for PII scanning is missing in one path.
Findings
[BLOCKER]
src-tauri/tauri.conf.json- Hardcoded corporate-internal CSPconnect-srcURLs remain unredactedEvidence:
"connect-src": "... https://genai-service.stage.commandcentral.com https://genai-service.commandcentral.com ..."Fix: Remove both
commandcentral.comentries from CSPconnect-src.[BLOCKER]
src-tauri/src/ai/openai.rsline ~219 - HardcodedX-msi-genai-clientheader references internal TFTSR serviceEvidence:
headers.insert("X-msi-genai-client", "troubleshooting-rca-assistant".parse()?)?;Fix: Remove or make header configurable; do not hardcode internal service references.
[WARNING]
src-tauri/src/commands/analysis.rsline ~185 -scan_text_for_piiacceptsStringinput without enforcing the documented 32 KB cap before scanningEvidence:
pub async fn scan_text_for_pii(text: String) -> Result<PiiDetectionResult, String> { ... let detector = PiiDetector::new(); let spans = detector.detect(&text); ... }— no explicit length check beforedetect()Fix: Add
if text.len() > 32 * 1024 { return Err("Input exceeds 32 KB limit".into()); }at function start.Verdict: REQUEST CHANGES
Automated PR Review (qwen3-coder-next via liteLLM):\n\nSummary
This PR implements comprehensive PII detection and sanitization across the entire application, specifically targeting two critical bypasses: (1) file attachments embedded directly in chat messages without redaction, and (2) typed chat messages sent to AI providers without any scanning. It also fixes a type mismatch in
detect_piithat broke the frontend PII review workflow. All PII is auto-redacted in-place before sending to AI, with full audit logging and redacted versions displayed to the user.Findings
[BLOCKER] src-tauri/src/ai/anthropic.rs:22, src-tauri/src/ai/gemini.rs:21, src-tauri/src/ai/mistral.rs:21, src-tauri/src/ai/ollama.rs:34, src-tauri/src/ai/openai.rs:25 -
ChatResponseconstructors are missing theuser_messagefield required by the updated command handler and TypeScript interface.Evidence (from
ai/openai.rs):pub struct ChatResponse { pub content: String, pub tool_calls: Option<Vec<ToolCall>>, }Fix: Add
user_message: Option<String>to allChatResponsestruct definitions in every AI provider module.[BLOCKER] src-tauri/src/commands/ai.rs:317-354 (chat_message function) - PII redaction is applied twice for file attachments: once in
apply_redactionson line 325, and again in the database insert on line 347. Theapply_redactionsfunction already returns the redacted text, but the insert re-applies redaction totextinstead of using the already-redactedredacted_contentvariable.Evidence (line 347):
INSERT INTO ai_messages (id, issue_id, role, content) VALUES (?1, ?2, ?3, ?4)withtextparameter, while line 325 producedlet redacted_content = apply_redactions(...)which was never used for the message insert.Fix: Replace the
textparameter in theINSERTwithredacted_content.[BLOCKER] src-tauri/src/ai/ollama.rs:130 - The Ollama provider's
toolsparameter is still named_tools(underscore-prefixed unused variable) in the function signature, but the implementation below referencestools. This is a typo that prevents tool calling from working with Ollama.Evidence (line 130):
async fn chat(&self, messages: &[Message], _config: &ProviderConfig, _tools: Option<&[Tool]>) -> Result<ChatResponse, String> {Fix: Rename
_toolstotoolsin the function signature and use it in the body (lines 175-188 already assumetoolsis bound).[WARNING] src-tauri/src/commands/ai.rs:340-343 - PII scanning on attachment content occurs after truncation to 8 KB, but PII could straddle the truncation boundary (e.g., an SSN split across the boundary). The comment says "detect PII on full file content", but the code truncates before calling
scan_text_for_pii.Evidence (lines 330-340):
let full_content = /* decompress */; let truncated = /* slice to 8KB */; let spans = detector.detect(truncated);— butfull_contentis never scanned.Fix: Call
detector.detect(&full_content)first, then truncate at a safe boundary that doesn't split any PII spans.[WARNING] docs/2026-HACKATHON-SUMMARY.md:1803 - The version summary table shows v1.0.9 status as "IN PROGRESS", but PR #44 ("Auto-Detect Tool Calling") is marked as MERGED in the PR list above (line 1812) and in the codebase (e.g.,
detect_tool_calling_supportcommand exists).Evidence: Line 1803 shows
PR #44 | docs: Update hackathon summary with team and metrics | IN PROGRESS, while line 1812 shows17. **PR #44**: feat: Auto-Detect Tool Calling Support - MERGED.Fix: Change v1.0.9 status to "✅ Merged" in the version summary table.
[WARNING] .gitea/workflows/test.yml:17,52,80,107,134 — The git remote URL points to a private Gogs instance (
http://172.0.0.29:3000/sarman/tftsr-devops_investigation.git) hardcoded in multiple CI workflow files.Evidence (line 17):
git remote add origin http://172.0.0.29:3000/sarman/tftsr-devops_investigation.gitFix: Replace hardcoded URLs with environment variables or GitHub Actions' built-in
$GITHUB_SERVER_URL/$GITHUB_REPOSITORYfor portability.Verdict: REQUEST CHANGES
Critical issues must be fixed before merging:
user_messagefield to all AI providerChatResponsestructschat_message(useredacted_content, nottext)toolsparameter namingAutomated PR Review (qwen3-coder-next via liteLLM):\n\nSummary
This PR performs a full copy from
apollo_nxt-trcaawith complete sanitization and significant restructuring, including rebranding (TRCAA→TFTSR), comprehensive shell execution with three-tier safety classification, MCP integration, tool calling auto-detection, and database schema expansion. The changes introduce production-ready security enhancements, audit logging, and cross-incident attachment recall, with extensive test coverage and documentation.Findings
http://172.0.0.29:3000/sarman/tftsr-devops_investigation.wiki.gitin wiki sync, but repository is namedtftsr-devops_investigation, notapollo_nxt-tftsr, causing wiki sync to failEvidence:
WIKI_URL="http://172.0.0.29:3000/sarman/tftsr-devops_investigation.wiki.git"Fix: Update wiki URL pattern to use consistent repo name; verify in
.gitignoreandAGENTS.mdforAPOLLO_NXT_*legacy references that should be removedupload_log_file_by_contentfunction signature changed to acceptlog_file_id: Option<String>but caller insrc/lib/tauriCommands.ts:uploadLogFileByContentCmdpassesstring(non-nullable), causing type mismatch and runtime panic on file uploadEvidence:
pub async fn upload_log_file_by_content(log_file_id: Option<String>, file_name: String, content: String) -> Result<LogFile, String>vsexport const uploadLogFileByContentCmd = (issueId: string, fileName: string, content: string) => invoke<LogFile>("upload_log_file_by_content", { issueId, fileName, content })Fix: Align Rust backend and TypeScript frontend signatures; change Rust to accept
issueIdand derive log_file_id from it, or expose log_file_id in frontendEvidence: Commit log shows 80+ files changed in v1.0.0-v1.0.8 iterations; summary only reflects initial PR #27
Fix: Update summary to include all post-v1.0.0 iterations or reference specific PRs for each version
regexcrate but no maximum regex complexity guard, allowing catastrophic backtracking in patterns like.*?or nested groups, leading to DoS via crafted command stringsEvidence: Pattern matching for pipes, command substitution, and kubectl subcommands uses unbounded
.*withoutRegexBuilder::size_limit()Fix: Add
RegexBuilder::new().size_limit(1024 * 1024).build()for all regexes in classifier, especially forcommand_substitutiondetection patternsEvidence:
let timeout = Duration::from_secs(180);withlet max_retries = 3;andlet retry_delay = Duration::from_secs(2);Fix: Implement circuit breaker: if 3 consecutive tool calls fail, return cached health status and skip subsequent calls for 60s
get_available_tools()includesget_execute_shell_command_tool()but does not guard against duplicate registration if called multiple times during hot-reload or plugin scenariosEvidence:
vec![get_add_ado_comment_tool(), get_execute_shell_command_tool()]returns new vec each call without deduplication checkFix: Use
std::sync::OnceLock<Vec<Tool>>or track registered tool names inAppStateto prevent duplicate tool keysrespond_to_shell_approvallacks input sanitization forapproval_idanddecisionparameters; malformedapproval_id(e.g., SQL injection payloads) could reach database layer before validationEvidence:
pub async fn respond_to_shell_approval(approval_id: String, decision: String, state: State<'_, AppState>)accepts raw strings without length or character set validationFix: Add
approval_idlength limit (e.g.,uuid::Uuid::from_bytesimplies 36 chars) anddecisionwhitelist check before database operationsCHANGELOG.mdshows v1.0.9 is missing from version history, making it unclear if PR #44 is includedEvidence: Version table only lists up to v1.0.8;
v1.0.9 (June 3, 2026) - Auto-Detect Tool Calling Supportis described but not in version summary tableFix: Update table to include v1.0.9 row with PR #44 and confirm changelog entry matches
user_messagefield inChatResponseis optional but not consistently set; Ollama provider inollama.rs:chat()does not populateuser_message, causing frontend to display undefined messagesEvidence:
impl ChatResponse { ... user_message: None }in providers vschat_messagebackend command that setsSome(redacted_message)Fix: Ensure all provider implementations populate
user_messagewith the raw user message before redaction or update frontend to handle None case explicitlyVerdict: REQUEST CHANGES
Automated PR Review (qwen3-coder-next via liteLLM):\n\nSummary
This PR implements a full copy from
apollo_nxt-trcaawith complete sanitization, including new agentic shell execution, MCP server support, and extensive CI/CD improvements. A critical security issue is found: PII detection is bypassed in AI chat messages and attachments.Findings
[BLOCKER] src-tauri/src/commands/ai.rs:1195-1220 - Chat messages with typed text bypass PII detection
Evidence:
chat_message()acceptslog_file_ids: Option<Vec<String>>, loads attachments from DB, detects PII on attachment content, but for the typed message text, onlyapply_redactionsis called on&message, and the redacted form is stored in DB. However, thescan_text_for_piifunction (line 1169) is not called on&messagebeforeapply_redactions. The function signature ofscan_text_for_pii(lines 1166-1171) expectstext: Stringand has a 32KB cap, but the typed message is never passed through it. This means PII in typed chat messages is not scanned (only redacted), missing the audit log hook and potentially sending unredacted PII to AI providers.Fix: Call
scan_text_for_pii(message.clone())beforeapply_redactions, collect spans, and recordpii_types_redactedin audit log for typed messages, matching the behavior of file attachments.[WARNING] src-tauri/src/commands/analysis.rs:1166-1171 -
scan_text_for_piiinput cap is insufficient for production log file sizesEvidence:
pub fn scan_text_for_pii(text: String) -> Result<PiiDetectionResult, String> { if text.len() > 32_768 { return Err("Input exceeds 32 KB limit".into()); } ... }— while log file upload truncates to 8KB for embedding,scan_text_for_piiis used for both attachment full-content scan and typed messages. Attachment full-content scan may exceed 32KB (e.g., logs >32KB), causing the scan to be skipped entirely and no PII spans generated. The ticketTICKET-pii-bypass-chat-attachments.mdstates that PII detection runs on full file content before truncating, but this cap prevents scanning large files.Fix: Increase the cap to at least 512KB or dynamically match the upload content cap (50MB), and ensure large text is scanned in chunks if needed.
[WARNING] .gitea/workflows/auto-tag.yml:270-280 — Wiki token fallback exposes repository to unauthenticated writes
Evidence:
WIKI_URL="http://${WIKI_TOKEN}@172.0.0.29:3000/sarman/tftsr-devops_investigation.wiki.git"— ifWIKI_TOKENis not set, the URL becomeshttp://172.0.0.29:3000/...without authentication, allowing anonymous git push. Gitea may allow unauthenticated pushes if the wiki is public or if anonymous write is enabled.Fix: Fail the workflow if
WIKI_TOKENis not set, or always require authentication by using the token even when empty (e.g.,http://:${WIKI_TOKEN}@...is invalid but could fail earlier).[WARNING] src-tauri/src/shell/classifier.rs:138-165 — Shell command classification missing fork bomb detection
Evidence: Tier 3 rule includes
rm -rf,mkfs,dd,iptables -F, but does not explicitly block common shell fork bombs like:(){:|:&};:,yes > /dev/null &, orwhile true; do ... done. The classification logic uses pattern matching on command names, and fork bombs may be expressed in obfuscated forms.Fix: Add regex pattern to detect fork bombs (
:.*\{.*\|.*\}:,yes.*>.*&,while.*do.*done) and classify as Tier 3.[SUGGESTION] src-tauri/src/commands/shell.rs:56-58 —
check_kubectl_installedmissing bundled binary detection logicEvidence: The function exists but is not implemented in the provided file contents. Other modules (
ollama/installer.rs,mcp/transport/stdio.rs) uselocate_binarypatterns. Without bundled binary detection, the frontend status UI may report missing kubectl even when bundled.Fix: Implement using
locate_kubectl()fromshell/kubectl.rsand report both system path and bundled fallback.[SUGGESTION] src-tauri/src/ai/tools.rs:80-120 —
get_execute_shell_command_toolmissing kubeconfig_id constraint in parameter descriptionEvidence: The
kubeconfig_idparameter documentation says "Optional kubeconfig ID for kubectl commands. Uses active config if not specified." — this should explicitly note that the value must be a valid UUID string from thekubeconfig_filestable, and an invalid ID will cause the command to fail with a clear error.Fix: Update description: "Optional kubeconfig ID (UUID) from Settings → Shell Execution → Manage Kubeconfigs. Uses active config if omitted."
Verdict: REQUEST CHANGES
The PII detection bypass in typed messages is a security-critical issue that allows credentials and sensitive data to be sent to AI providers without scanning or audit logging. This must be fixed before merging. The other findings should also be addressed to ensure robustness and security.
Automated PR Review (qwen3-coder-next via liteLLM):\n\nSummary
This PR implements a full copy from
apollo_nxt-trcaawith complete sanitization, including extensive security hardening (PII detection, encryption, audit logging), an agentic shell command execution system with three-tier safety classification, MCP server integration, kubectl binary bundling, and database schema modernization. A critical BLOCKER was found in the CI/CD workflows.Findings
[BLOCKER] .gitea/workflows/auto-tag.yml:266 - Hardcoded IP
172.0.0.29used for internal Gitea services in CIEvidence:
WIKI_URL="http://${WIKI_TOKEN}@172.0.0.29:3000/sarman/tftsr-devops_investigation.wiki.git"Fix: Replace with configurable environment variables or Docker service DNS (
wiki-service,gitea-service) to avoid hardcoding IPs that will change in production.[BLOCKER] .gitea/workflows/auto-tag.yml:261-262 - Wiki sync fails if the target wiki repository does not exist
Evidence:
if ! git clone "$WIKI_URL" wiki 2>/dev/null; then ... git checkout -b master— attempts to push to a non-existent branch on origin without checking if the remote wiki repo actually existsFix: Add a pre-check (e.g., HTTP HEAD request to the wiki repo's
info/refsorgit ls-remote) before cloning; if it fails, skip the sync with a warning and skip the push.[BLOCKER] .gitea/workflows/auto-tag.yml:411-418 - Release asset upload logic contains duplicate asset deletion blocks and may fail on concurrent runs
Evidence: The asset deletion loop
printf '%s\n' "$EXISTING_IDS" | while IFS= read -r id; do ... doneruns inside awhile IFS= read -r floop over artifacts — this re-deletes assets for every file in the list, causing race conditions on rerunsFix: Extract asset deletion logic into a single, separate loop before the artifact upload loop, or ensure deletion is idempotent by checking asset existence after listing.
[WARNING] docs/2026-hackathon_AgenticFeature.md:1204 - Command classifier does not handle shell built-in aliases that resolve to dangerous commands (e.g.,
lsaliased tols --color=always | rm -rf)Evidence: Classification parses raw command string and pipes/strings but does not resolve shell aliases (which are expanded by the shell before classification)
Fix: Document this limitation clearly and require users to use full paths in shell commands to avoid alias bypass; consider adding a warning modal before executing commands with shell: true.
[WARNING] docs/2026-hackathon_AgenticFeature.md:1286 - kubectl binary bundling stores binaries in
src-tauri/externalBin/which is not listed in .gitignore in the changed .gitignore fileEvidence:
.gitignorelistssrc-tauri/target/andsrc-tauri/binaries/but notsrc-tauri/externalBin/; the PR adds# kubectl binaries (downloaded during build) src-tauri/binaries/but not the externalBin directoryFix: Add
src-tauri/externalBin/to.gitignoreto prevent accidentally committing large binary artifacts.Verdict: REQUEST CHANGES
Automated PR Review (qwen3-coder-next via liteLLM):\n\nSummary
The PR claims a "full copy from apollo_nxt-trcaa with complete sanitization", but introduces significant data loss and inconsistency issues: (1) 17 duplicate migration entries in the database schema with no clear resolution, potentially causing runtime errors during migration application, and (2) the removal of critical documentation files that were part of the prior state, breaking auditability and continuity.
Findings
[BLOCKER] src-tauri/src/db/migrations.rs:101 - Duplicate migration
011_create_integrationsappears three times (lines 101, 119, 124), and020_add_log_content_compressedand021_add_image_dataeach appear twice (lines 107, 120 and 108, 121 respectively). This will cause runtime panics or silent failures when migrations are applied, as rusqlite's migration system expects unique version numbers. Theapply_migrations()function does not deduplicate or skip identical IDs, leading to unpredictable database state.Evidence:
011_create_integrationson lines 101, 119, 124;020_add_log_content_compressedon lines 107, 120;021_add_image_dataon lines 108, 121.Fix: Remove all duplicate migration entries from the list in
migrations.rs, retaining only one instance of each migration ID in ascending order (e.g., keep line 101, 107, 108, 109, and delete 119-124).[BLOCKER] CHANGELOG.md:0 - The changelog was completely replaced with a new file, erasing years of commit history, release notes, and bug fixes documented in the prior project state. The previous changelog (e.g., for v0.2.x releases) is lost, breaking traceability and compliance requirements that depend on full historical audit trails.
Evidence: The new CHANGELOG.md begins at v0.3.9, with no reference to prior versions or commits before the PR base.
Fix: Restore the previous changelog content as a historical baseline and append new entries, preserving full version history.
[BLOCKER] docs/2026-hackathon_AgenticFeature.md:1 — A critical feature document for the v1.0.0 Agentic Shell feature is now a newly added file with no prior state to compare against. If the feature was present in apollo_nxt-trcaa, this is a data loss; if it is new, the baseline must be preserved. Either way, the file is missing from previous commits, indicating a copy-paste error rather than a full migration.
Evidence: File is listed as "new file mode 100644" in the diff, not a modification of an existing file.
Fix: Confirm whether this file existed in the source repo. If yes, ensure the original path and content are preserved; if no, add a clear migration note.
[WARNING] .gitea/workflows/auto-tag.yml:260 — Docker image tag references changed from
trcaa-*totftsr-*across multiple workflow jobs (lines 263, 333, 434), but no correspondingbuild-images.ymlchanges were made to reflect the new image names in the push step. Thebuild-images.ymlworkflow still pushes totftsr-*images (lines 68, 94, 119), so the tag change is incomplete and may cause CI failures if builds rely on the old tag pattern.Evidence: Line 263:
image: 172.0.0.29:3000/sarman/tftsr-linux-amd64:rust1.88-node22, butbuild-images.ymlline 68 usestftsr-linux-amd64. This is consistent, but the PR description claims it's a "full copy" — iftrcaa-*was correct, these changes are unintentional.Fix: Confirm the canonical image naming scheme with CI owners. If the old scheme (
trcaa-*) was correct, revert to those names.[WARNING] .gitignore:25 — SECURITY_AUDIT.md is listed in
.gitignore, which would prevent it from being committed. Given the PR title emphasizes security and sanitization, committing this file is likely intentional. Its inclusion in.gitignorecontradicts the stated goal of a "complete sanitization" that should preserve security artifacts.Evidence:
SECURITY_AUDIT.mdappears in.gitignoreline 25, yet the PR changes includeSECURITY_AUDIT.mdin the file list (implying it was meant to be committed).Fix: Remove
SECURITY_AUDIT.mdfrom.gitignoreso it can be tracked.Verdict: REQUEST CHANGES
Automated PR Review (qwen3-coder-next via liteLLM):\n\nSummary
This PR implements comprehensive shell command execution with three-tier safety classification, integrates MCP server support, and includes PII detection/enforcement improvements. Critical security and correctness issues were found in CI workflows, database schema duplication, and file handling.
Findings
[BLOCKER] .gitea/workflows/auto-tag.yml:263 - Hardcoded Gitea URL in CI jobs without authentication support
Evidence:
WIKI_URL="http://172.0.0.29:3000/sarman/tftsr-devops_investigation.wiki.git"— no token fallback whenWIKI_TOKENis unset, causing auth failuresFix: Use
http://172.0.0.29:3000/sarman/tftsr-devops_investigation.wiki.gitwith explicit empty token handling or switch to HTTPS with token authentication.[BLOCKER] src-tauri/src/db/migrations.rs:219 - Duplicate migration entries for
011_create_integrations,020_add_log_content_compressed,021_add_image_data,022_attachment_viewsEvidence: List contains
011_create_integrationsthree times;020,021,022each appear twiceFix: Remove duplicate entries — use unique migration IDs (029, 030, etc.) or consolidate into single correct entries.
[BLOCKER] docs/2026-hackathon_AgenticFeature.md:397-403 — Misleading example suggests running
git initinside container without remote setupEvidence:
git initfollowed bygit remote add origin ...thengit fetch --depth=1 origin "$GITHUB_SHA"—originis undefined before fetch unless remote is addedFix: Either replace
git initwithgit cloneor add remote beforegit fetch.[WARNING] src-tauri/src/ollama/installer.rs:132 — Potential binary path injection in command construction
Evidence:
format!("curl -sSL {} -o {}", tarball_url, bin_path)—bin_pathderived from user可控 input (ollama_dir) without sanitizationFix: Validate
bin_pathis within safe directory, usePath::joinand canonicalization, or hardcode known paths.[SUGGESTION] .docker/Dockerfile.linux-arm64:35 — Missing
makein Step 1 breaks vendored OpenSSL buildEvidence: Step 4 installs Node.js but Step 1 lacks
make, required for./configure && makein Step 5 (OpenSSL vendoring)Fix: Add
maketoapt-get installin Step 1.[SUGGESTION] src-tauri/src/ollama/installer.rs:78-81 — Redundant
.exists()check beforeremove_dir_allEvidence:
if bin_dir.join("ollama").exists() { std::fs::remove_dir_all(...) }—remove_dir_allhandles non-existent paths gracefullyFix: Remove
exists()check; catch errors explicitly if needed.[SUGGESTION] cli/src/main.ts:34-41 — CLI describes “full AI-powered triage” while no actual AI integration exists
Evidence: Prompts users to “launch the TRCAA desktop application” for “AI-powered analysis” but provides no backend CLI integration
Fix: Update help text to clarify CLI is for basic operations only; add
--dry-runor fallback notice.[SUGGESTION] docs/MCP_SERVER_SUPPORT.md:37 — Test checklist item not actionable — “All existing tests continue to pass (185 Rust, 94 Vitest)”
Evidence: Numbering doesn’t match current counts in codebase (Rust: 226 tests, Vitest: 103)
Fix: Replace with actual expected counts or remove specific numbers.
[SUGGESTION] docs/2026-hackathon_AgenticFeature.md:1605 — Example kubectl binary path uses incorrect OS string for macOS
Evidence:
"https://dl.k8s.io/release/$KUBECTL_VERSION/bin/darwin/amd64/kubectl"should bedarwin/amd64notdarwin/amd64/kubectlFix: Ensure download script uses correct platform strings (
macos,darwin).Verdict: REQUEST CHANGES
Automated PR Review (qwen3-coder-next via liteLLM):\n\nSummary
This PR implements a major "full copy from apollo_nxt-trcaa" with complete sanitization, including extensive CI/CD infrastructure, security features (PII detection, encryption, audit logging), and new agentic capabilities (shell execution, MCP server integration). Multiple security-critical features are already battle-tested and production-ready, and the codebase is well-documented and thoroughly tested.
Findings
[BLOCKER] .gitea/workflows/auto-tag.yml:263 - Hardcoded internal IP address
172.0.0.29used for all Gitea/Gogs internal service communication, including artifact uploads and wiki sync. This makes the build pipeline completely non-portable and fails outside the author's internal network.Evidence:
GITHUB_WORKSPACE/docs/wiki/*.md . 2>/dev/null || echo "No wiki files to copy"\n git add -A\n if ! git diff --staged --quiet; then\n git commit -m "docs: sync from docs/wiki/ at commit ${GITHUB_SHA:0:8}"\n echo "Pushing to wiki..."\n if git push origin master; then\n echo "✓ Wiki successfully synced"\n else\n echo "⚠ Wiki push failed - check token permissions"\n exit 1\n fi\n else\n echo "No wiki changes to commit"\n fi\n \n build-linux-amd64:\n needs: autotag\n runs-on: linux-amd64\n container:\n- image: 172.0.0.29:3000/sarman/trcaa-linux-amd64:rust1.88-node22\n+ image: 172.0.0.29:3000/sarman/tftsr-linux-amd64:rust1.88-node22Fix: Replace hardcoded internal IPs with configurable environment variables or relative Docker image references (e.g., assume builder images are built/pulled in same CI stage) to enable open-source usage.
[BLOCKER] .docker/Dockerfile.windows-cross:37-43 - Runtime download and compilation of libsodium from external source (download.libsodium.org) inside Docker build without integrity verification. This introduces supply chain attack risk and build reproducibility issues.
Evidence:
# Pre-build libsodium for x86_64-pc-windows-gnu so libsodium-sys-stable\n# does not attempt a network download at cargo build time (no DNS in CI containers).\nRUN set -eu \\\n && SODIUM_VER="1.0.20" \\\n && curl -fsSL "https://download.libsodium.org/libsodium/releases/libsodium-${SODIUM_VER}.tar.gz" \\\n | tar -xz -C /tmp \\\n && cd "/tmp/libsodium-${SODIUM_VER}" \\\n && ./configure \\\n --host=x86_64-w64-mingw32 \\\n --prefix=/usr/x86_64-w64-mingw32 \\\n --disable-shared \\\n --enable-static \\\n && make -j"$(nproc)" \\\n && make install \\\n && rm -rf "/tmp/libsodium-${SODIUM_VER}"Fix: Use a pre-built libsodium package from the Debian/Ubuntu MinGW repository or download the tarball, verify its SHA256 checksum, and cache it in the repo or CI.
[WARNING] docs/2026-hackathon_AgenticFeature.md:1533-1537 - Suggests bundling kubectl binaries in
src-tauri/externalBin/but contradicts the .gitignore entry# kubectl binaries (downloaded during build)\nsrc-tauri/binaries/. This inconsistency will cause build failures or accidental binary commits if CI uses the wrong directory.Evidence:
mkdir -p "$EXTERNAL_BIN_DIR"\n\n# Linux amd64\ncurl -L -o "$EXTERNAL_BIN_DIR/kubectl-x86_64-unknown-linux-gnu" \\\n "https://dl.k8s.io/release/$KUBECTL_VERSION/bin/linux/amd64/kubectl"Fix: Standardize the binary download path in both
.gitignoreand the script (e.g., usesrc-tauri/binaries/everywhere).[WARNING] docs/2026-hackathon_AgenticFeature.md:1675 - Proposes
curl -sS -o "$RESP_FILE" -w "%{http_code}" -X POST "$API/releases/$RELEASE_ID/assets"to upload assets without checking if the API is accessible, potentially causing indefinite hangs if the internal Gitea host is unreachable.Evidence:
HTTP_CODE=$(curl -sS -o "$RESP_FILE" -w "%{http_code}" -X POST "$API/releases/$RELEASE_ID/assets" \\\n -H "Authorization: token $RELEASE_TOKEN" \\\n -F "attachment=@$f;filename=$UPLOAD_NAME")Fix: Add a timeout and a health-check step before attempting uploads to avoid stuck CI jobs.
[WARNING] cli/src/main.ts:112-115 - CLI stubs functionality that is not implemented, misleading users into thinking it works.
config setcommand outputs "Configuration is managed by the TRCAA desktop application" instead of actually persisting configuration.Evidence:
case "set":\n console.log(Configuration: ${args[2]} = ${args[3]});\n console.log("Note: Configuration is managed by the TRCAA desktop application.");\n break;Fix: Either fully implement CLI configuration management or remove
config setfrom help documentation.[SUGGESTION] README.md:214-216 - Uses inconsistent brand name (TRCAA vs tftsr) across documentation (README.md, AGENTS.md, CHANGELOG.md), which could confuse users. The PR title says "full copy from apollo_nxt-trcaa", suggesting this is a rename but some files still reference the old name.
Evidence:
# Troubleshooting and RCA Assistant\n\nA structured, AI-backed desktop tool for IT incident triage...Fix: Ensure consistent naming across all documentation (the new name should be
tftsrorTRCAAuniformly). SinceREADME.mdsays "TRCAA IT Triage & RCA", clarify whether the app is being rebranded to "tftsr" or kept as "TRCAA".Verdict: REQUEST CHANGES
The hardcoded internal IPs and unverified external downloads constitute security and portability blockers. These must be fixed before merge. The other findings are important for maintainability and user experience but are less critical.
Automated PR Review (qwen3-coder-next via liteLLM):\n\nSummary
This PR implements a comprehensive "full copy from apollo_nxt-trcaa with complete sanitization" by refactoring the codebase for rebranding (trcaa → tftsr), adding a new agentic shell execution feature with three-tier safety classification, MCP server support, and extensive PII detection fixes. Multiple critical security issues were identified, including potential PII bypasses in chat attachments and typed messages, and critical SQL injection risks in database query handling.
Findings
[BLOCKER] src-tauri/src/commands/analysis.rs:69 - SQL injection vulnerability in log file upload
Evidence:
db.execute("INSERT INTO log_files (id, issue_id, file_name, file_path, file_size, file_hash, content_compressed, created_at) VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8)", rusqlite::params![...])Fix: The
issue_idparameter is not validated before being used in the INSERT statement. While rusqlite uses parameterized queries, theissue_idis passed directly from user input without validation. Always validate thatissue_idis a valid UUID format before use.[BLOCKER] src-tauri/src/db/migrations.rs:262 - Duplicate migration IDs in database schema
Evidence:
011_create_integrationsappears three times in the migration list, and migration IDs020_add_log_content_compressed,021_add_image_data, and022_attachment_viewseach appear twiceFix: Remove duplicate migration entries. Each migration should have a unique ID.
[WARNING] src-tauri/src/commands/ai.rs:897-907 - PII scan does not block execution on detection
Evidence:
if !spans.is_empty() { tracing::warn!(tool = %tool_call.name, pii_spans = spans.len(), "PII detected in tool call arguments"); }Fix: The comment in docs/TICKET-pii-bypass-chat-attachments.md states "Auto-redact, not block", so this is intentional behavior. No change needed.
[WARNING] src-tauri/src/commands/shell.rs:141 - No timeout enforcement for shell command execution
Evidence: The
execute_shell_commandcommand lacks timeout parameters or enforcementFix: Add timeout configuration and enforcement to prevent indefinite hanging from long-running commands.
[WARNING] .gitea/workflows/auto-tag.yml:260-280 - Hardcoded Gitea IP address exposes internal network
Evidence:
WIKI_URL="http://${WIKI_TOKEN}@172.0.0.29:3000/sarman/tftsr-devops_investigation.wiki.git"Fix: Use a configuration variable or environment variable for the internal IP address instead of hardcoding it.
[WARNING] docs/2026-hackathon_AgenticFeature.md:1649 - Shell execution approval bypass risk in command substitution detection
Evidence: "Commands containing
$()or backticks are automatically escalated to Tier 2 for approval."Fix: The detection logic needs to be more robust to prevent bypass via alternative injection techniques (e.g.,
${var},$(echo test)).Verdict: REQUEST CHANGES
Automated PR Review (qwen3-coder-next via liteLLM):\n\nSummary
The PR introduces a full copy from
apollo_nxt-trcaawith "complete sanitization", but the changes contain several critical issues: hardcoded internal Gitea URLs, missing input validation in critical backend commands, and insecure storage of sensitive credentials (like OAuth tokens and API keys) in the database without proper key derivation or encryption context. These findings indicate potential data leakage, audit trail bypass, and unauthorized access risks.Findings
[BLOCKER]
.gitea/workflows/auto-tag.yml:282- Hardcoded internal Gitea URL (172.0.0.29:3000) in CI workflow container image tagEvidence:
image: 172.0.0.29:3000/sarman/tftsr-linux-amd64:rust1.88-node22Fix: Replace hardcoded registry host with a configuration-driven or environment variable-based value to avoid hardcoding internal infrastructure.
[BLOCKER]
src-tauri/src/commands/shell.rs:115-upload_kubeconfigcommand stores kubeconfig content as raw encrypted string without validation or schema enforcementEvidence: No validation of kubeconfig YAML structure before encryption and storage; any malformed content results in DB corruption or silent failure later during activation.
Fix: Parse kubeconfig YAML with
serde_yamlfirst, validate required fields (clusters,users,contexts) before encryption; return clear error if invalid.[WARNING]
src-tauri/src/ai/openai.rs:76-chatmethod usestemperature: Nonefor OpenAI which maps to server default (~0.7), butdetect_tool_calling_supporthardcodestemperature: 0.0in the same fileEvidence:
temperatureisNoneinchat, butdetect_tool_calling_supportexplicitly setstemperature: Some(0.0)viaconfig.temperature = Some(0.0)Fix: Inconsistent use of temperature may cause unpredictable behavior; unify configuration or document why defaults differ.
[WARNING]
src-tauri/src/integrations/auth.rs:44-decrypt_tokenreuses the same nonce across calls (nonce is a const string or reused value)Evidence: Nonce is not shown explicitly, but
encrypt_tokenanddecrypt_tokenboth use a fixedAES_GCMkey and no unique nonce per encryption is evident in the pattern (common anti-pattern).Fix: Use a cryptographically random nonce per encryption operation and store it alongside the ciphertext (e.g., prepend to
encrypted_token).[SUGGESTION]
.gitignore:21-SECURITY_AUDIT.mdis added to.gitignore, but it appears in the root directory and was committed previouslyEvidence:
SECURITY_AUDIT.mdappears inREADME.mdassets andAGENTS.md, suggesting it should be tracked or removed entirely.Fix: Decide whether this file should be committed or truly internal; if internal, ensure it's not referenced externally.
[SUGGESTION]
cli/src/main.ts:17— CLI binarytrcaa-clireports version0.1.0but backend version isv1.0.8+per changelogEvidence:
printVersion()returns"TRCAA CLI v0.1.0"regardless of app version.Fix: Sync CLI version with
tauri.conf.jsonorCargo.tomlvia build script or runtime fallback.Verdict: REQUEST CHANGES
Automated PR Review (qwen3-coder-next via liteLLM):\n\nSummary
The PR introduces a full codebase copy with comprehensive sanitization and security enhancements—including PII detection, shell command safety classification (three-tier system), MCP integration, and encrypted storage. Most changes appear intentional and secure, but two critical issues are found: (1) the Windows cross-compile Dockerfile attempts to install MinGW and build libsodium, but the path for
--prefixin the configure command is incorrect, leading to build failure and potential runtime failure; and (2) theAGENTS.mdandCLAUDE.mdfiles still reference outdated and insecure environment variable names (TRCAA_*vs legacyTFTSR_*) whileAGENTS.mdalso contains a typo in a shell command example that risks executing commands withsh -c, violating the project’s own security policies.Findings
[BLOCKER]
.docker/Dockerfile.windows-cross:38-42— libsodium configure uses--prefix=/usr/x86_64-w64-mingw32butmingw-w64cross-toolchain on Debian/Ubuntu uses--prefix=/usr/x86_64-w64-mingw32without/usrin the prefix path for the sysroot.Evidence:
Fix: Use
--prefix=/usror ensure the libsodium build system’sx86_64-w64-mingw32sysroot is correctly set up—however, the standard approach is to letmeson/autotoolsdetect cross-prefix automatically and avoid manually overriding prefix. The safest fix is to omit--prefixentirely or use--prefix=/usr/x86_64-w64-mingw32only if that directory exists in the sysroot (it typically does not); instead, configure libsodium with--prefix=/usrand rely on cross-compilation helpers, or simply remove the manual install and letlibsodium-sys-stabledownload at build time (CI containers allow DNS, and the comment about DNS is outdated—add DNS to CI config instead).[WARNING]
AGENTS.md:221-222— Example shell command for manual wiki sync usessource ~/.cargo/env, but later in the same file (line 23) the project explicitly states: “Rust toolchain must be in PATH:source ~/.cargo/env”, yet the actual codebase usesTRCAA_*env vars, notTFTSR_*. This inconsistency suggests legacy references persist and could cause confusion or misconfiguration.Evidence:
Fix: Replace
/tmp/apollo-wikiwith/tmp/tftsr-wikito match the new repository name consistently throughout the codebase.[WARNING]
AGENTS.md:221— Example suggestssource ~/.cargo/envbut does not clarify that environment is not persistent across terminal sessions; however, the more critical issue is the incorrect use ofsourcein bash vs zsh for cross-platform documentation, potentially confusing Windows or macOS users using non-bash shells. This is a documentation hazard leading to misconfiguration.Evidence:
AGENTS.md:23: “Rust toolchain must be in PATH:source ~/.cargo/env” — no mention of shell type or Windows PowerShell alternative (~/.cargo/env.ps1), despite the project supporting Windows and macOS.[SUGGESTION]
AGENTS.md:273-274— CI/CD table usestftsr-*registry image names, but earlierAGENTS.md:221references/tmp/apollo-wiki, conflicting repo names (tftsr-devops_investigationvsapollo_nxt-tftsr); whileCHANGELOG.mdandREADME.mdconsistently usetftsr, the inconsistency suggests legacy naming remains.Evidence:
followed by
tftsr-*:rust1.88-node22names, butAGENTS.md:221usesapollo-wiki.Fix: Standardize all references to
tftsr-devops_investigationortftsrdepending on context; the codebase usestftsr-devops_investigationin.gitignore,.gitea/workflows/*, etc.[SUGGESTION]
CLAUDE.md:31— Still referencesmainbranch, while.gitea/workflows/auto-tag.ymlshowsmasterbranch as the default merge target in CI triggers.Evidence:
CLAUDE.md:31: “- Release pipeline:.github/workflows/release.yml— runs on every push tomain, auto-tags…”Fix: Change
main→masterthroughout to match CI and repository defaults.[SUGGESTION]
docs/architecture/adrs/ADR-002-sqlcipher-encrypted-database.md:37-38— UsesTFTSR_DB_KEY(legacy name), while all new code usesTRCAA_DB_KEY. The migration (migrate_plain_to_encrypted()) in the codebase usesTRCAA_DB_KEY, making the ADR inconsistent with implementation.Evidence:
ADR-002-sqlcipher-encrypted-database.md:37: “Override:TFTSR_DB_KEYenvironment variable”Fix: Replace
TFTSR_DB_KEY→TRCAA_DB_KEYconsistently across all documentation and ADRs.[SUGGESTION]
docs/architecture/adrs/ADR-005-auto-generate-encryption-keys.md:12-13— UsesTFTSR_*env var names while Rust code and.dockerfiles useTRCAA_*. The ADR says “auto-generate”, but the environment variable fallback naming mismatch could cause operational confusion in enterprise deployments.Evidence:
ADR-005-auto-generate-encryption-keys.md:12: “Override:TFTSR_DB_KEY(or legacyTFTSR_DB_KEY) environment variable”[SUGGESTION]
docs/architecture/adrs/ADR-007-three-tier-shell-safety.md:61— Tier 2 example listschmodwithout context; while technically accurate,chmodcan be safe (e.g.,chmod 644 file) or dangerous (e.g.,chmod 777 /). This imprecision could mislead users or auditors into underestimating risk.Evidence:
ADR-007-three-tier-shell-safety.md:61: “Tier 2 — Mutating, potentially disruptive…chmod,systemctl restart”[SUGGESTION]
docs/2026-hackathon_AgenticFeature.md:458-459— The scriptscripts/download-kubectl.shuseschmod +xon Windows binaries, which is harmless but misleading; Windows executables don’t need execute bits in Linux filesystems. This could cause CI confusion.Evidence:
However, this is minor and intentional (no Windows chmod in script), not an error.
Verdict: REQUEST CHANGES
Automated PR Review (qwen3-coder-next via liteLLM):\n\nSummary
This PR implements a comprehensive "full copy" from
apollo_nxt-trcaawith extensive sanitization, including major new features like agentic shell execution (with three-tier safety classification), MCP server support, and database enhancements. However, critical security and correctness issues were identified.Findings
[BLOCKER] .gitea/workflows/auto-tag.yml:274 - Hardcoded IP
172.0.0.29:3000used for internal services and Gitea API calls across all CI workflowsEvidence:
WIKI_URL="http://${WIKI_TOKEN}@172.0.0.29:3000/..."andAPI="http://172.0.0.29:3000/api/v1/repos/..."(lines 267, 312, 433, 523)Fix: Replace hardcoded IP with configurable environment variable (e.g.,
GITEA_URL), document required pre-flight setup[BLOCKER] .gitea/workflows/build-images.yml:104 - Docker login uses
RELEASE_TOKENas both username and password, but should use dedicatedREGISTRY_USEREvidence:
echo "$RELEASE_TOKEN" | docker login $REGISTRY -u $REGISTRY_USER --password-stdin(line 52)Fix: This is actually correct —
$RELEASE_TOKENis the password,$REGISTRY_USERis the username. No fix needed.[BLOCKER] cli/src/main.ts:84 - CLI says "Launch the TRCAA desktop application" instead of "TFTSR" (branding inconsistency)
Evidence:
console.log("For AI-powered analysis, launch the TRCAA desktop application.");(line 84)Fix: Update to consistent branding: "Launch the TFTSR desktop application."
[BLOCKER] src-tauri/src/ai/mod.rs:1 -
ChatResponsestruct added withuser_message: Option<String>but no#[derive(Serialize)]Evidence:
pub struct ChatResponse { pub content: String, pub tool_calls: Option<Vec<ToolCall>>, pub why_level: Option<i32>, pub user_message: Option<String> }(line 43 inai/mod.rs)Fix: Add
#[derive(Serialize)]toChatResponseor this will fail to serialize over IPC[WARNING] src-tauri/src/db/migrations.rs:1 - Duplicate migration names present (
011_create_integrationsappears 3 times,020_add_log_content_compressedtwice)Evidence:
011_create_integrations(lines 253, 651, 652),020_add_log_content_compressed(lines 464, 651)Fix: Remove duplicate entries — this indicates migration versioning drift
[WARNING] src-tauri/src/db/migrations.rs:1 - Migration 027
approval_decisionsmissingsession_idforeign key constraintEvidence:
CREATE TABLE approval_decisions (...)(lines 549-559) —session_idcolumn exists but has noREFERENCES sessions(id)Fix: Add
session_id TEXT REFERENCES sessions(id)for referential integrity or document intentional omission[WARNING] src-tauri/src/commands/ai.rs:1 -
execute_shell_tool_callimportscrate::pii::detector::PiiDetectorbut PII detection in command arguments is documented as "non-blocking warning" — actual PII in shell commands should NOT block executionEvidence:
if !spans.is_empty() { tracing::warn!(...); }(line 897) — this is correct per spec, but the comment "non-blocking warning" is accurateFix: No change needed — behavior is as designed.
[WARNING] src-tauri/src/ollama/installer.rs:1 -
is_safe_filefunction exists but may not cover all edge cases for bundle installationEvidence:
pub fn is_safe_file(path: &Path) -> bool(line insrc-tauri/src/ollama/installer.rs)Fix: Review
is_safe_filelogic — ensure it rejects symlinks, paths with.., and non-regular files[WARNING] docs/architecture/adrs/ADR-002-sqlcipher-encrypted-database.md:1 -
cipher_page_size = 16384tuned for Apple Silicon, but macOS Intel uses 4096-page kernelsEvidence:
PRAGMA cipher_page_size = 16384; -- Matches 16KB kernel page (Apple Silicon)(line 62)Fix: Document that page size mismatch requires migration or use dynamic detection
[SUGGESTION] .gitea/workflows/auto-tag.yml:260 - Wiki sync URL uses hardcoded internal IP — should be external-facing URL if wiki is public
Evidence:
WIKI_URL="http://${WIKI_TOKEN}@172.0.0.29:3000/..."(line 267)Fix: Consider using
https://gogs.tftsr.com/sarman/tftsr-devops_investigation.wiki.gitif externally accessibleVerdict: REQUEST CHANGES
The critical issues must be fixed before merge:
Serializederive breaks IPC