Skip to content

Commit 349c4e8

Browse files
yosriadyclaude
yosriady
and
claude
authored
fix: remediate deepsec security scan findings (#9)
- release.yml: close GitHub Actions script-injection vector via attacker-controlled git tag name (CWE-78). Bind workflow context to job-level env (TAG/REPO/COMMIT_SHA), add a fail-closed semver tag-format gate as the first step, remove all ${{ }} interpolation from run: blocks, quote shell expansions. - config.ts: enforce 0o700/0o600 via explicit chmod after write in saveConfig and clearConfig — writeFileSync/mkdirSync mode is create-only, so a pre-existing config kept loose perms and could leak the plaintext API key on a multi-user host. - segments.ts: validate --filter-sets is a JSON array (matches the documented contract and buildImportBody); add regression test. - profiles.ts: document the intentional GET-with-body profile-search contract so it is not "fixed" to POST (would break the Formo API). Build (tsc), lint (eslint), and 88 tests pass. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent a3194c1 commit 349c4e8

5 files changed

Lines changed: 63 additions & 13 deletions

File tree

‎.github/workflows/release.yml‎

Lines changed: 38 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,28 @@ permissions:
1212
jobs:
1313
publish:
1414
runs-on: ubuntu-latest
15+
# Workflow-context values are bound to env here and referenced as
16+
# shell variables ($TAG/$REPO/$COMMIT_SHA) in run: blocks instead of
17+
# `${{ }}` interpolation, so an attacker-controlled tag name cannot be
18+
# injected into a script body. Tag pushes are attacker-controllable:
19+
# anyone able to push a v* tag triggers this workflow.
20+
env:
21+
TAG: ${{ github.ref_name }}
22+
REPO: ${{ github.repository }}
23+
COMMIT_SHA: ${{ github.sha }}
1524
steps:
25+
- name: Validate tag format
26+
# Fail closed before any other step runs. A strict semver gate
27+
# rejects a tag containing shell metacharacters ($(), backticks,
28+
# ;, |) so it never reaches a later run: block.
29+
run: |
30+
if [[ ! "$TAG" =~ ^v[0-9]+\.[0-9]+\.[0-9]+(-[0-9A-Za-z.-]+)?(\+[0-9A-Za-z.-]+)?$ ]]; then
31+
echo "❌ Error: Tag '$TAG' is not a valid vMAJOR.MINOR.PATCH semver tag"
32+
echo "Releases must be triggered by a strict semver tag, e.g. v1.2.3 or v1.2.3-rc.1"
33+
exit 1
34+
fi
35+
echo "✅ Tag format validated: $TAG"
36+
1637
- name: Checkout
1738
uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
1839
with:
@@ -21,7 +42,7 @@ jobs:
2142
- name: Verify tag is on main branch
2243
run: |
2344
git fetch origin main
24-
if ! git merge-base --is-ancestor ${{ github.sha }} origin/main; then
45+
if ! git merge-base --is-ancestor "$COMMIT_SHA" origin/main; then
2546
echo "❌ Tag is not on the main branch — aborting release"
2647
exit 1
2748
fi
@@ -46,19 +67,23 @@ jobs:
4667
- name: Extract version from tag
4768
id: version
4869
run: |
49-
VERSION=${GITHUB_REF#refs/tags/v}
50-
echo "version=$VERSION" >> $GITHUB_OUTPUT
70+
# $GITHUB_REF is a runner-provided env var (safe shell
71+
# expansion, not template interpolation); the tag was strictly
72+
# validated above, so VERSION is a clean semver string.
73+
VERSION="${GITHUB_REF#refs/tags/v}"
74+
echo "version=$VERSION" >> "$GITHUB_OUTPUT"
5175
echo "Publishing version: $VERSION"
5276
5377
- name: Verify tag matches package.json version
78+
env:
79+
VERSION: ${{ steps.version.outputs.version }}
5480
run: |
55-
TAG_VERSION=${{ steps.version.outputs.version }}
5681
PKG_VERSION=$(node -p "require('./package.json').version")
57-
if [ "$TAG_VERSION" != "$PKG_VERSION" ]; then
58-
echo "❌ Tag v$TAG_VERSION does not match package.json version $PKG_VERSION"
82+
if [ "$VERSION" != "$PKG_VERSION" ]; then
83+
echo "❌ Tag v$VERSION does not match package.json version $PKG_VERSION"
5984
exit 1
6085
fi
61-
echo "✅ Version confirmed: $TAG_VERSION"
86+
echo "✅ Version confirmed: $VERSION"
6287
6388
- name: Build
6489
run: pnpm build
@@ -75,11 +100,11 @@ jobs:
75100

76101
- name: Generate release notes
77102
id: release_notes
103+
env:
104+
VERSION: ${{ steps.version.outputs.version }}
78105
run: |
79-
CURRENT_TAG=${{ github.ref_name }}
80-
PREV_TAG=$(git tag -l 'v*' --sort=-version:refname | grep -v "^${CURRENT_TAG}$" | head -1)
106+
PREV_TAG=$(git tag -l 'v*' --sort=-version:refname | grep -v "^${TAG}$" | head -1)
81107
RELEASE_DATE=$(date +%Y-%m-%d)
82-
VERSION=${{ steps.version.outputs.version }}
83108
84109
if [ -n "$PREV_TAG" ]; then
85110
COMMITS=$(git log ${PREV_TAG}..HEAD --pretty=format:"%s %h" --no-merges)
@@ -96,11 +121,11 @@ jobs:
96121
if [[ $message =~ \(#([0-9]+)\) ]]; then
97122
PR_NUM="${BASH_REMATCH[1]}"
98123
CLEAN_MESSAGE=$(echo "$message" | sed -E 's/ ?\(#[0-9]+\)//')
99-
PR_LINK="[#$PR_NUM](https://github.com/${{ github.repository }}/pull/$PR_NUM)"
100-
COMMIT_LINK="[$hash](https://github.com/${{ github.repository }}/commit/$hash)"
124+
PR_LINK="[#$PR_NUM](https://github.com/${REPO}/pull/$PR_NUM)"
125+
COMMIT_LINK="[$hash](https://github.com/${REPO}/commit/$hash)"
101126
ITEM="$CLEAN_MESSAGE ($PR_LINK) ($COMMIT_LINK)"
102127
else
103-
COMMIT_LINK="[$hash](https://github.com/${{ github.repository }}/commit/$hash)"
128+
COMMIT_LINK="[$hash](https://github.com/${REPO}/commit/$hash)"
104129
ITEM="$message ($COMMIT_LINK)"
105130
fi
106131

‎src/commands/profiles.ts‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,12 @@ export function searchProfilesRun(options: SearchProfilesOptions) {
121121
}
122122
}
123123

124+
// INTENTIONAL: the Formo search API is `GET /v0/profiles` with the
125+
// `{ conditions, logic }` filter object in the *request body* (see
126+
// docs.formo.so/api/profiles/search — it has a "Request Body (Filters)"
127+
// section under a GET endpoint). This GET-with-body shape is the
128+
// documented, server-supported contract. Do NOT "fix" it to POST — that
129+
// breaks the API. Filter-less searches still go over query params only.
124130
return client.request({ method: 'get', url: '/v0/profiles/', params, data: body })
125131
}
126132

‎src/commands/segments.ts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,9 @@ export function buildCreateSegmentBody(options: CreateSegmentOptions) {
3333
let parsedFilterSets: unknown
3434
try {
3535
parsedFilterSets = JSON.parse(options.filterSets)
36+
if (!Array.isArray(parsedFilterSets)) {
37+
throw new Error('not an array')
38+
}
3639
} catch {
3740
throw new Error('--filter-sets must be a valid JSON array')
3841
}

‎src/lib/config.ts‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,10 +23,17 @@ export function readConfig(): FormoConfig {
2323
export function saveConfig(updates: Partial<FormoConfig>): void {
2424
const existing = readConfig();
2525
const merged = { ...existing, ...updates };
26+
// The `mode` option on mkdir/writeFile is honored ONLY when the path is
27+
// newly created. A pre-existing dir/file (older CLI version, dotfile-sync
28+
// tool, another app under ~/.config) keeps its old, possibly
29+
// group/world-readable perms — leaking the plaintext API key on a
30+
// multi-user host. chmod unconditionally so 0o700/0o600 always holds.
2631
fs.mkdirSync(CONFIG_DIR, { recursive: true, mode: 0o700 });
32+
fs.chmodSync(CONFIG_DIR, 0o700);
2733
fs.writeFileSync(CONFIG_FILE, JSON.stringify(merged, null, 2), {
2834
mode: 0o600,
2935
});
36+
fs.chmodSync(CONFIG_FILE, 0o600);
3037
}
3138

3239
export function clearConfig(): void {
@@ -35,6 +42,9 @@ export function clearConfig(): void {
3542
fs.writeFileSync(CONFIG_FILE, JSON.stringify({}, null, 2), {
3643
mode: 0o600,
3744
});
45+
// Same create-only-mode caveat as saveConfig: enforce 0o600 on the
46+
// already-existing file so the cleared config can't be left readable.
47+
fs.chmodSync(CONFIG_FILE, 0o600);
3848
}
3949
} catch {
4050
// Ignore errors if file doesn't exist

‎test/commands/segments.test.ts‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,5 +19,11 @@ describe('commands/segments', function () {
1919
it('throws on invalid --filter-sets JSON', function () {
2020
expect(() => createSegmentRun({ title: 'x', filterSets: 'not-json' })).to.throw(/filter-sets/);
2121
});
22+
23+
it('throws when --filter-sets is valid JSON but not an array', function () {
24+
expect(() => createSegmentRun({ title: 'x', filterSets: '{"a":1}' })).to.throw(/filter-sets/);
25+
expect(() => createSegmentRun({ title: 'x', filterSets: '5' })).to.throw(/filter-sets/);
26+
expect(() => createSegmentRun({ title: 'x', filterSets: '"foo"' })).to.throw(/filter-sets/);
27+
});
2228
});
2329
});

0 commit comments

Comments
 (0)