Skip to content

[5/7] feat: constrain network access and approve temporary secrets - #135

Closed
BIackFIame wants to merge 58 commits into
howdeploy:mainfrom
BIackFIame:backlog/4-protection
Closed

BIackFIame wants to merge 58 commits into
howdeploy:mainfrom
BIackFIame:backlog/4-protection

Conversation

@BIackFIame

@BIackFIame BIackFIame commented Oct 7, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Part 5/7 of the CanvasTTY agent-workflow series. Restricted agents can use an effective network policy and request named secrets with bounded human approval, expiry and revocation. Isolation hardening and authentication cancellation/status handling accompany these controls; unsupported enforcement remains visible.

Review scope and dependencies

Depends on: #134.

This is a cumulative upstream PR: GitHub's Files changed tab includes earlier unmerged parts because all PRs target upstream main; cross-fork stacked bases are unsupported. Review this part in the isolated predecessor-to-head diff. The intended merge order follows the numbered series; do not merge a later part independently of its predecessors.

No additional plugin is required for the core protection controls.

Validation and current blockers

Tested feature head: 9eb11095d6900c999a09b7d8e2da8ace953643e8. Local helpers, typecheck, build and secret audit passed; full suite: 2,016 passed, 10 skipped, zero failures. Historical fork CI passed on macOS, Linux and Windows using the exact candidate tree under a temporary workflow overlay. That overlay is not included here.

Draft: merge preview against updated upstream 6faadcdc found conflicts in src/preload/index.ts, src/renderer/src/features/workspace/WorkspaceCanvas.tsx, src/shared/contracts.ts. These historical results do not validate a conflict-resolved integration with current main. Preserve the newly accepted material/terminal-link behavior, resolve the overlaps, and rerun checks before marking ready.

Historical macOS fixtures used fake credentials and exercised approval/revocation, masking, allowed model connectivity and denied unrelated access. Passing cross-platform CI does not prove live enforcement on Linux or Windows; Windows restrictions remain unenforced where unsupported. Revocation cannot erase a value already delivered to a running command.

Series order

  1. macOS clipboard
  2. Orchestration, reviews and budgets — Draft
  3. Worktrees and checkpoints — Draft
  4. Timeline, usage and attention — Draft
  5. Network policies and temporary secrets — Draft
  6. Workspace context and navigation — Draft
  7. Experimental routing, handoff and remote controls — Draft

Parts 2–7 are cumulative Draft PRs against upstream main, with an isolated layer diff in each description. Their tested feature heads currently conflict with newer upstream changes; integration and fresh checks are required before readiness. Part 7 also retains its independent experimental acceptance gate.

@BIackFIame

Copy link
Copy Markdown
Contributor Author

@codex review

Please review this PR for correctness, regressions and security issues. Review only; post findings on GitHub without changing code.

This is part 5/7 of a cumulative series. Focus on changes introduced after predecessor 21d5c99f7957ec98ea5aa893127ec6360b6084fe through current head 9eb11095d6900c999a09b7d8e2da8ace953643e8, including their interactions with the inherited code. The isolated layer diff is linked in the PR description. Known merge conflicts with newer main are already documented; look for substantive code defects in addition to those integration blockers.

@howdeploy

Copy link
Copy Markdown
Owner

@BIackFIame, the maintainer has now inspected #135 in the combined Linux DEV build and rejected its current UI and product scope. We are closing this PR without merging it. The concerns are consistent with the feedback on #132, #134, #137, and the redesign requested for #136.

The navigation adds an unnecessary administration layer

To reach an agent-related security control, the user currently opens the card options, chooses Activity and task, then switches to Safety & secrets inside a multi-tab inspector. That route does not communicate the purpose of the operation. Configuring access should not require first entering an activity/task administration window and navigating around timelines, reports, usage, tasks, budgets and notifications.

This is visible in the implementation: TerminalCard.tsx routes the action through onOpenInspector, and BacklogSessionInspector.tsx places both SecretGrantsPanel and NetworkPolicyPanel in the same secrets tab. The number of controls is not the only issue. Their ownership and entry point are wrong for the simple canvas interaction we want.

Different scopes and workflows are mixed in one menu

The network policy is stored for a project, yet it is edited from an individual agent card. registerNetworkPolicyIpc.ts resolves that card through the task root to a project working directory before saving the policy. A change made from one card can therefore affect later launches of other agents in that project. The current placement makes it too easy to read this as a setting for only the card the user opened.

Temporary secret approval has a different scope: it authorizes a named key for a particular session, with a time-, turn-, or session-based lifetime. Reviewing a pending request, configuring project network access, maintaining provider keys and selecting an API profile are different operations. Packing their controls into one inspector creates a large set of fields and choices instead of a clear decision at the point where it is needed.

The network panel also requires saving and restarting agents before its policy applies to their processes. That is a material distinction between saved configuration and active protection. Any future design must clearly identify the affected project and sessions, show what is currently enforced, and make the restart requirement understandable without requiring users to learn the internal harness model.

The interface violates the existing design contract

The inherited inspector uses its own dark background, white text, borders, fixed pixel dimensions and separately styled controls instead of consistently using CanvasTTY's palette roles, UI scale and shared components. NetworkPolicyPanel.tsx hard-codes English headings, options, messages and explanatory text, even when the rest of the application is in Russian. Its native select, checkboxes and textarea participate in this separate form layout rather than the established settings and permission interaction patterns.

Please follow the UI contract: existing components, theme tokens, typography, spacing, official icons, localization and keyboard/focus behavior. Recoloring this panel or moving the same form behind another menu would not solve the navigation and complexity problems.

Direction for any future proposal

Keep project/agent configuration in the appropriate existing settings flow with an explicit scope. Present a temporary permission request as a focused approval decision for the requesting agent, rather than making the user browse an administration dashboard to find it. More elaborate harness-policy management should be an optional plugin workflow; if it needs host support, propose a small, separately reviewable core API extension.

This product rejection does not establish that the security implementation is broken, and it does not justify weakening host-side enforcement. Independently useful enforcement fixes can be proposed separately, preserving validation, secret handling and refusal when a requested restriction cannot be enforced.

The series is cumulative. Remaining proposals must exclude the rejected #132, #134, #135 and #137 functionality instead of bringing it back through another PR. The separately accepted #133 functionality remains accepted in scope; closing #135 does not reverse that decision.

This PR is rejected in its current form and will be closed. A future contribution needs a simpler, correctly scoped interaction and a design that follows the existing application.

@howdeploy howdeploy closed this Oct 12, 2026
howdeploy added a commit that referenced this pull request Oct 12, 2026
This integrates the six contributions accepted during the maintainer's DEV review, together with the maintainer's local UI and minimap fixes. Rejected orchestration features, including #133, are excluded entirely.

## Included contributions and credit

- #125 by @tab11pm: read-only file cards, directory browsing, and richer Markdown/code previews.
- #130 by @SaneSanders: image/material remarks and handoff to an existing agent session.
- #131 by @BIackFIame: macOS clipboard/edit context menus for text fields.
- #138 by @pepechka-kun: usage history, measured quota changes, local-log evidence, and explicit attribution uncertainty.
- #139 by @CLOSETTTY: interactive mascots and the Codex mascot creation workflow.
- #140 by @teo-nex: the macOS node-pty file-descriptor leak fix, patch delivery, and resource diagnostics.

Thank you to all six contributors. Each original PR head and its author commits are retained in Git history through separate merge commits. This integration must use a merge commit rather than squash or rebase.

## Maintainer patches

- Fit the minimap overview to the camera viewport and all canvas objects together, with padding; retain stable drag coordinates during a gesture.
- Keep the material handoff icon fixed in the header, remove the count beneath it, and place equally sized Cancel/Send actions at the bottom right of the handoff dialog.
- Split usage history into five left-side sections: conditional estimate, quotas, agents, local-log tokens, and source state. Use pages within sections to fit the window without a long scrolling report.
- Resolve shared App/IPC/preload integrations while retaining the file, handoff, and usage-history flows.
- Retain the approved DEV dependency set and Electron 43.7.7; reuse the matching lockfile with patch-package 8.0.1.

## Scope

#132, #133, #134, #135, and #137 are not included. #136 is also excluded pending the previously requested UI/UX redesign. No commits from that cumulative orchestration series are ancestors of this branch.

Material handoff in #130 does not automatically apply a saved JSON agent report to the material state; this remains the reviewed behavior.

## Validation

- `VITE_BUILD_CHANNEL=DEV npm run build` passed, including the script's TypeScript checks and native-helper build.
- `git diff --check` passed.
- Verified that all six accepted PR heads are ancestors of the integration and all six excluded PR heads are not.
- Verified that the three approved local UI/minimap implementations match the selected DEV copy.
- Added minimap geometry coverage for distant camera positions and adapted usage-history fixtures for section navigation.
- The maintainer manually reviewed the accepted features in the earlier combined DEV build. The final integration without #133 has not received another manual UI pass. Automated tests and macOS/Windows runtime checks were not run locally for this integration.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants