mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-30 05:16:50 +01:00
fix(ui): show environment variables on agent Secrets pages (#18121)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Operators inspect agent configuration to understand which credentials an agent can use. > - Agent environment bindings can contain inline values, company secret references, and user secret references. > - The Secrets page did not mount the environment editor. Inline credentials were absent, and its empty message said no secrets were bound. > - The existing environment editor already handles these binding types and redacted values. > - This PR mounts that editor on the Secrets page in both UI layouts and clarifies the empty reference message. > - Operators can inspect and edit the configured bindings. Values remain masked, and the Runtime editor remains available. ## Linked Issues or Issue Description **What happened?** An agent can receive an inline credential through its environment while its Secrets page shows no matching entry. That page only rendered the secret-reference overview and API access controls. **Expected behavior** The agent Secrets page shows existing environment bindings as well as secret references. Saved inline values stay redacted. Saving another binding preserves existing credentials. **Steps to reproduce** 1. Save an agent with an inline environment credential. 2. Open the agent Secrets page. 3. The credential row is missing, although the runtime configuration contains it. **Paperclip version or commit** Reproduced on master `27270e9bf8b7a294cf03f8b84f4857f1b1ff05c7` before the fix. Related: [#12517](https://github.com/paperclipai/paperclip/pull/12517) proposes changes to secret assignment controls. This PR covers display of existing inline environment bindings through the current editor. ## What Changed - Show the environment editor on agent Secrets pages in both UI layouts. Keep the Runtime editor available. - Change the empty overview message to describe missing company secret references. Personal references remain visible in the environment editor. - Add native and legacy agent regressions through the actual page components. - Add a browser check for masked values, secret references, save/reload, and exact credential preservation through an opaque configuration revision. - Update the feature and agent access maps for the existing UI entry point. ## Verification - Red before the fix: the native and legacy page regressions could not find the saved environment credential row. - The focused UI suites pass: 207 tests, including all four layout/adapter page cases. - The server redaction and configuration revision suites pass: 42 tests. Native Runner environment assembly suites pass: 40 tests. - An isolated Chromium flow passes against the real local API and database. It checks a masked inline value, a company secret reference, and save/reload of an added variable. It then removes the new variable and checks that the opaque server revision matches its original value. This proves that the hidden credentials survive both saves without reading them. The initial browser attempt exposed a fixture ordering assumption; the test now compares variable-name sets because PostgreSQL JSONB does not retain insertion order. - `pnpm -r typecheck`, `pnpm build`, `pnpm check:token-gates`, and `node scripts/check-maps.mjs` pass. - The full local `pnpm test:run` attempt found three failures in unchanged server fixtures. The chat-recovery case passes in isolation and in its 22-test group. The two workspace cases inherit `init.defaultBranch=main`; all 179 workspace tests pass when the child process uses `init.defaultBranch=master`, as the fixtures expect. The long local invocation was then stopped. Later local groups were not reached. All corresponding CI test shards pass. - [CI passes on the final commit](https://github.com/paperclipai/paperclip/actions/runs/40809064907). The first canary dry run found an npm version collision. Its single retry passed without a source change. - Apex passes at 5/5 on the current commit with no unresolved findings. ## Risks Low risk. The change reuses the existing editor and save path. No API, secret delivery, permission, or database contract changes. The tests cover masked values and existing references during save/reload. ## Model Used OpenAI GPT-6 through Codex, with tool use and code execution. The session does not expose a more specific model ID or context-window size. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass — scoped checks pass; the full local command limits are recorded above - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip <noreply@paperclip.ing>
This commit is contained in:
1 parent
b06ea111a2
commit
fd98075a22
7 files changed
+98
-6
No files matched your search
@@ -4,6 +4,11 @@ Product map: [Secrets, personal credentials, and proposals](../feature-map/secre
|
||||
|
||||
Feature ID: `secrets`.
|
||||
|
||||
The agent **Secrets** page shows inline environment variables alongside company
|
||||
and user secret references. Inline values stay redacted on configuration reads.
|
||||
This presentation uses the existing agent configuration APIs and permissions;
|
||||
it does not grant another agent access to those values.
|
||||
|
||||
Source revision: `3bbc2104b65caf9f7763938a01260a5673e12f7a`.
|
||||
|
||||
Review depth and executed proof are separate. The operation records enumerate the named behavior and access boundaries. Their evidence states describe the reviewed sources; run results and environment limits belong in the PR or verification artifacts. A listed API or source test does not establish a live authorized journey. Follow the [map contract](../README.md).
|
||||
|
||||
@@ -26,6 +26,12 @@ Agent access: [companion](../agent-access-map/secrets.md).
|
||||
|
||||
Open company Settings → Secrets and the company/personal secret views.
|
||||
|
||||
An agent's **Secrets** page also shows its environment variables, including
|
||||
masked inline values, company secret references, and user secret references.
|
||||
Environment bindings remain available from **Harness / Runtime** as well.
|
||||
Save an added variable and reload to confirm that the existing masked bindings
|
||||
and their secret references remain present.
|
||||
|
||||
### `proposal-import`
|
||||
|
||||
Use the Proposals tab or Import from vault where available.
|
||||
|
||||
@@ -78,6 +78,53 @@ async function save(page: Page) {
|
||||
await expect(page.getByRole("alert")).toHaveCount(0);
|
||||
}
|
||||
|
||||
test("Secrets shows masked inline variables and preserves them when another variable is saved", async ({ page, request }) => {
|
||||
const { company, agent, url } = await setup(request);
|
||||
const secret = await json(await request.post(`/api/companies/${company.id}/secrets`, { data: {
|
||||
name: "Release credential", value: "disposable-reference-credential",
|
||||
} }));
|
||||
await json(await request.patch(`/api/agents/${agent.id}`, { data: { adapterConfig: {
|
||||
command: "true",
|
||||
env: {
|
||||
RELEASE_WEBHOOK: { type: "plain", value: "disposable-inline-credential" },
|
||||
SHARED_TOKEN: { type: "secret_ref", secretId: secret.id, version: "latest" },
|
||||
},
|
||||
} } }));
|
||||
|
||||
await page.goto(`${url}/secrets`);
|
||||
const variableNames = () => page.getByRole("textbox", { name: "Variable name", exact: true })
|
||||
.evaluateAll(elements => elements.map(element => (element as HTMLInputElement).value).sort());
|
||||
await expect(page.getByRole("heading", { name: "Environment variables", exact: true })).toBeVisible();
|
||||
await expect.poll(variableNames).toEqual(["RELEASE_WEBHOOK", "SHARED_TOKEN"]);
|
||||
await expect(page.getByLabel("Variable value", { exact: true })).toHaveValue("***REDACTED***");
|
||||
await expect(page.getByText("Release credential", { exact: true }).first()).toBeVisible();
|
||||
expect(await page.locator("body").textContent()).not.toContain("disposable-inline-credential");
|
||||
const before = await json(await request.get(`/api/agents/${agent.id}/configuration`));
|
||||
expect(before.configurationRevision).toMatch(/^[a-f0-9]{64}$/);
|
||||
|
||||
await page.getByRole("button", { name: "Add variable", exact: true }).click();
|
||||
await page.getByRole("textbox", { name: "Variable name", exact: true }).last().fill("RELEASE_CHANNEL");
|
||||
await page.getByLabel("Variable value", { exact: true }).last().fill("drafts");
|
||||
await page.getByRole("button", { name: "Save changes", exact: true }).click();
|
||||
await expect(page.getByRole("button", { name: "Save changes", exact: true })).toBeDisabled();
|
||||
const config = await json(await request.get(`/api/agents/${agent.id}/configuration`));
|
||||
expect(config.adapterConfig.env).toMatchObject({
|
||||
RELEASE_WEBHOOK: { type: "plain", value: "***REDACTED***" },
|
||||
SHARED_TOKEN: { type: "secret_ref", secretId: secret.id, version: "latest" },
|
||||
RELEASE_CHANNEL: { type: "plain", value: "***REDACTED***" },
|
||||
});
|
||||
await page.reload();
|
||||
await expect.poll(variableNames).toEqual(["RELEASE_CHANNEL", "RELEASE_WEBHOOK", "SHARED_TOKEN"]);
|
||||
await expect(page.getByLabel("Variable value", { exact: true }).first()).toHaveValue("***REDACTED***");
|
||||
await page.getByRole("button", { name: "Remove RELEASE_CHANNEL", exact: true }).click();
|
||||
await page.getByRole("button", { name: "Save changes", exact: true }).click();
|
||||
await expect(page.getByRole("button", { name: "Save changes", exact: true })).toBeDisabled();
|
||||
const after = await json(await request.get(`/api/agents/${agent.id}/configuration`));
|
||||
// The server's opaque revision includes the actual hidden values. Returning
|
||||
// to the same revision proves they survived both saves without reading them.
|
||||
expect(after.configurationRevision).toBe(before.configurationRevision);
|
||||
});
|
||||
|
||||
test("Advanced YAML persists settings and commands through their real resource APIs", async ({ page, request, toolProvider }, info) => {
|
||||
test.setTimeout(120_000);
|
||||
const { company, agent, url } = await setup(request);
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
// @vitest-environment jsdom
|
||||
|
||||
import { useState } from "react";
|
||||
import { MemoryRouter } from "react-router-dom";
|
||||
import { createRoot, type Root } from "react-dom/client";
|
||||
import { flushSync } from "react-dom";
|
||||
import { QueryClient, QueryClientProvider } from "@tanstack/react-query";
|
||||
@@ -20,6 +21,8 @@ import { CodexLocalConfigFields } from "../adapters/codex-local/config-fields";
|
||||
import type { AdapterConfigFieldsProps } from "../adapters/types";
|
||||
import { buildClaudeLocalConfig } from "@paperclipai/adapter-claude-local/ui";
|
||||
import { DEFAULT_CODEX_LOCAL_MODEL } from "@paperclipai/adapter-codex-local";
|
||||
import { ConfigurationTab } from "../pages/AgentDetail";
|
||||
import { ConfigurationTab as ClassicConfigurationTab } from "../pages/AgentDetail.production";
|
||||
|
||||
const mockAgentsApi = vi.hoisted(() => ({
|
||||
get: vi.fn(),
|
||||
@@ -103,9 +106,9 @@ vi.mock("../lib/clipboard", () => ({
|
||||
|
||||
vi.mock("../context/CompanyContext", () => ({
|
||||
useCompany: () => ({
|
||||
companies: [{ id: "company-1", name: "Paperclip" }],
|
||||
companies: [{ id: "company-1", name: "Paperclip", issuePrefix: "CO" }],
|
||||
selectedCompanyId: "company-1",
|
||||
selectedCompany: { id: "company-1", name: "Paperclip" },
|
||||
selectedCompany: { id: "company-1", name: "Paperclip", issuePrefix: "CO" },
|
||||
selectionSource: "bootstrap",
|
||||
loading: false,
|
||||
error: null,
|
||||
@@ -1480,10 +1483,40 @@ describe("AgentConfigForm environment selector", () => {
|
||||
roots.push(result.root);
|
||||
|
||||
expect(result.container.textContent).toContain("Secret access");
|
||||
expect(result.container.textContent).toContain("No secrets are bound to this agent yet.");
|
||||
expect(result.container.textContent).toContain("No company secret references are bound to this agent yet.");
|
||||
expect(result.container.textContent).not.toContain("Environment variables");
|
||||
});
|
||||
|
||||
it.each([
|
||||
{ variant: "current", adapterType: "paperclip_runner" },
|
||||
{ variant: "current", adapterType: "claude_local" },
|
||||
{ variant: "classic", adapterType: "paperclip_runner" },
|
||||
{ variant: "classic", adapterType: "claude_local" },
|
||||
])("shows saved $adapterType environment credentials on the $variant agent Secrets page", async ({ variant, adapterType }) => {
|
||||
mockEnvironmentsApi.list.mockResolvedValue([]);
|
||||
const container = document.createElement("div");
|
||||
document.body.appendChild(container);
|
||||
const root = createRoot(container);
|
||||
roots.push(root);
|
||||
const agent = { ...makeAgent({ adapterType, adapterConfig: {
|
||||
provider: "acpx", acpxAgent: "claude",
|
||||
env: { RELEASE_WEBHOOK: { type: "plain", value: "***REDACTED***" } },
|
||||
} }), chainOfCommand: [], access: { canAssignTasks: false, taskAssignSource: "none" as const, membership: null, grants: [] } };
|
||||
const Settings = variant === "current" ? ConfigurationTab : ClassicConfigurationTab;
|
||||
await act(async () => renderFormView(root, createFormQueryClient(), <MemoryRouter>
|
||||
<Settings agent={agent} companyId="company-1" content="secrets"
|
||||
onDirtyChange={vi.fn()} onSaveActionChange={vi.fn()} onCancelActionChange={vi.fn()} onSavingChange={vi.fn()}
|
||||
updatePermissions={{ mutate: vi.fn(), isPending: false }} />
|
||||
</MemoryRouter>));
|
||||
await flushReact();
|
||||
|
||||
expect(container.querySelector<HTMLInputElement>('input[aria-label="Variable name"]')?.value).toBe("RELEASE_WEBHOOK");
|
||||
expect(container.querySelector<HTMLInputElement>('input[aria-label="Variable value"]')?.value).toBe("***REDACTED***");
|
||||
expect(container.textContent).toContain("Environment variables");
|
||||
expect(container.textContent).toContain("Secret access");
|
||||
expect(container.textContent).not.toContain("No environment variables");
|
||||
});
|
||||
|
||||
it("shows concise Environment copy when one runnable non-local environment exists", async () => {
|
||||
const result = await renderForm(e2bEnvironments());
|
||||
roots.push(result.root);
|
||||
|
||||
@@ -285,7 +285,7 @@ export function AgentSecretAccessEditor({
|
||||
))}
|
||||
</div>
|
||||
) : (
|
||||
<p className="text-sm text-muted-foreground">No secrets are bound to this agent yet.</p>
|
||||
<p className="text-sm text-muted-foreground">No company secret references are bound to this agent yet.</p>
|
||||
)}
|
||||
|
||||
{/* Pending binding proposals targeting this agent (PAP-14731). */}
|
||||
|
||||
@@ -2083,7 +2083,7 @@ function AgentConfigurePage({
|
||||
|
||||
/* ---- Configuration Tab ---- */
|
||||
|
||||
function ConfigurationTab({
|
||||
export function ConfigurationTab({
|
||||
agent,
|
||||
companyId,
|
||||
onDirtyChange,
|
||||
@@ -2209,6 +2209,7 @@ function ConfigurationTab({
|
||||
hideInstructionsFile={hideInstructionsFile}
|
||||
content={content}
|
||||
sectionLayout="cards"
|
||||
environmentVariablesPlacement={content === "secrets" ? "secrets" : "configuration"}
|
||||
/>
|
||||
{content === "configuration" ? (
|
||||
<p className="text-xs text-muted-foreground">
|
||||
|
||||
@@ -2135,7 +2135,7 @@ export function ConfigurationTab({
|
||||
hideInstructionsFile={hideInstructionsFile}
|
||||
content={content === "runtime" ? "configuration" : "secrets"}
|
||||
sectionLayout="cards"
|
||||
environmentVariablesPlacement="configuration"
|
||||
environmentVariablesPlacement={content === "secrets" ? "secrets" : "configuration"}
|
||||
compactTestFeedback
|
||||
sectionOrder={["identity", "adapter", "configuration", "environment", "environment-variables", "run-policy"]}
|
||||
sectionTitles={{ adapter: "Adapter", configuration: "Configuration", identity: "Agent identity" }}
|
||||
|
||||
Reference in new issue
Block a user