Skip to content

Commit 66f1459

Browse files
authored
fix(security): harden cli trust boundaries
Address CLI security hardening findings and Copilot review follow-up.
1 parent 96f054c commit 66f1459

10 files changed

Lines changed: 313 additions & 27 deletions

File tree

.github/workflows/backfill-release-assets.yml

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -15,16 +15,20 @@ jobs:
1515
timeout-minutes: 5
1616
permissions:
1717
contents: read
18+
outputs:
19+
tag_name: ${{ steps.validate.outputs.tag_name }}
1820

1921
steps:
2022
- name: Validate tag name
23+
id: validate
2124
env:
2225
TAG_NAME: ${{ inputs.tag_name }}
2326
run: |
2427
if [[ ! "$TAG_NAME" =~ ^v[0-9]+\.[0-9]+\.[0-9]+([-+][0-9A-Za-z.-]+)?$ ]]; then
25-
echo "Invalid release tag: $TAG_NAME" >&2
28+
echo "Invalid release tag." >&2
2629
exit 1
2730
fi
31+
printf 'tag_name=%s\n' "$TAG_NAME" >> "$GITHUB_OUTPUT"
2832
2933
build-unix-binaries:
3034
name: Build ${{ matrix.os }} release assets
@@ -58,7 +62,7 @@ jobs:
5862
uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6
5963
with:
6064
fetch-depth: 0
61-
ref: ${{ inputs.tag_name }}
65+
ref: ${{ needs.validate-release-tag.outputs.tag_name }}
6266

6367
- name: Set up Vite+
6468
uses: voidzero-dev/setup-vp@45e5c098f1095cc6b65fd92534603e7be70386c1 # v1
@@ -78,7 +82,7 @@ jobs:
7882
- name: Package release assets
7983
shell: pwsh
8084
env:
81-
TAG_NAME: ${{ inputs.tag_name }}
85+
TAG_NAME: ${{ needs.validate-release-tag.outputs.tag_name }}
8286
run: |
8387
$version = $env:TAG_NAME.TrimStart("v")
8488
$assetBase = "putio-cli-$version-${{ matrix.asset_os }}-${{ matrix.asset_arch }}"
@@ -103,7 +107,7 @@ jobs:
103107
uses: softprops/action-gh-release@b4309332981a82ec1c5618f44dd2e27cc8bfbfda # v3
104108
with:
105109
token: ${{ steps.release-bot.outputs.token }}
106-
tag_name: ${{ inputs.tag_name }}
110+
tag_name: ${{ needs.validate-release-tag.outputs.tag_name }}
107111
files: |
108112
.artifacts/release/*
109113
@@ -129,7 +133,7 @@ jobs:
129133
uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6
130134
with:
131135
fetch-depth: 0
132-
ref: ${{ inputs.tag_name }}
136+
ref: ${{ needs.validate-release-tag.outputs.tag_name }}
133137

134138
- name: Set up Vite+
135139
uses: voidzero-dev/setup-vp@45e5c098f1095cc6b65fd92534603e7be70386c1 # v1
@@ -149,7 +153,7 @@ jobs:
149153
- name: Package release assets
150154
shell: pwsh
151155
env:
152-
TAG_NAME: ${{ inputs.tag_name }}
156+
TAG_NAME: ${{ needs.validate-release-tag.outputs.tag_name }}
153157
run: |
154158
$version = $env:TAG_NAME.TrimStart("v")
155159
$assetBase = "putio-cli-$version-windows-amd64"
@@ -169,6 +173,6 @@ jobs:
169173
uses: softprops/action-gh-release@b4309332981a82ec1c5618f44dd2e27cc8bfbfda # v3
170174
with:
171175
token: ${{ steps.release-bot.outputs.token }}
172-
tag_name: ${{ inputs.tag_name }}
176+
tag_name: ${{ needs.validate-release-tag.outputs.tag_name }}
173177
files: |
174178
.artifacts/release/*

AGENTS.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ Focused:
3232
Runtime proofs:
3333

3434
- `./dist/bin.mjs describe`
35-
- `./dist/bin.mjs whoami --output json`
35+
- `./dist/bin.mjs whoami --fields auth --output json`
3636

3737
## Development Guidance
3838

README.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -100,10 +100,10 @@ Link your account:
100100
putio auth login
101101
```
102102

103-
Check the account:
103+
Check the auth source:
104104

105105
```bash
106-
putio whoami --output json
106+
putio whoami --fields auth --output json
107107
```
108108

109109
Read a small JSON result:

install.sh

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -113,8 +113,22 @@ verify_checksum() {
113113
}
114114

115115
ensure_install_dir() {
116-
mkdir -p "$INSTALL_DIR"
116+
if [ ! -d "$INSTALL_DIR" ]; then
117+
mkdir -p "$INSTALL_DIR"
118+
chmod 0755 "$INSTALL_DIR"
119+
fi
120+
117121
[ -w "$INSTALL_DIR" ] || fail "install directory is not writable: $INSTALL_DIR"
122+
123+
install_dir_target="$(cd "$INSTALL_DIR" && pwd -P)" || fail "unable to resolve install directory: $INSTALL_DIR"
124+
permissions="$(ls -ld "$install_dir_target" | awk '{print $1}')"
125+
group_write="$(printf '%s' "$permissions" | cut -c6)"
126+
other_write="$(printf '%s' "$permissions" | cut -c9)"
127+
128+
if { [ "$group_write" = "w" ] || [ "$other_write" = "w" ]; } &&
129+
[ "${PUTIO_CLI_ALLOW_SHARED_INSTALL_DIR:-}" != "1" ]; then
130+
fail "install directory is group/world-writable: $INSTALL_DIR. Set PUTIO_CLI_ALLOW_SHARED_INSTALL_DIR=1 to allow this intentionally."
131+
fi
118132
}
119133

120134
print_path_hint() {
@@ -156,8 +170,9 @@ printf '%s\n' "putio installer: verifying checksum"
156170
verify_checksum "$checksum_path" "$archive_path"
157171

158172
tar -xzf "$archive_path" -C "$work_dir"
159-
chmod +x "$work_dir/putio"
173+
chmod 0755 "$work_dir/putio"
160174
mv "$work_dir/putio" "$INSTALL_DIR/putio"
175+
chmod 0755 "$INSTALL_DIR/putio"
161176

162177
printf '%s\n' "putio installer: installed to $INSTALL_DIR/putio"
163178
print_path_hint

skills/putio-cli/references/reads.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,14 +3,14 @@
33
Prefer structured output:
44

55
```bash
6-
putio whoami
6+
putio whoami --fields auth --output json
77
putio files list --output json
88
```
99

1010
Use `--fields` with top-level keys only:
1111

1212
```bash
13-
putio whoami --fields auth,info
13+
putio whoami --fields auth --output json
1414
putio files list --fields files,total --output json
1515
```
1616

src/command-paths.test.ts

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -929,6 +929,58 @@ describe("cli command paths", () => {
929929
);
930930
});
931931

932+
it("rejects repeated cursors while streaming file list pages", async () => {
933+
mocks.listFilesMock.mockReturnValueOnce(
934+
Effect.succeed({
935+
cursor: "cursor-1",
936+
files: [{ id: 1, name: "Movies" }],
937+
total: 2,
938+
}),
939+
);
940+
mocks.continueFilesMock.mockReturnValueOnce(
941+
Effect.succeed({
942+
cursor: "cursor-1",
943+
files: [{ id: 2, name: "Shows" }],
944+
total: 2,
945+
}),
946+
);
947+
948+
await expect(
949+
runCliInTest(["putio", "files", "list", "--page-all", "--output", "ndjson"]),
950+
).rejects.toMatchObject({
951+
message: "`files list` pagination returned a repeated cursor.",
952+
});
953+
954+
expect(mocks.continueFilesMock).toHaveBeenCalledTimes(1);
955+
expect(mocks.writeOutputMock).toHaveBeenCalledTimes(1);
956+
});
957+
958+
it("rejects cumulative item overflow while streaming file list pages", async () => {
959+
mocks.listFilesMock.mockReturnValueOnce(
960+
Effect.succeed({
961+
cursor: "cursor-1",
962+
files: Array.from({ length: 60_000 }, (_value, id) => ({ id })),
963+
total: 110_001,
964+
}),
965+
);
966+
mocks.continueFilesMock.mockReturnValueOnce(
967+
Effect.succeed({
968+
cursor: null,
969+
files: Array.from({ length: 50_001 }, (_value, id) => ({ id: id + 60_000 })),
970+
total: 110_001,
971+
}),
972+
);
973+
974+
await expect(
975+
runCliInTest(["putio", "files", "list", "--page-all", "--output", "ndjson"]),
976+
).rejects.toMatchObject({
977+
message: "`files list` pagination exceeded 100000 items.",
978+
});
979+
980+
expect(mocks.continueFilesMock).toHaveBeenCalledTimes(1);
981+
expect(mocks.writeOutputMock).toHaveBeenCalledTimes(1);
982+
});
983+
932984
it("executes files rename", async () => {
933985
await expect(
934986
runCliInTest([

src/internal/command.test.ts

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -185,6 +185,43 @@ describe("resolveReadOutputControls", () => {
185185
expect(String(exit.cause)).toContain("cannot include `?` or `#` fragments");
186186
}
187187
});
188+
189+
it("does not reflect control-bearing field selectors in errors", async () => {
190+
const payload = "\u001B]52;c;cHduZWQ=\u0007";
191+
const exit = await Effect.runPromiseExit(
192+
provideRuntime(
193+
resolveReadOutputControls({
194+
fields: Option.some(payload),
195+
output: "json",
196+
}),
197+
),
198+
);
199+
200+
expect(exit._tag).toBe("Failure");
201+
if (exit._tag === "Failure") {
202+
const cause = String(exit.cause);
203+
expect(cause).toContain("`--fields` selector #1 cannot contain control characters");
204+
expect(cause).not.toContain(payload);
205+
}
206+
});
207+
208+
it("identifies the invalid field selector by position without echoing it", async () => {
209+
const exit = await Effect.runPromiseExit(
210+
provideRuntime(
211+
resolveReadOutputControls({
212+
fields: Option.some("auth,bad.field"),
213+
output: "json",
214+
}),
215+
),
216+
);
217+
218+
expect(exit._tag).toBe("Failure");
219+
if (exit._tag === "Failure") {
220+
const cause = String(exit.cause);
221+
expect(cause).toContain("`--fields` selector #2 only accepts top-level field names");
222+
expect(cause).not.toContain("bad.field");
223+
}
224+
});
188225
});
189226

190227
describe("selectTopLevelFields", () => {
@@ -272,6 +309,34 @@ describe("collectAllCursorPages", () => {
272309
total: 3,
273310
});
274311
});
312+
313+
it("rejects repeated cursors", async () => {
314+
const continueWithCursor = () =>
315+
Effect.succeed({
316+
cursor: "cursor-1",
317+
files: [{ id: 2 }],
318+
total: 2,
319+
});
320+
321+
const exit = await Effect.runPromiseExit(
322+
collectAllCursorPages({
323+
command: "files list",
324+
continueWithCursor,
325+
initial: {
326+
cursor: "cursor-1",
327+
files: [{ id: 1 }],
328+
total: 2,
329+
},
330+
itemKey: "files",
331+
pageAll: true,
332+
}),
333+
);
334+
335+
expect(exit._tag).toBe("Failure");
336+
if (exit._tag === "Failure") {
337+
expect(String(exit.cause)).toContain("pagination returned a repeated cursor");
338+
}
339+
});
275340
});
276341

277342
describe("agent-safe string validation", () => {

0 commit comments

Comments
 (0)