Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 38 additions & 13 deletions .github/workflows/release.yml
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,28 @@ permissions:
jobs:
publish:
runs-on: ubuntu-latest
# Workflow-context values are bound to env here and referenced as
# shell variables ($TAG/$REPO/$COMMIT_SHA) in run: blocks instead of
# `${{ }}` interpolation, so an attacker-controlled tag name cannot be
# injected into a script body. Tag pushes are attacker-controllable:
# anyone able to push a v* tag triggers this workflow.
env:
TAG: ${{ github.ref_name }}
REPO: ${{ github.repository }}
COMMIT_SHA: ${{ github.sha }}
steps:
- name: Validate tag format
# Fail closed before any other step runs. A strict semver gate
# rejects a tag containing shell metacharacters ($(), backticks,
# ;, |) so it never reaches a later run: block.
run: |
if [[ ! "$TAG" =~ ^v[0-9]+\.[0-9]+\.[0-9]+(-[0-9A-Za-z.-]+)?(\+[0-9A-Za-z.-]+)?$ ]]; then
echo "❌ Error: Tag '$TAG' is not a valid vMAJOR.MINOR.PATCH semver tag"
echo "Releases must be triggered by a strict semver tag, e.g. v1.2.3 or v1.2.3-rc.1"
exit 1
fi
echo "✅ Tag format validated: $TAG"

