Skip to content

fix: remediate deepsec security scan findings#9

Merged
yosriady merged 1 commit into
mainfrom
sec/deepsec
May 18, 2026
Merged

fix: remediate deepsec security scan findings#9
yosriady merged 1 commit into
mainfrom
sec/deepsec

Conversation

@yosriady
Copy link
Copy Markdown
Contributor

@yosriady yosriady commented May 18, 2026

  • 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.


View in Codesmith
Need help on this PR? Tag @codesmith with what you need.

  • Let Codesmith autofix CI failures and bot reviews

- 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>
@yosriady yosriady merged commit 349c4e8 into main May 18, 2026
5 checks passed
@yosriady yosriady deleted the sec/deepsec branch May 18, 2026 14:34
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces several improvements, including documentation for the GET-with-body API contract in profiles, stricter validation for filter sets in segments, and enhanced security for configuration files by explicitly enforcing file permissions using chmodSync. A review comment suggests refactoring the filter set validation to separate JSON parsing from type checking for better clarity and consistency.

Comment thread src/commands/segments.ts
Comment on lines 34 to 41
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')
}
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');
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant