From 66575fe519db7320147aece94fa66e15eba375c1 Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Mon, 10 Aug 2026 21:45:58 -0400 Subject: [PATCH] fix(paperclip-page): scope uploader credentials to the publish helper (#10894) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Agents publish static pages with the paperclip-page skill and its `publish.sh` helper > - The skill docs told operators to bind the page-uploader IAM keys as the global `AWS_ACCESS_KEY_ID` / `AWS_SECRET_ACCESS_KEY` > - Static env keys have precedence over `AWS_PROFILE` in the AWS CLI and in all AWS SDKs > - Because of this, each agent run lost the host role identity and lost access to Secrets Manager and other AWS services > - This pull request adds namespaced credential variables that apply only to the helper's own `aws` calls > - The benefit is a stable host AWS identity in agent runs, with no change to page publishing ## Linked Issues or Issue Description No public GitHub issue exists. Description of the problem: **What happened?** Agent runs on a host with `AWS_PROFILE` set lost access to AWS Secrets Manager. The failures looked intermittent. The cause is deterministic: the page-uploader keys were bound as global `AWS_ACCESS_KEY_ID` / `AWS_SECRET_ACCESS_KEY` in agent run environments. These static keys shadow `AWS_PROFILE`. Each process in the agent run then used the S3-upload-only uploader identity. **Expected behavior** The page-uploader credentials apply only to the page publish helper. All other processes keep the host identity from `AWS_PROFILE`. **Steps to reproduce** 1. Set `AWS_PROFILE` to a role with Secrets Manager access. 2. Export `AWS_ACCESS_KEY_ID` / `AWS_SECRET_ACCESS_KEY` for an IAM user without that access. 3. Run `aws sts get-caller-identity`. The identity is the IAM user, not the role. 4. Run `aws secretsmanager list-secrets`. The call fails with `AccessDeniedException`. ## What Changed - `publish.sh` reads `PAPERCLIP_PAGE_AWS_ACCESS_KEY_ID` and `PAPERCLIP_PAGE_AWS_SECRET_ACCESS_KEY`, with optional `PAPERCLIP_PAGE_AWS_SESSION_TOKEN`. - The helper applies these values only to its own `aws` invocations. It clears ambient `AWS_PROFILE` and `AWS_SESSION_TOKEN` for those calls. - Credential precedence is: namespaced key pair, then `PAPERCLIP_PAGE_AWS_PROFILE`, then the ambient credential chain. Existing global-name bindings continue to work during migration. - Validation: the key pair must be set together. The pair plus `PAPERCLIP_PAGE_AWS_PROFILE` is an error. A session token without the pair is an error. - `SKILL.md` and `README.md` now instruct operators to bind the secrets under the namespaced names and explain the shadowing hazard. ## Verification - Run `node --test .agents/skills/paperclip-page/scripts/publish.test.mjs`. All 11 tests pass. - New tests cover: the incomplete key pair, the pair-plus-profile conflict, the token-without-pair error, and a fake-`aws` environment capture that proves the helper's calls see the page keys while `AWS_PROFILE` and `AWS_SESSION_TOKEN` stay unset. - Run `bash -n .agents/skills/paperclip-page/scripts/publish.sh` for a syntax check. ## Risks - Low risk. The change is contained in one skill helper and its documents. - The ambient credential chain remains the fallback, so current deployments do not break before operators rebind the secrets. - Operators must rebind the two page secrets to the namespaced names to get the benefit. The README documents this. ## Model Used Claude Fable 5 (`claude-fable-5`), Anthropic. Context window: 1,000,000 tokens (128K max output). Agentic coding session with extended thinking and tool use (Claude Code harness). ## 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 - [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 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Fable 5 --- .agents/skills/paperclip-page/README.md | 36 ++- .agents/skills/paperclip-page/SKILL.md | 14 +- .../skills/paperclip-page/scripts/publish.sh | 52 ++++- .../paperclip-page/scripts/publish.test.mjs | 209 ++++++++++++++++++ 4 files changed, 300 insertions(+), 11 deletions(-) diff --git a/.agents/skills/paperclip-page/README.md b/.agents/skills/paperclip-page/README.md index 4ba859741b..cfe2d87882 100644 --- a/.agents/skills/paperclip-page/README.md +++ b/.agents/skills/paperclip-page/README.md @@ -73,8 +73,8 @@ Required for live publishes: export AWS_REGION=us-east-1 export PAPERCLIP_PAGE_BUCKET=paperclip-pages-prod export PAPERCLIP_PAGE_BASE_URL=https://pages.paperclip.ing -export AWS_ACCESS_KEY_ID=... -export AWS_SECRET_ACCESS_KEY=... +export PAPERCLIP_PAGE_AWS_ACCESS_KEY_ID=... +export PAPERCLIP_PAGE_AWS_SECRET_ACCESS_KEY=... ``` Optional: @@ -82,15 +82,35 @@ Optional: ```bash export PAPERCLIP_PAGE_DEFAULT_PREFIX="" export PAPERCLIP_PAGE_AWS_PROFILE=paperclip-page-uploader +export PAPERCLIP_PAGE_AWS_SESSION_TOKEN=... # only with the namespaced key pair ``` +Credential resolution order inside `publish.sh`: + +1. `PAPERCLIP_PAGE_AWS_ACCESS_KEY_ID` + `PAPERCLIP_PAGE_AWS_SECRET_ACCESS_KEY` + (scoped to the helper's `aws` calls; the surrounding process identity is + untouched) +2. `PAPERCLIP_PAGE_AWS_PROFILE`, passed to `aws` as `--profile` (ambient + `AWS_ACCESS_KEY_ID` / `AWS_SECRET_ACCESS_KEY` / `AWS_SESSION_TOKEN` / + `AWS_PROFILE` are stripped from the helper's `aws` calls so the named + profile always wins) +3. The ambient AWS credential chain + +Setting both the namespaced key pair and `PAPERCLIP_PAGE_AWS_PROFILE` is an +error. + Recommended Paperclip secret names: - `paperclip-page-aws-access-key-id` - `paperclip-page-aws-secret-access-key` -Bind those secrets into publisher agents as `AWS_ACCESS_KEY_ID` and -`AWS_SECRET_ACCESS_KEY`. Do not reuse Paperclip's internal S3 attachment/object +Bind those secrets into publisher agents as `PAPERCLIP_PAGE_AWS_ACCESS_KEY_ID` +and `PAPERCLIP_PAGE_AWS_SECRET_ACCESS_KEY`. Never bind them as the global +`AWS_ACCESS_KEY_ID` / `AWS_SECRET_ACCESS_KEY` names: static env keys override +`AWS_PROFILE` in the AWS CLI and every SDK, so global names silently switch the +whole agent run — and every subprocess — to the page-uploader identity and +break access to anything the uploader cannot reach (Secrets Manager, STS role +use, other buckets). Do not reuse Paperclip's internal S3 attachment/object storage credentials. ## AWS Setup @@ -484,16 +504,18 @@ pnpm paperclipai secrets create \ --value-env PAPERCLIP_PAGE_AWS_SECRET_ACCESS_KEY ``` -Bind runtime env to publishing agents: +Bind runtime env to publishing agents. Use the namespaced names — never the +global `AWS_ACCESS_KEY_ID` / `AWS_SECRET_ACCESS_KEY`, which would shadow the +host `AWS_PROFILE` identity for the entire agent run: ```json { - "AWS_ACCESS_KEY_ID": { + "PAPERCLIP_PAGE_AWS_ACCESS_KEY_ID": { "type": "secret_ref", "secretId": "", "version": "latest" }, - "AWS_SECRET_ACCESS_KEY": { + "PAPERCLIP_PAGE_AWS_SECRET_ACCESS_KEY": { "type": "secret_ref", "secretId": "", "version": "latest" diff --git a/.agents/skills/paperclip-page/SKILL.md b/.agents/skills/paperclip-page/SKILL.md index e9e74cfe08..df77444874 100644 --- a/.agents/skills/paperclip-page/SKILL.md +++ b/.agents/skills/paperclip-page/SKILL.md @@ -19,10 +19,20 @@ host, for example `https://pages.paperclip.ing//`. - `PAPERCLIP_PAGE_BUCKET` - `PAPERCLIP_PAGE_BASE_URL` - `AWS_REGION` - - AWS credentials via Paperclip Secrets or an approved AWS vault + - `PAPERCLIP_PAGE_AWS_ACCESS_KEY_ID` and `PAPERCLIP_PAGE_AWS_SECRET_ACCESS_KEY` + with the page-uploader credentials from Paperclip Secrets - Optional environment variables: - `PAPERCLIP_PAGE_DEFAULT_PREFIX` - - `PAPERCLIP_PAGE_AWS_PROFILE` + - `PAPERCLIP_PAGE_AWS_PROFILE` (alternative to the namespaced key pair) + - `PAPERCLIP_PAGE_AWS_SESSION_TOKEN` (only together with the namespaced key + pair) + +Do not bind the page-uploader credentials as global `AWS_ACCESS_KEY_ID` / +`AWS_SECRET_ACCESS_KEY`: static env keys take precedence over `AWS_PROFILE` in +every AWS SDK, so global names silently replace the host identity for every +process in the agent run. The namespaced variables scope the uploader identity +to this helper only. The ambient credential chain still works as a fallback +when none of the `PAPERCLIP_PAGE_AWS_*` credential variables are set. ## Workflow diff --git a/.agents/skills/paperclip-page/scripts/publish.sh b/.agents/skills/paperclip-page/scripts/publish.sh index 2aa3a356ad..086003a1e1 100755 --- a/.agents/skills/paperclip-page/scripts/publish.sh +++ b/.agents/skills/paperclip-page/scripts/publish.sh @@ -15,6 +15,15 @@ Required environment for live publish: Optional environment: PAPERCLIP_PAGE_DEFAULT_PREFIX, PAPERCLIP_PAGE_AWS_PROFILE + PAPERCLIP_PAGE_AWS_ACCESS_KEY_ID, PAPERCLIP_PAGE_AWS_SECRET_ACCESS_KEY, + PAPERCLIP_PAGE_AWS_SESSION_TOKEN + +Credential resolution for aws calls made by this helper: + 1. PAPERCLIP_PAGE_AWS_ACCESS_KEY_ID + PAPERCLIP_PAGE_AWS_SECRET_ACCESS_KEY + (used only by this helper; ambient AWS_PROFILE/AWS_* identity is untouched) + 2. PAPERCLIP_PAGE_AWS_PROFILE (passed as --profile; ambient AWS_* identity + variables are stripped from the helper's aws calls) + 3. Ambient AWS credential chain (env keys, profile, instance role) Options: --slug SLUG Lowercase URL slug. Allowed: a-z, 0-9, hyphen. @@ -125,9 +134,26 @@ join_prefix() { } aws_base_args=() +aws_env_unset=() +aws_env_overrides=() aws_cli() { - aws "${aws_base_args[@]}" "$@" + local name pair + if [[ ${#aws_env_unset[@]} -gt 0 || ${#aws_env_overrides[@]} -gt 0 ]]; then + # Scope the page-uploader identity to this helper's aws calls only, and + # drop the ambient identity variables that would otherwise mix with or + # shadow the configured credential source. Apply the overrides with shell + # builtins in a subshell — passing them to an external `env` command would + # expose the credential values in its argv (world-readable via + # /proc//cmdline) while it runs. + ( + for name in "${aws_env_unset[@]}"; do unset "$name"; done + for pair in "${aws_env_overrides[@]}"; do export "$pair"; done + exec aws "${aws_base_args[@]}" "$@" + ) + else + aws "${aws_base_args[@]}" "$@" + fi } object_exists() { @@ -297,6 +323,18 @@ default_prefix="$(normalize_default_prefix "${PAPERCLIP_PAGE_DEFAULT_PREFIX:-}") [[ -n "$base_url" ]] || die "PAPERCLIP_PAGE_BASE_URL is required" base_url="$(normalize_base_url "$base_url")" +page_access_key_id="${PAPERCLIP_PAGE_AWS_ACCESS_KEY_ID:-}" +page_secret_access_key="${PAPERCLIP_PAGE_AWS_SECRET_ACCESS_KEY:-}" +if [[ -n "$page_access_key_id" || -n "$page_secret_access_key" ]]; then + [[ -n "$page_access_key_id" && -n "$page_secret_access_key" ]] || + die "PAPERCLIP_PAGE_AWS_ACCESS_KEY_ID and PAPERCLIP_PAGE_AWS_SECRET_ACCESS_KEY must be set together" + [[ -z "${PAPERCLIP_PAGE_AWS_PROFILE:-}" ]] || + die "set PAPERCLIP_PAGE_AWS_PROFILE or the PAPERCLIP_PAGE_AWS_* key pair, not both" +fi +if [[ -n "${PAPERCLIP_PAGE_AWS_SESSION_TOKEN:-}" && -z "$page_access_key_id" ]]; then + die "PAPERCLIP_PAGE_AWS_SESSION_TOKEN requires the PAPERCLIP_PAGE_AWS_* key pair" +fi + explicit_slug=0 if [[ -n "$slug_arg" ]]; then explicit_slug=1 @@ -310,8 +348,18 @@ if [[ "$dry_run" == "0" ]]; then require_command curl [[ -n "$region" ]] || die "AWS_REGION is required for live publish" aws_base_args=(--region "$region") - if [[ -n "${PAPERCLIP_PAGE_AWS_PROFILE:-}" ]]; then + if [[ -n "$page_access_key_id" ]]; then + aws_env_unset=(AWS_PROFILE AWS_SESSION_TOKEN) + aws_env_overrides=( + AWS_ACCESS_KEY_ID="$page_access_key_id" + AWS_SECRET_ACCESS_KEY="$page_secret_access_key" + ) + if [[ -n "${PAPERCLIP_PAGE_AWS_SESSION_TOKEN:-}" ]]; then + aws_env_overrides+=(AWS_SESSION_TOKEN="$PAPERCLIP_PAGE_AWS_SESSION_TOKEN") + fi + elif [[ -n "${PAPERCLIP_PAGE_AWS_PROFILE:-}" ]]; then aws_base_args+=(--profile "$PAPERCLIP_PAGE_AWS_PROFILE") + aws_env_unset=(AWS_ACCESS_KEY_ID AWS_SECRET_ACCESS_KEY AWS_SESSION_TOKEN AWS_PROFILE) fi fi diff --git a/.agents/skills/paperclip-page/scripts/publish.test.mjs b/.agents/skills/paperclip-page/scripts/publish.test.mjs index 8ef4b47bd2..bb8384bfb0 100644 --- a/.agents/skills/paperclip-page/scripts/publish.test.mjs +++ b/.agents/skills/paperclip-page/scripts/publish.test.mjs @@ -131,6 +131,215 @@ test("rejects hidden files in the source tree", () => { assert.match(result.output, /hidden files and dot paths are not allowed/); }); +test("namespaced page keys require both halves of the pair", () => { + const result = runPublish( + [createSite(), "--slug", "demo-page", "--dry-run"], + { PAPERCLIP_PAGE_AWS_ACCESS_KEY_ID: "AKIAPAGEUPLOADER" }, + ); + + assert.notEqual(result.status, 0); + assert.match(result.output, /must be set together/); +}); + +test("namespaced session token requires the namespaced key pair", () => { + const result = runPublish( + [createSite(), "--slug", "demo-page", "--dry-run"], + { PAPERCLIP_PAGE_AWS_SESSION_TOKEN: "page-session-token" }, + ); + + assert.notEqual(result.status, 0); + assert.match(result.output, /requires the PAPERCLIP_PAGE_AWS_\* key pair/); +}); + +test("namespaced page keys conflict with PAPERCLIP_PAGE_AWS_PROFILE", () => { + const result = runPublish( + [createSite(), "--slug", "demo-page", "--dry-run"], + { + PAPERCLIP_PAGE_AWS_ACCESS_KEY_ID: "AKIAPAGEUPLOADER", + PAPERCLIP_PAGE_AWS_SECRET_ACCESS_KEY: "page-secret", + PAPERCLIP_PAGE_AWS_PROFILE: "paperclip-page-uploader", + }, + ); + + assert.notEqual(result.status, 0); + assert.match(result.output, /not both/); +}); + +test("namespaced page keys are scoped to the helper's aws calls", () => { + const siteDir = createSite(); + const binDir = mkdtempSync(join(tmpdir(), "paperclip-page-bin-")); + tempDirs.add(binDir); + const envDump = join(binDir, "aws-env.txt"); + + writeExecutable( + join(binDir, "aws"), + `#!/usr/bin/env bash +set -euo pipefail +{ + echo "AWS_ACCESS_KEY_ID=\${AWS_ACCESS_KEY_ID:-}" + echo "AWS_SECRET_ACCESS_KEY=\${AWS_SECRET_ACCESS_KEY:-}" + echo "AWS_SESSION_TOKEN=\${AWS_SESSION_TOKEN:-}" + echo "AWS_PROFILE=\${AWS_PROFILE:-}" +} >"${envDump}" +while [[ "$1" == "--region" || "$1" == "--profile" ]]; do + shift 2 +done +if [[ "$1" == "s3api" ]]; then + echo "None" + exit 0 +fi +if [[ "$1" == "s3" && "$2" == "sync" ]]; then + exit 0 +fi +echo "unexpected aws call: $*" >&2 +exit 1 +`, + ); + writeExecutable( + join(binDir, "curl"), + `#!/usr/bin/env bash +exit 0 +`, + ); + + const result = runPublish([siteDir, "--slug", "demo-page"], { + AWS_REGION: "us-east-1", + PATH: `${binDir}:${process.env.PATH}`, + AWS_ACCESS_KEY_ID: "AKIAAMBIENTIDENTITY", + AWS_SECRET_ACCESS_KEY: "ambient-secret", + AWS_SESSION_TOKEN: "ambient-session-token", + AWS_PROFILE: "ambient-profile", + PAPERCLIP_PAGE_AWS_ACCESS_KEY_ID: "AKIAPAGEUPLOADER", + PAPERCLIP_PAGE_AWS_SECRET_ACCESS_KEY: "page-secret", + }); + + assert.equal(result.status, 0); + + const seen = readFileSync(envDump, "utf8"); + assert.match(seen, /^AWS_ACCESS_KEY_ID=AKIAPAGEUPLOADER$/m); + assert.match(seen, /^AWS_SECRET_ACCESS_KEY=page-secret$/m); + assert.match(seen, /^AWS_SESSION_TOKEN=$/m); + assert.match(seen, /^AWS_PROFILE=$/m); +}); + +test("credential values never pass through an external env command's argv", () => { + const siteDir = createSite(); + const binDir = mkdtempSync(join(tmpdir(), "paperclip-page-bin-")); + tempDirs.add(binDir); + const envArgvDump = join(binDir, "env-argv.txt"); + + writeExecutable( + join(binDir, "aws"), + `#!/usr/bin/env bash +set -euo pipefail +while [[ "$1" == "--region" || "$1" == "--profile" ]]; do + shift 2 +done +if [[ "$1" == "s3api" ]]; then + echo "None" + exit 0 +fi +if [[ "$1" == "s3" && "$2" == "sync" ]]; then + exit 0 +fi +echo "unexpected aws call: $*" >&2 +exit 1 +`, + ); + writeExecutable( + join(binDir, "curl"), + `#!/usr/bin/env bash +exit 0 +`, + ); + // Shim env: record every argv it is invoked with, then behave normally. + // Credentials in that argv would be world-readable via /proc//cmdline. + writeExecutable( + join(binDir, "env"), + `#!/bin/bash +printf '%s\\n' "$@" >>"${envArgvDump}" +exec /usr/bin/env "$@" +`, + ); + + const result = runPublish([siteDir, "--slug", "demo-page"], { + AWS_REGION: "us-east-1", + PATH: `${binDir}:${process.env.PATH}`, + PAPERCLIP_PAGE_AWS_ACCESS_KEY_ID: "AKIAPAGEUPLOADER", + PAPERCLIP_PAGE_AWS_SECRET_ACCESS_KEY: "page-secret-argv-canary", + PAPERCLIP_PAGE_AWS_SESSION_TOKEN: "page-session-argv-canary", + }); + + assert.equal(result.status, 0); + + const argvSeen = existsSync(envArgvDump) ? readFileSync(envArgvDump, "utf8") : ""; + assert.doesNotMatch(argvSeen, /page-secret-argv-canary/); + assert.doesNotMatch(argvSeen, /page-session-argv-canary/); + assert.doesNotMatch(argvSeen, /AKIAPAGEUPLOADER/); +}); + +test("PAPERCLIP_PAGE_AWS_PROFILE strips ambient static credentials", () => { + const siteDir = createSite(); + const binDir = mkdtempSync(join(tmpdir(), "paperclip-page-bin-")); + tempDirs.add(binDir); + const envDump = join(binDir, "aws-env.txt"); + + writeExecutable( + join(binDir, "aws"), + `#!/usr/bin/env bash +set -euo pipefail +profile="" +while [[ "$1" == "--region" || "$1" == "--profile" ]]; do + if [[ "$1" == "--profile" ]]; then + profile="$2" + fi + shift 2 +done +{ + echo "PROFILE_ARG=$profile" + echo "AWS_ACCESS_KEY_ID=\${AWS_ACCESS_KEY_ID:-}" + echo "AWS_SECRET_ACCESS_KEY=\${AWS_SECRET_ACCESS_KEY:-}" + echo "AWS_SESSION_TOKEN=\${AWS_SESSION_TOKEN:-}" + echo "AWS_PROFILE=\${AWS_PROFILE:-}" +} >"${envDump}" +if [[ "$1" == "s3api" ]]; then + echo "None" + exit 0 +fi +if [[ "$1" == "s3" && "$2" == "sync" ]]; then + exit 0 +fi +echo "unexpected aws call: $*" >&2 +exit 1 +`, + ); + writeExecutable( + join(binDir, "curl"), + `#!/usr/bin/env bash +exit 0 +`, + ); + + const result = runPublish([siteDir, "--slug", "demo-page"], { + AWS_REGION: "us-east-1", + PATH: `${binDir}:${process.env.PATH}`, + AWS_ACCESS_KEY_ID: "AKIAAMBIENTIDENTITY", + AWS_SECRET_ACCESS_KEY: "ambient-secret", + AWS_SESSION_TOKEN: "ambient-session-token", + AWS_PROFILE: "ambient-profile", + PAPERCLIP_PAGE_AWS_PROFILE: "paperclip-page-uploader", + }); + + assert.equal(result.status, 0); + + const seen = readFileSync(envDump, "utf8"); + assert.match(seen, /^PROFILE_ARG=paperclip-page-uploader$/m); + assert.match(seen, /^AWS_ACCESS_KEY_ID=$/m); + assert.match(seen, /^AWS_SECRET_ACCESS_KEY=$/m); + assert.match(seen, /^AWS_SESSION_TOKEN=$/m); + assert.match(seen, /^AWS_PROFILE=$/m); +}); + test("live publish writes state before URL verification", () => { const siteDir = createSite(); const binDir = mkdtempSync(join(tmpdir(), "paperclip-page-bin-"));