fix(ui): hide environment variable values and mask inputs as password

[bluelovers]

Summary

• Display only environment variable names in instance info panel (hide values)
• Display only environment variable names in logs view (hide values)
• Change environment variable value inputs to password type in settings editor

Motivation

Environment variable values (which may contain sensitive data like API keys, tokens, or secrets) were displayed in plain text across multiple UI surfaces. This change:

1. Reduces visual exposure — instance info and logs view now show only variable names
2. Masks input fields — the settings editor uses type="password" for value inputs, preventing shoulder-surfing and accidental exposure in screenshots or screen recordings

[shantur]

@CodeNomadBot

I want you to perform the gatekeeper checks for latest PR state

check for regressions
check for better possible implementations
Be the PR gatekeeper
Be ruthless about code quality

[codenomadbot]

Gatekeeper review for the latest PR state.

What I understood: this PR is meant to stop exposing environment variable values in passive UI surfaces, while still allowing editing in settings via masked inputs. I reviewed the current branch diff (cd16800) against origin/dev and checked for regressions / stronger implementations.

Findings:

1. Medium: packages/ui/src/components/instance-info.tsx:154-161 does not actually implement the stated behavior of "display only environment variable names". The code still renders a second value column with a hard-coded ***. That avoids leaking the secret, but it is not the same UX as logs view and not the same behavior described in the PR summary. If the intent is truly names-only, this should render just the key and remove the placeholder column.

2. Medium: packages/ui/src/components/environment-variables-editor.tsx:83-90 and :119-126 switch these fields to type="password", but they do not disable password-manager/autofill heuristics. In Chromium-based shells this can trigger credential suggestions or save-password behavior on fields that are not account passwords. At minimum I would expect autocomplete="off" or autocomplete="new-password", and likely spellcheck={false} / autocapitalize="off", to avoid a regression in data entry behavior.

What I checked:

• Diff scope is limited to 3 UI files.
• No existing PR review comments were present.
• I attempted a local UI build, but verification is blocked in this workspace because the installed dependencies are incomplete: vite is missing, so npm run build:ui fails before compiling.

Non-blocking suggestion:

• packages/ui/src/styles/components/env-vars.css:22-28 still contains .env-var-separator and .env-var-value, which appear unused after this change. I would not block on that, but it is leftover styling debt from the old implementation.

--
Yours,
CodeNomadBot

[shantur]

@bluelovers - Seeing the values is helpful for non-secret environment variables but I understand the requirement.
Maybe the correct implementation would be to add a flag for each environment variable called, Secure that only hides the value of that particular variable and rest all stay the same.

[shantur]

@CodeNomadBot

Do your thing again
Also check if the new implementation matches my suggestion

[codenomadbot]

Gatekeeper review for the latest PR state.

What I understood: you wanted a fresh gatekeeper pass on the updated implementation, and specifically a check for whether it now matches your suggestion from the thread: add a per-environment-variable Secure flag so only selected variables hide their values.

Conclusion: the new implementation does not match that suggestion. It still treats all environment variable values as sensitive in passive views, and it adds a local show/hide toggle in the editor instead of introducing a per-variable Secure model.

Findings:

1. Medium: the implementation still does not match the suggested design. packages/ui/src/components/environment-variables-editor.tsx:17-42, packages/ui/src/components/instance-info.tsx:153-161, and packages/ui/src/components/logs-view.tsx:111-115 add masked editor inputs and hide all displayed values, but there is no data model, UI control, or rendering branch for a per-variable Secure flag. If the expectation is to follow the suggestion, this PR does not do that.

2. Medium: the new PasswordInput wrapper introduced a keyboard regression in the add-new-variable row. Previously, the value field had onKeyPress={handleKeyPress} so pressing Enter from either input could submit the new variable. In the current code, packages/ui/src/components/environment-variables-editor.tsx:156-161 renders PasswordInput without any key handler support, and PasswordInput itself (:17-42) does not forward onKeyPress. That means Enter in the value field no longer adds the variable.

3. Low: the new visibility-toggle control hardcodes user-facing strings in English: packages/ui/src/components/environment-variables-editor.tsx:36 uses "Hide value" / "Show value" directly instead of the existing i18n layer. That is inconsistent with the rest of the component and will not localize.

4. Low: the previous autofill/password-manager concern is still present. packages/ui/src/components/environment-variables-editor.tsx:23-29 renders a password field without autocomplete guidance, so Chromium-based password-manager heuristics may still interfere.

What changed versus the last review:

• The logs view now correctly shows names only.
• The instance info view still renders a second placeholder column (***) rather than becoming truly names-only.
• The editor now adds a visibility toggle, but that is a different solution than the suggested per-variable Secure flag.

Non-blocking suggestion:

• If the team wants the suggested approach, this likely needs a small data-shape change rather than more UI-only masking logic: something like storing { value, secure } per env var, then using secure to decide whether passive views hide the value.

--
Yours,
CodeNomadBot

[bluelovers]

I want to keep the changes minimal to avoid potential merge conflicts.

Regarding the environment masking feature, implementing it might require updates to the existing configuration schema (e.g., adding an enabled field or a list of target environments). I am concerned that large-scale changes to the config structure might lead to merge difficulties or deviate from the project's design philosophy. I would prefer a solution that is as non-intrusive as possible to keep the PR focused and easy to integrate.

[shantur]

@bluelovers - I prefer things done properly in the first go to keep the quality of the codebase high

[github-actions]

PR builds are available as GitHub Actions artifacts:

https://github.com/NeuralNomadsAI/CodeNomad/actions/runs/23974801196

Artifacts expire in 7 days.
Artifacts: (none found on this run)

[github-actions]

PR builds are available as GitHub Actions artifacts:

https://github.com/NeuralNomadsAI/CodeNomad/actions/runs/23974801196

Artifacts expire in 7 days.
Artifacts: (none found on this run)

[shantur]

@CodeNomadBot

Do your thing again

[codenomadbot]

Gatekeeper review for the latest PR state.

What I understood: you wanted another fresh gatekeeper pass on the newest revision, with emphasis on regressions, implementation quality, and whether the branch now follows the earlier suggestion to support per-variable secure masking.

Conclusion on the suggestion: yes, the new direction now broadly matches that suggestion. The branch introduces a secureEnvVars preference and uses it to hide only selected environment variable values in passive views, instead of hiding all values globally.

Findings:

1. High: packages/ui/src/stores/preferences.tsx:318-325 and :485-517 contain duplicate function implementations for normalizeSecureEnvVars, isSecureEnvVar, and toggleSecureEnvVar. In TypeScript modules this is a compile-time error (Duplicate function implementation) and should block merge.

2. Medium: packages/ui/src/components/environment-variables-editor.tsx:89-96 and :137-144 reverted the value inputs back to type="text". That drops the masking behavior that the PR originally set out to add for the settings editor, so secure values are again visible while being edited. Even if the new per-variable secure model is the right direction for passive displays, this is still a regression relative to the stated goal of masking value inputs.

What changed versus the prior review:

• The implementation now does match the earlier architectural suggestion at a high level: there is a per-variable secure flag model (secureEnvVars) instead of a blanket hide-all approach.
• The previous keyboard regression from the temporary PasswordInput wrapper is gone because that wrapper was removed.
• The previous English-only show/hide strings are also gone with that wrapper removal.

Additional notes:

• packages/ui/src/components/instance-info.tsx and packages/ui/src/components/logs-view.tsx now use isSecureEnvVar(key) to hide only secure values, which is the right shape for the suggested design.
• I did not run a full local build in this workspace because earlier verification was blocked by missing installed dependencies (vite was not available locally). The duplicate functions above are visible directly in source and do not require build execution to confirm.

--
Yours,
CodeNomadBot

[shantur]

@CodeNomadBot

Do your thing

[codenomadbot]

Gatekeeper review for the latest PR state.

What I understood: another fresh gatekeeper pass on the current revision, with focus on whether the previous blockers were resolved and whether the implementation is now merge-ready.

Current status:

• The duplicate-function blocker in preferences.tsx is fixed.
• The per-variable secure design is still in place and matches the earlier suggestion at a high level.
• I found one remaining behavioral gap.

Findings:

1. Medium: newly entered environment variable values are still shown in plain text before the variable is added and marked secure. In packages/ui/src/components/environment-variables-editor.tsx:137-144, the add-new-variable value field is still type="text", while existing secure entries switch to type="password" at :89-96. That means the editor still exposes a value during creation, which does not fully satisfy the PR’s original security goal of masking value inputs in the settings editor.

Why this matters:

• For the exact case this PR is trying to protect against (API keys, tokens, secrets), the most sensitive moment is often initial entry.
• With the current flow, users must first type the secret in clear text, add it, and only then can mark it secure.

Everything else I previously flagged appears resolved in this revision.

Non-blocking suggestion:

• If the team wants to keep the current secureEnvVars model, the smallest follow-up would be to mask the new-value field by default, or provide a secure toggle in the add row before the variable is created.

--
Yours,
CodeNomadBot

[shantur]

@bluelovers - Are you planning to work on this, please let me know

[bluelovers]

@bluelovers - Are you planning to work on this, please let me know

my agens is very dead now