- name: Checkout
uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
with:
Expand All @@ -21,7 +42,7 @@ jobs:
- name: Verify tag is on main branch
run: |
git fetch origin main
if ! git merge-base --is-ancestor ${{ github.sha }} origin/main; then
if ! git merge-base --is-ancestor "$COMMIT_SHA" origin/main; then
echo "❌ Tag is not on the main branch — aborting release"
exit 1
fi
Expand All @@ -46,19 +67,23 @@ jobs:
- name: Extract version from tag
id: version
run: |
VERSION=${GITHUB_REF#refs/tags/v}
echo "version=$VERSION" >> $GITHUB_OUTPUT
# $GITHUB_REF is a runner-provided env var (safe shell
# expansion, not template interpolation); the tag was strictly
# validated above, so VERSION is a clean semver string.
VERSION="${GITHUB_REF#refs/tags/v}"
echo "version=$VERSION" >> "$GITHUB_OUTPUT"
echo "Publishing version: $VERSION"

- name: Verify tag matches package.json version
env:
VERSION: ${{ steps.version.outputs.version }}
run: |
TAG_VERSION=${{ steps.version.outputs.version }}
PKG_VERSION=$(node -p "require('./package.json').version")
if [ "$TAG_VERSION" != "$PKG_VERSION" ]; then
echo "❌ Tag v$TAG_VERSION does not match package.json version $PKG_VERSION"
if [ "$VERSION" != "$PKG_VERSION" ]; then
echo "❌ Tag v$VERSION does not match package.json version $PKG_VERSION"
exit 1
fi
echo "✅ Version confirmed: $TAG_VERSION"
echo "✅ Version confirmed: $VERSION"

- name: Build
run: pnpm build
Expand All @@ -75,11 +100,11 @@ jobs:

- name: Generate release notes
id: release_notes
env:
VERSION: ${{ steps.version.outputs.version }}
run: |
CURRENT_TAG=${{ github.ref_name }}
PREV_TAG=$(git tag -l 'v*' --sort=-version:refname | grep -v "^${CURRENT_TAG}$" | head -1)
PREV_TAG=$(git tag -l 'v*' --sort=-version:refname | grep -v "^${TAG}$" | head -1)
RELEASE_DATE=$(date +%Y-%m-%d)
VERSION=${{ steps.version.outputs.version }}

if [ -n "$PREV_TAG" ]; then
COMMITS=$(git log ${PREV_TAG}..HEAD --pretty=format:"%s %h" --no-merges)
Expand All @@ -96,11 +121,11 @@ jobs:
if [[ $message =~ \(#([0-9]+)\) ]]; then
PR_NUM="${BASH_REMATCH[1]}"
CLEAN_MESSAGE=$(echo "$message" | sed -E 's/ ?\(#[0-9]+\)//')
PR_LINK="[#$PR_NUM](https://github.com/${{ github.repository }}/pull/$PR_NUM)"
COMMIT_LINK="[$hash](https://github.com/${{ github.repository }}/commit/$hash)"
PR_LINK="[#$PR_NUM](https://github.com/${REPO}/pull/$PR_NUM)"
COMMIT_LINK="[$hash](https://github.com/${REPO}/commit/$hash)"
ITEM="$CLEAN_MESSAGE ($PR_LINK) ($COMMIT_LINK)"
else
COMMIT_LINK="[$hash](https://github.com/${{ github.repository }}/commit/$hash)"
COMMIT_LINK="[$hash](https://github.com/${REPO}/commit/$hash)"
ITEM="$message ($COMMIT_LINK)"
fi

Expand Down
6 changes: 6 additions & 0 deletions src/commands/profiles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,12 @@ export function searchProfilesRun(options: SearchProfilesOptions) {
}
}

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

Expand Down
3 changes: 3 additions & 0 deletions src/commands/segments.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,9 @@ export function buildCreateSegmentBody(options: CreateSegmentOptions) {
let parsedFilterSets: unknown
try {
parsedFilterSets = JSON.parse(options.filterSets)
if (!Array.isArray(parsedFilterSets)) {
throw new Error('not an array')
}
} catch {
throw new Error('--filter-sets must be a valid JSON array')
}
Comment on lines 34 to 41
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

For better code clarity and consistency with other parts of the codebase (like parseSearchConditions in profiles.ts), it's better to separate the JSON parsing from the type validation. This makes the control flow more explicit and avoids throwing an error just to have it immediately caught with its message discarded.

Suggested change
try {
parsedFilterSets = JSON.parse(options.filterSets)
if (!Array.isArray(parsedFilterSets)) {
throw new Error('not an array')
}
} catch {
throw new Error('--filter-sets must be a valid JSON array')
}
try {
parsedFilterSets = JSON.parse(options.filterSets);
} catch {
throw new Error('--filter-sets must be a valid JSON array');
}
if (!Array.isArray(parsedFilterSets)) {
throw new Error('--filter-sets must be a valid JSON array');
}

Expand Down
10 changes: 10 additions & 0 deletions src/lib/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,17 @@ export function readConfig(): FormoConfig {
export function saveConfig(updates: Partial<FormoConfig>): void {
const existing = readConfig();
const merged = { ...existing, ...updates };
// The `mode` option on mkdir/writeFile is honored ONLY when the path is
// newly created. A pre-existing dir/file (older CLI version, dotfile-sync
// tool, another app under ~/.config) keeps its old, possibly
// group/world-readable perms — leaking the plaintext API key on a
// multi-user host. chmod unconditionally so 0o700/0o600 always holds.
fs.mkdirSync(CONFIG_DIR, { recursive: true, mode: 0o700 });
fs.chmodSync(CONFIG_DIR, 0o700);
fs.writeFileSync(CONFIG_FILE, JSON.stringify(merged, null, 2), {
mode: 0o600,
});
fs.chmodSync(CONFIG_FILE, 0o600);
}

export function clearConfig(): void {
Expand All @@ -35,6 +42,9 @@ export function clearConfig(): void {
fs.writeFileSync(CONFIG_FILE, JSON.stringify({}, null, 2), {
mode: 0o600,
});
// Same create-only-mode caveat as saveConfig: enforce 0o600 on the
// already-existing file so the cleared config can't be left readable.
fs.chmodSync(CONFIG_FILE, 0o600);
}
} catch {
// Ignore errors if file doesn't exist
Expand Down
6 changes: 6 additions & 0 deletions test/commands/segments.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,5 +19,11 @@ describe('commands/segments', function () {
it('throws on invalid --filter-sets JSON', function () {
expect(() => createSegmentRun({ title: 'x', filterSets: 'not-json' })).to.throw(/filter-sets/);
});

it('throws when --filter-sets is valid JSON but not an array', function () {
expect(() => createSegmentRun({ title: 'x', filterSets: '{"a":1}' })).to.throw(/filter-sets/);
expect(() => createSegmentRun({ title: 'x', filterSets: '5' })).to.throw(/filter-sets/);
expect(() => createSegmentRun({ title: 'x', filterSets: '"foo"' })).to.throw(/filter-sets/);
});
});
});