You are viewing a plain text version of this content. The canonical link for it is here.

Posted to commits@maka.apache.org by "Sun-GLiang (via GitHub)" <gi...@apache.org> on 2026/08/24 10:17:01 UTC

[PR] fix(desktop): prevent proxy password mask corruption [maka]

Sun-GLiang opened a new pull request, #3704:
URL: https://github.com/apache/maka/pull/3704

   ## Summary
   
   - keep saved proxy credentials out of persisted settings and Renderer input values
   - add an explicit keep/replace/delete credential contract with a shared Runtime Host operation lane
   - keep password editing local until blur or Enter, make Eye reveal the current draft, and remove Copy only from the proxy password field
   - preserve schema-v1 config import/export compatibility with preflight validation
   
   Fixes #3696
   
   ## Dependency
   
   - Built on `fix/proxy-auth-test-consistency`, the implementation branch for #3682.
   - Until #3682 lands, the compare view against `main` also contains dependency commit `ec3b38b3c`.
   - Rebase this branch onto the updated `main` after #3682 lands so the PR contains only `b94e9ab0d`.
   
   ## Verification
   
   - `npm test` — all workspace tests passed; Desktop 1393/1393
   - focused proxy password tests — 36/36
   - offline authenticated proxy E2E — 1/1
   - `npm run lint`
   - `npm run format:check`
   - `npm run build`
   - `npm run typecheck`
   - `npx knip --workspace apps/desktop`
   - `npx knip --workspace packages/ui`
   
   ## AI use
   
   Select exactly one:
   
   - [ ] No generative tool made a substantive contribution
   - [x] Generative tooling made a substantive contribution
   
   Tool(s) and scope: OpenAI Codex implemented the fix, tests, verification, and review-driven corrections.
   
   ## Checklist
   
   - [x] Tests cover the change and fail without it
   - [x] Lint, format, typecheck and the affected suites pass locally
   
   Does this PR entail a change in behavior?
   
   - [x] Yes — described under Summary above
   - [ ] No
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org

Re: [PR] fix(desktop): make proxy credential updates explicit and Host-owned [maka]

Posted by "me2seeks (via GitHub)" <gi...@apache.org>.

me2seeks merged PR #3704:
URL: https://github.com/apache/maka/pull/3704


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org

Re: [PR] fix(desktop): make proxy credential updates explicit and Host-owned [maka]

Posted by "Astro-Han (via GitHub)" <gi...@apache.org>.

Astro-Han commented on code in PR #3704:
URL: https://github.com/apache/maka/pull/3704#discussion_r3890169226


##########
apps/desktop/src/main/runtime-host-config-ipc-main.ts:
##########
@@ -150,11 +189,17 @@ export async function gatherRuntimeHostConfig(
   if (selected.has('connections') && catalog) {
     data.connections = projectHostConnections(catalog);
   }
+  // Schema v1 stores the network-proxy password and Tavily key in the
+  // settings payload. Keep a credentials-only request lossless by making the
+  // dependency explicit in the generated bundle.
   if (selected.has('settings')) {
     const settings = await deps.getSettings();
     data.settings = selected.has('credentials')
       ? restoreHostSettingsSecrets(settings, secrets)
       : stripSettingsSecretsForExport(settings);
+  } else if (selected.has('credentials')) {
+    const settingsSecrets = projectHostSettingsSecrets(secrets);

Review Comment:
   [P2] (category ① — normal credentials restore path)
   
   Thanks for keeping credentials-only export lossless. A proxy password by itself currently produces `{network.proxy.password}` without `authEnabled`; on a fresh/default target the imported patch is merged with `authEnabled: false`, then adapted to `credential: replace`, which the policy/credential operation rejects. That makes a normal credentials-only round trip fail and also prevents later Tavily/model credentials in the bundle from being applied. Could the bundle carry the minimum proxy-auth policy needed for this secret, or skip/report the incompatible secret without aborting unrelated credentials, with an export→default-target import regression? Please push back if credentials-only restore intentionally requires preconfiguring identical proxy policy on the target.



##########
apps/desktop/src/main/config-transfer-service.ts:
##########
@@ -91,13 +91,25 @@ export async function applyConfigImport(
         ? reconcileConnectionAfterEnabledModelsChange(connection, connection.enabledModelIds)
         : null;
       await deps.connectionStore.save(selection ? { ...connection, ...selection } : connection);
-      appliedConnectionSlugs.add(connection.slug);
+      credentialTargetSlugs.add(connection.slug);
     }
     result.connections = {
       created: plan.create.length,
       overwritten: plan.overwrite.length,
       skipped: plan.skipped.length,
     };
+  } else if (
+    !bundle.includedData.includes('connections') &&
+    Array.isArray(bundle.data.credentials)
+  ) {
+    // Without a connection snapshot, the credential slug names an existing
+    // logical connection directly. A bundle that does include connections
+    // still uses the create/overwrite set above so an explicit skip cannot
+    // overwrite the target's credential.
+    const existing = await deps.connectionStore.list();

Review Comment:
   [P1] (category ① — normal credentials restore path)
   
   Thanks for restoring credentials-only bundles against existing connections. The source entry still carries only `{slug, kind, value}`, while this branch treats every target connection with the same slug as eligible. A normal restore between profiles can therefore take an API key/request-header secret for source slug `team` and write it into a target `team` whose provider or custom `baseUrl` is different; the next model request sends that secret to the target endpoint. This is the remaining boundary from the earlier connection-identity comment: slug plus credential kind is not an endpoint identity. Could the exported entry include a minimal verifiable binding (at least provider type plus canonical effective endpoint/fingerprint), skip/report mismatches, and cover same-slug/different-provider and same-provider/different-baseUrl restores? Please feel free to push back if another authenticated binding is preserved outside this payload.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org