Skip to content

Comments

Reject invalid characters in quickstart#804

Merged
miles-kt-inkeep merged 5 commits intomainfrom
fix/quickstart-character-allowlist
Oct 23, 2025
Merged

Reject invalid characters in quickstart#804
miles-kt-inkeep merged 5 commits intomainfrom
fix/quickstart-character-allowlist

Conversation

@miles-kt-inkeep
Copy link
Contributor

No description provided.

@changeset-bot
Copy link

changeset-bot bot commented Oct 22, 2025

🦋 Changeset detected

Latest commit: 92d957c

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 7 packages
Name Type
@inkeep/agents-cli Patch
@inkeep/agents-core Patch
@inkeep/agents-manage-api Patch
@inkeep/agents-manage-ui Patch
@inkeep/agents-run-api Patch
@inkeep/agents-sdk Patch
@inkeep/create-agents Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@vercel
Copy link

vercel bot commented Oct 22, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Comments Updated (UTC)
agents-manage-api Ready Ready Preview Comment Oct 22, 2025 10:08pm
agents-manage-ui Ready Ready Preview Comment Oct 22, 2025 10:08pm
agents-run-api Ready Ready Preview Comment Oct 22, 2025 10:08pm
1 Skipped Deployment
Project Deployment Preview Comments Updated (UTC)
agents-docs Skipped Skipped Oct 22, 2025 10:08pm

@claude
Copy link
Contributor

claude bot commented Oct 22, 2025

Claude finished @miles-kt-inkeep's task —— View job


TypeScript Staff Engineer Review

Review Checklist

  • Read repository documentation and setup instructions
  • Analyze the specific changes in utils.ts
  • Research existing patterns and related code
  • Evaluate security and validation approach
  • Check for architectural consistency
  • Assess customer impact and edge cases
  • Provide comprehensive feedback and recommendations

Comprehensive Technical Review

Context & Changes

This PR introduces character validation for directory names in the createAgents function at packages/create-agents/src/utils.ts:114-116 and utils.ts:128-132. The validation restricts directory names to alphanumeric characters, hyphens, and underscores using the regex /^[a-zA-Z0-9_-]+$/.

Positive Aspects

1. Consistency with Existing Patterns

  • The validation regex /^[a-zA-Z0-9_-]+$/ perfectly matches the existing idSchema in /agents-manage-ui/src/lib/validation.ts:12, demonstrating good architectural consistency
  • Both interactive prompt validation and programmatic input validation are handled, ensuring comprehensive coverage

2. Security & Robustness

  • Prevents potential directory traversal attacks (../, ./)
  • Blocks problematic characters that could cause issues across different file systems (Windows/Unix)
  • Consistent error messaging between prompt and programmatic validation

⚠️ Areas for Improvement

1. DRY Principle Violation (Critical)

The regex /^[a-zA-Z0-9_-]+$/ and error message are duplicated across:

  • Interactive validation function (lines 114-116)
  • Programmatic validation (lines 128-132)
  • Similar pattern exists in /agents-manage-ui/src/lib/validation.ts:12

Recommendation: Extract to a shared constant:

const DIRECTORY_NAME_PATTERN = /^[a-zA-Z0-9_-]+$/;
const DIRECTORY_NAME_ERROR = 'Directory name can only contain letters, numbers, hyphens (-), and underscores (_)';

2. Missing Edge Cases (High)

The current validation doesn't handle several problematic scenarios:

Reserved Names: Windows reserves names like CON, PRN, AUX, NUL, COM1-9, LPT1-9

const RESERVED_NAMES = /^(CON|PRN|AUX|NUL|COM[1-9]|LPT[1-9])$/i;

Length Constraints: No minimum/maximum length validation (filesystem limits vary)

Leading/Trailing Issues: Names starting/ending with hyphens could cause issues

if (value.startsWith('-') || value.endsWith('-')) {
  return 'Directory name cannot start or end with hyphens';
}

3. Test Coverage Gap (High)

Examining /packages/create-agents/src/__tests__/utils.test.ts, there are no tests specifically covering the new character validation logic. This is concerning for a security-related feature.

