2 Commits

Include renames

Author	SHA1	Message	Date
4 people	89ce36d7af	feat(skills): open-by-default company skill policy and core UX (#9564) ... ## Thinking Path > - Paperclip uses company skills to make agent capabilities reusable across an organization. > - Skill operations currently mix capability availability with permission checks, which creates avoidable setup friction and inconsistent denial handling. > - The policy contract needs to remain open by default while allowing company-scoped restrictions for governed deployments. > - Core owns the canonical policy actions, persistence, evaluation, API behavior, safe import boundaries, and generic denial/read-only UI. > - Enterprise policy-editor implementation belongs in the separate `paperclip-ee` repository and is intentionally excluded from this PR. ### Problem or motivation Company skill operations can encounter permission dead ends even when no explicit restriction has been configured, and import-source classification can drift between policy evaluation and execution. ### Proposed solution Define eight canonical skill policy actions, default all actions to allowed, persist company-scoped restrictions, expose policy evaluation APIs, normalize import sources at the boundary, and update Skill Studio to present actionable restriction states without embedding Enterprise Edition implementation in the core repository. ### Alternatives considered Keeping capability checks distributed across routes and UI surfaces was rejected because it duplicates policy logic and makes denial behavior inconsistent. Shipping the Enterprise policy editor in this repository was rejected because `paperclip-ee` is a separate repository and must receive its own PR. ### Roadmap alignment Extends the completed **Skills Manager** roadmap area by adding coherent governance and removing workflow dead ends. ### Additional context The core API contract remains suitable for a separate Enterprise Edition editor, but this PR contains no `paperclip-ee` package or EE-specific UI integration code. ## What Changed - Added the company skill policy contract to product and implementation documentation, including the open-by-default rule, eight canonical actions, decision shape, and core/EE ownership boundary. - Added the company-scoped policy schema, migration `0170`, shared validators, policy service, REST routes, OpenAPI coverage, and focused server tests. - Hardened import policy enforcement by normalizing import sources and keeping source classification consistent between policy evaluation and execution. - Updated core Skill Studio behavior to remove generic permission dead ends and show actionable policy/platform denial states only when an operation is actually denied. - Removed the `plugin-paperclip-ee` package, Docker wiring, EE discovery/deep-link helpers, and EE-specific UI tests/stories from this PR so that implementation can be submitted separately to the EE repository. - Preserved open-by-default behavior when no explicit company restriction exists. ## Verification - `pnpm --filter @paperclipai/ui exec vitest run src/components/skill-studio/SkillPolicySurfaces.test.tsx src/lib/skill-policy-denial.test.ts` — 20/20 passed. - `pnpm --filter @paperclipai/ui exec tsc --noEmit` — passed. - `pnpm --filter @paperclipai/server exec vitest run src/__tests__/worktree-config.test.ts` — 12/12 passed. - `pnpm check:token-gates` — passed with all gates clean. - `git diff --check` — passed. - `git diff --name-only origin/master | rg 'paperclip-ee|ee-skill-policy'` — no matches. ## Risks - Migration `0170` introduces company policy persistence; rollout depends on the migration applying before policy routes are exercised. - Open-by-default is an intentional behavioral policy: deployments expecting implicit denials must configure explicit restrictions. - Import normalization is security-sensitive and should retain focused review. - The separate EE editor must stay contract-compatible with the core policy API as policy actions evolve. ## Model Used - OpenAI Codex CLI, runtime model identifier and context-window size not exposed by this execution environment; reasoning, repository tool use, shell execution, and code review capabilities enabled. ## 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 available to this runtime) - [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 or described the result 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 and contains no internal Paperclip ticket id or instance-derived details - [x] I have run focused tests locally and they pass - [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 on the latest head - [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> Co-authored-by: Claude Fable 5 <noreply@anthropic.com> Co-authored-by: Evyatar Bluzer <bluzername@users.noreply.github.com>	2026-07-15 11:42:40 -05:00
3 people	bb7978327e	fix(logger): redact passwords and tokens from HTTP error log lines (#8013) ... Resubmits #5820 by @echokos. The original PR's head fork could not accept maintainer edits (organization-owned fork without cross-org maintainer-edit access), so we've resubmitted the commits here with original authorship preserved. Thanks to @echokos for the contribution. --- ## Thinking Path > - Paperclip orchestrates AI agents for zero-human companies; the server is an Express app with structured HTTP logging via pino-http. > - The middleware in `server/src/middleware/logger.ts` defines a `customProps` hook that attaches request context (`req.body` / `req.params` / `req.query`) to every 4xx/5xx log entry so operators can diagnose failed requests. > - That hook copies the body verbatim. Better Auth's `POST /api/auth/sign-in/email` carries an `{ email, password }` body — on a wrong-password attempt the request lands in the 4xx branch and the plaintext password is written to `~/.paperclip/logs/server.log`. > - Two existing issues raise this (#3072 plaintext-password leak, #4759 similar concerns) and neither has a fix. > - Same exposure surface applies to sign-up, reset-password, API key creation, and any endpoint that accepts a credential in the body and can return 4xx. > - This pull request introduces a small `redactSensitive` walker that returns a shallow copy of the input with values for known credential-shaped keys replaced with `[REDACTED]`, and applies it at every body/params/query log site in `customProps`. > - The benefit is that operators can keep diagnostic logging on without their disk silently accumulating user passwords and bearer tokens. ## What Changed - `server/src/middleware/redact-sensitive.ts` (new): depth-capped, case-insensitive walker. Sensitive keys covered: `password`, `currentPassword`, `newPassword`, `passwordConfirmation`, `passwordConfirm`, `confirmPassword` (+ snake_case variants), `secret`, `client_secret`, `access_token`, `refresh_token`, `id_token`, `auth_token`, `session_token`, `api_key`, `authorization`, `private_key`. Bare `token` deliberately not in the list — pagination cursors and CSRF tokens are not credentials (per Greptile review). - `server/src/middleware/logger.ts`: wraps the six log sites in `customProps` (3 ctx-path + 3 fallback-path) with `redactSensitive`. - `server/src/__tests__/redact-sensitive.test.ts` (new): covers plaintext password, case-insensitive matching, multiple credential keys, nested objects/arrays, bare `token` left untouched, primitives untouched, cycle safety. - Depth-cap returns `undefined` (field absent from log line) rather than a sentinel string, per Greptile review. ## Verification - `pnpm --filter @paperclipai/server test redact-sensitive` should run the new test file green. - Manual: tail `~/.paperclip/logs/server.log`, hit `POST /api/auth/sign-in/email` with a deliberately wrong password, confirm the logged `reqBody.password` reads `[REDACTED]` (not the plaintext) and the surrounding fields still appear for diagnosis. ## Risks Low. The walker only rewrites values at the log-emit boundary — the actual `req.body` object handed to downstream handlers is unchanged because `redactSensitive` returns a new object. Standard log fields (email, route path, status code) remain visible. The sensitive-key list is conservative enough that the only risk is over-redacting a non-credential field that happens to share a name with a known credential; the bare `token` carve-out in this revision addresses the most obvious such case. ## Model Used - Claude Opus 4.7 (`claude-opus-4-7`), Anthropic, extended thinking mode, working through Claude Code CLI. ## 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 - [ ] I have run tests locally and they pass (no `node_modules` in my disposable PR-prep checkout; CI vitest will exercise) - [x] I have added or updated tests where applicable - [ ] If this change affects the UI, I have included before/after screenshots (N/A — server-only) - [ ] I have updated relevant documentation to reflect my changes (no doc surface affected) - [x] I have considered and documented any risks above - [x] I will address all Greptile and reviewer comments before requesting merge Closes #3072. Refs #4759. --- ## Original Context `customProps` in the HTTP logger copies `req.body` / `req.params` / `req.query` verbatim into 4xx/5xx log entries. Better Auth's wrong-password flow therefore writes: ``` {"reqBody":{"email":"…","password":"founding6gomez6croaking"},"msg":"POST /api/auth/sign-in/email 401"} ``` …to disk. This PR rewrites credential-shaped values to `[REDACTED]` at that boundary. --- ## Cross-references and status (maintainer) Closes #5820 Closes #3095 Closes #4760 Closes #4886 --------- Co-authored-by: Aurora <aurora@majorimpact.com> Co-authored-by: Paperclip <noreply@paperclip.ing>	2026-06-11 17:09:32 -07:00