Missing Test Cases:

  • Invalid characters (spaces, @, #, etc.)
  • Reserved Windows names
  • Edge cases (empty string, very long names)
  • Both interactive and programmatic validation paths

4. User Experience Considerations (Medium)

Error Message Clarity: The current message doesn't explain why these characters are restricted. Consider:

'Directory name can only contain letters, numbers, hyphens (-), and underscores (_) to ensure compatibility across all operating systems'

Suggestion Support: Could provide auto-sanitization suggestions:

const suggested = value.replace(/[^a-zA-Z0-9_-]/g, '-');
return `Invalid characters detected. Suggested: "${suggested}"`;

5. Architectural Concerns (Medium)

Cross-Package Validation: Similar directory/ID validation exists in agents-manage-ui. Consider creating a shared validation utility in agents-core to maintain consistency across packages.

Integration Points: This validation affects:

  • File system operations (fs.mkdir, fs.pathExists)
  • Git operations (if enabled)
  • Template cloning paths
  • Database project creation

All downstream effects should be considered for the validation rules.

Security Assessment

Good: Prevents directory traversal
Good: Blocks shell injection characters
⚠️ Missing: Reserved name protection
⚠️ Missing: Path length limits

Recommendations by Priority

High Priority:

  1. Add comprehensive test coverage for all validation scenarios
  2. Extract validation logic to shared constants/utilities
  3. Add reserved name validation for Windows compatibility

Medium Priority:
4. Implement length constraints (1-255 characters)
5. Improve error messages with context
6. Consider creating shared validation utilities across packages

Low Priority:
7. Add auto-suggestion functionality for invalid names
8. Consider internationalization for error messages

Implementation Suggestions

// Shared validation utility
const DIRECTORY_VALIDATION = {
  pattern: /^[a-zA-Z0-9_-]+$/,
  reservedNames: /^(CON|PRN|AUX|NUL|COM[1-9]|LPT[1-9])$/i,
  minLength: 1,
  maxLength: 255,
  
  validate(value: string): string | undefined {
    if (!value || value.trim() === '') return 'Directory name is required';
    if (value.length < this.minLength || value.length > this.maxLength) {
      return `Directory name must be between ${this.minLength} and ${this.maxLength} characters`;
    }
    if (this.reservedNames.test(value)) {
      return 'Directory name cannot be a reserved system name';
    }
    if (!this.pattern.test(value)) {
      return 'Directory name can only contain letters, numbers, hyphens (-), and underscores (_)';
    }
    return undefined;
  }
};

Conclusion

While this PR addresses an important security concern and follows existing patterns well, it needs enhanced test coverage, better edge case handling, and consolidation of validation logic to meet production standards. The core approach is sound but requires refinement for robustness.


@vercel vercel bot temporarily deployed to Preview – agents-docs October 22, 2025 22:02 Inactive
@vercel vercel bot temporarily deployed to Preview – agents-docs October 22, 2025 22:06 Inactive
@miles-kt-inkeep miles-kt-inkeep merged commit 96c499d into main Oct 23, 2025
6 checks passed
@miles-kt-inkeep miles-kt-inkeep deleted the fix/quickstart-character-allowlist branch October 23, 2025 12:58
nick-inkeep added a commit that referenced this pull request Feb 4, 2026
SDK versions 0.2.27+ have a bug causing AJV validation crashes
before any API calls are made. This affects all PR reviews.

Tracking issue: anthropics/claude-code-action#892
Related: #852, #880, #804

Will revert to @v1 when the upstream issue is resolved.
nick-inkeep added a commit that referenced this pull request Feb 4, 2026
SDK versions 0.2.27+ have a bug causing AJV validation crashes
before any API calls are made. This affects all PR reviews.

Tracking issue: anthropics/claude-code-action#892
Related: #852, #880, #804

Will revert to @v1 when the upstream issue is resolved.
dimaMachina pushed a commit that referenced this pull request Feb 5, 2026
SDK versions 0.2.27+ have a bug causing AJV validation crashes
before any API calls are made. This affects all PR reviews.

Tracking issue: anthropics/claude-code-action#892
Related: #852, #880, #804

Will revert to @v1 when the upstream issue is resolved.
dimaMachina added a commit that referenced this pull request Feb 5, 2026
…chat or if custom headers are invalid (#1699)

* upd

* upd

* upd

* upd

* upd

* upd

* upd

* typecheck is ok now

* wip custom headers dialog

* upddd

* upddd

* upddd

* brand color

* upd

* validate on mount

* polish

* fix lint

* format

* review fixes

* feat(pr-review): add clickable links to inline comments in review summary (#1714)

- Add `url` field to GraphQL queries for review threads and PR comments
- Add Phase 5.4 to capture inline comment URLs after posting
- Update Point-Fix Edits section to include clickable links
- Update Pending Recommendations to use URLs from pr-context skill
- Add `gh api` to allowed tools for fetching comment URLs
- Add secure debug artifact uploads for Claude review runs

* Revert "fix(agents-core): remove refine call in resource id schema (#1689)" (#1691)

This reverts commit 938ffb8.

* fix: pin claude-code-action to SDK 0.2.25 to avoid AJV crash (#1716)

SDK versions 0.2.27+ have a bug causing AJV validation crashes
before any API calls are made. This affects all PR reviews.

Tracking issue: anthropics/claude-code-action#892
Related: #852, #880, #804

Will revert to @v1 when the upstream issue is resolved.

* bump zod to latest 4.3.6 and fix `.omit() cannot be used on object schemas containing refinements` error (#1712)

* Revert "fix(agents-core): remove refine call in resource id schema (#1689)"

This reverts commit 938ffb8.

* Revert "fix(agents-core): remove refine call in resource id schema (#1689)"

This reverts commit 938ffb8.

* bump

* upd

* remove zod from pnpm overrides

* update zod peerdependencies too, and we have error reproducible locally

* minimal fix

* rm unrelated changes

* rm unrelated changes

* rm unrelated changes

* rm unrelated changes

* rm unrelated changes

* rm unrelated changes

* rm unrelated changes

* rm unrelated changes

* Version Packages (#1701)

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

* feat: add Vercel production deployment workflow (#1679)

* feat: add Vercel production deployment workflow

Add GitHub Actions workflow to deploy to Vercel production only when
a GitHub release is published. This integrates with the existing
changesets release flow.

* feat: add deployment checks before promoting to production

Deploy to preview URL first, wait for Vercel deployment checks to pass,
then promote to production. This ensures API health before going live.

* PRD for vercel deployment strategy

* docs: add Vercel staging/production deployment strategy documentation

- Document Production Branch configuration (set to '_disabled_')
- Document GitHub Actions workflow for release-triggered deployments
- Document required secrets (VERCEL_TOKEN, VERCEL_ORG_ID, VERCEL_PROJECT_ID)
- Document optional staging domain configuration
- Add deployment flow diagram and troubleshooting section
- Add secrets documentation comments to workflow file

Completes US-001, US-002, US-003, US-004 from vercel-deployment-strategy PRD.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* docs: update Vercel staging/production strategy to use never-deploy branch

Replace _disabled_ approach (which Vercel doesn't support) with a
never-deploy orphan branch. Update staging domain examples to use
api-staging.agents.yourdomain.com pattern.

* Apply suggestion from @claude[bot]

Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com>

* revert: remove Vercel deployment docs changes and PRD

Revert vercel.mdx to main branch version and remove the PRD file.

* feat: deploy both agents-api and agents-manage-ui to Vercel

Update production workflow to deploy both projects in parallel using a
matrix strategy. Each project uses its own secret for the Vercel project ID.

---------

Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com>

* remove cursor specific rules in favor or skills and agents.md (#1717)

* chore: trigger release for all packages (#1718)

No-op patch bumps to trigger a new release.

* Version Packages (#1719)

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

* updating release action (#1720)

* fix: add --scope flag to Vercel CLI commands in production workflow (#1721)

The vercel inspect and promote commands were failing with authorization
errors because VERCEL_ORG_ID as an environment variable isn't used by
the CLI for scope resolution - it requires the --scope flag explicitly.

* fix: use staged production deployments in Vercel workflow (#1722)

- Deploy with --prod --skip-domain to create staged production builds
- This ensures production env vars are used and avoids rebuild on promote
- Add --yes flag to auto-confirm prompts in CI environment
- Fixes issue where promoting preview deployments triggered interactive prompt

* fix: add --archive=tgz to prevent CLI hanging during deploy (#1724)

Large file uploads can cause the Vercel CLI to hang. The --archive=tgz
flag compresses files before upload which resolves this issue.

* fix: use repository variables instead of secrets for non-sensitive values (#1723)

Move TURBO_TEAM and VERCEL_ORG_ID from secrets to vars to prevent
GitHub Actions from masking these values in logs. Secret values are
automatically masked, which was causing "inkeep" to appear as "***"
throughout CI logs.

* fix: simplify Vercel workflow to use direct production deploy (#1725)

- Remove --skip-domain flag which was causing CLI to hang
- Remove separate promote step (--prod auto-assigns domains)
- Simpler, more reliable workflow

* fix: use secrets for VERCEL_ORG_ID (#1726)

* apply review

* pnpm i

* polish

* format

* Rename convert-json-schema-to-zod.ts to convert-json-schema-to-zod.test.ts

* add tests

* wip tests

* wip tests

* wip tests

* upd

* upd

* upd

* upd

* polish error names

* upd

* move to __tests__

* format

* chore: add changeset for custom headers validation feature

Co-authored-by: Dimitri POSTOLOV <undefined@users.noreply.github.com>

---------

Co-authored-by: Nick Gomez <122398915+nick-inkeep@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Andrew Mikofalvy <5668128+amikofalvy@users.noreply.github.com>
Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com>
Co-authored-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com>
Co-authored-by: Dimitri POSTOLOV <undefined@users.noreply.github.com>
dimaMachina added a commit that referenced this pull request Feb 5, 2026
…fromJSONSchema()` method (#1735)

* upd

* upd

* upd

* upd

* upd

* upd

* upd

* typecheck is ok now

* wip custom headers dialog

* upddd

* upddd

* upddd

* brand color

* upd

* validate on mount

* polish

* fix lint

* format

* review fixes

* feat(pr-review): add clickable links to inline comments in review summary (#1714)

- Add `url` field to GraphQL queries for review threads and PR comments
- Add Phase 5.4 to capture inline comment URLs after posting
- Update Point-Fix Edits section to include clickable links
- Update Pending Recommendations to use URLs from pr-context skill
- Add `gh api` to allowed tools for fetching comment URLs
- Add secure debug artifact uploads for Claude review runs

* Revert "fix(agents-core): remove refine call in resource id schema (#1689)" (#1691)

This reverts commit 938ffb8.

* fix: pin claude-code-action to SDK 0.2.25 to avoid AJV crash (#1716)

SDK versions 0.2.27+ have a bug causing AJV validation crashes
before any API calls are made. This affects all PR reviews.

Tracking issue: anthropics/claude-code-action#892
Related: #852, #880, #804

Will revert to @v1 when the upstream issue is resolved.

* bump zod to latest 4.3.6 and fix `.omit() cannot be used on object schemas containing refinements` error (#1712)

* Revert "fix(agents-core): remove refine call in resource id schema (#1689)"

This reverts commit 938ffb8.

* Revert "fix(agents-core): remove refine call in resource id schema (#1689)"

This reverts commit 938ffb8.

* bump

* upd

* remove zod from pnpm overrides

* update zod peerdependencies too, and we have error reproducible locally

* minimal fix

* rm unrelated changes

* rm unrelated changes

* rm unrelated changes

* rm unrelated changes

* rm unrelated changes

* rm unrelated changes

* rm unrelated changes

* rm unrelated changes

* Version Packages (#1701)

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

* feat: add Vercel production deployment workflow (#1679)

* feat: add Vercel production deployment workflow

Add GitHub Actions workflow to deploy to Vercel production only when
a GitHub release is published. This integrates with the existing
changesets release flow.

* feat: add deployment checks before promoting to production

Deploy to preview URL first, wait for Vercel deployment checks to pass,
then promote to production. This ensures API health before going live.

* PRD for vercel deployment strategy

* docs: add Vercel staging/production deployment strategy documentation

- Document Production Branch configuration (set to '_disabled_')
- Document GitHub Actions workflow for release-triggered deployments
- Document required secrets (VERCEL_TOKEN, VERCEL_ORG_ID, VERCEL_PROJECT_ID)
- Document optional staging domain configuration
- Add deployment flow diagram and troubleshooting section
- Add secrets documentation comments to workflow file

Completes US-001, US-002, US-003, US-004 from vercel-deployment-strategy PRD.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* docs: update Vercel staging/production strategy to use never-deploy branch

Replace _disabled_ approach (which Vercel doesn't support) with a
never-deploy orphan branch. Update staging domain examples to use
api-staging.agents.yourdomain.com pattern.

* Apply suggestion from @claude[bot]

Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com>

* revert: remove Vercel deployment docs changes and PRD

Revert vercel.mdx to main branch version and remove the PRD file.

* feat: deploy both agents-api and agents-manage-ui to Vercel

Update production workflow to deploy both projects in parallel using a
matrix strategy. Each project uses its own secret for the Vercel project ID.

---------

Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com>

* remove cursor specific rules in favor or skills and agents.md (#1717)

* chore: trigger release for all packages (#1718)

No-op patch bumps to trigger a new release.

* Version Packages (#1719)

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

* updating release action (#1720)

* fix: add --scope flag to Vercel CLI commands in production workflow (#1721)

The vercel inspect and promote commands were failing with authorization
errors because VERCEL_ORG_ID as an environment variable isn't used by
the CLI for scope resolution - it requires the --scope flag explicitly.

* fix: use staged production deployments in Vercel workflow (#1722)

- Deploy with --prod --skip-domain to create staged production builds
- This ensures production env vars are used and avoids rebuild on promote
- Add --yes flag to auto-confirm prompts in CI environment
- Fixes issue where promoting preview deployments triggered interactive prompt

* fix: add --archive=tgz to prevent CLI hanging during deploy (#1724)

Large file uploads can cause the Vercel CLI to hang. The --archive=tgz
flag compresses files before upload which resolves this issue.

* fix: use repository variables instead of secrets for non-sensitive values (#1723)

Move TURBO_TEAM and VERCEL_ORG_ID from secrets to vars to prevent
GitHub Actions from masking these values in logs. Secret values are
automatically masked, which was causing "inkeep" to appear as "***"
throughout CI logs.

* fix: simplify Vercel workflow to use direct production deploy (#1725)

- Remove --skip-domain flag which was causing CLI to hang
- Remove separate promote step (--prod auto-assigns domains)
- Simpler, more reliable workflow

* fix: use secrets for VERCEL_ORG_ID (#1726)

* apply review

* pnpm i

* polish

* format

* Rename convert-json-schema-to-zod.ts to convert-json-schema-to-zod.test.ts

* add tests

* wip tests

* wip tests

* wip tests

* upd

* upd

* upd

* upd

* polish error names

* upd

* move to __tests__

* rm jsonSchemaToZod

* rm jsonSchemaToZod

* rm jsonSchemaToZod

* rm jsonSchemaToZod

* format

* chore: add changeset for custom headers validation feature

Co-authored-by: Dimitri POSTOLOV <undefined@users.noreply.github.com>

* chore: add changeset for jsonSchemaToZod removal

Replace custom implementation with Zod's native z.fromJSONSchema() method

Co-authored-by: Dimitri POSTOLOV <dimaMachina@users.noreply.github.com>

* fix lint

* Update validation.test.ts

---------

Co-authored-by: Nick Gomez <122398915+nick-inkeep@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Andrew Mikofalvy <5668128+amikofalvy@users.noreply.github.com>
Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com>
Co-authored-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com>
Co-authored-by: Dimitri POSTOLOV <undefined@users.noreply.github.com>
Co-authored-by: Dimitri POSTOLOV <dimaMachina@users.noreply.github.com>
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