feat: various improvements to database commands#8200
Conversation
📝 WalkthroughSummary by CodeRabbitRelease Notes
WalkthroughReworks CLI database output: consolidates migration info under a "Migrations" section that shows the migrations directory (project-relative when possible), then "Applied" and "Pending" subsections with counts, status-dependent emojis, and updated hints (run Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
📊 Benchmark resultsComparing with 0bc12bd
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/commands/database/db-status.ts (1)
319-328: Consider consolidating consecutive blank lines.Lines 321 and 323 both emit blank lines. If two blank lines are intentional for visual separation, this is fine. Otherwise, one could be removed.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/commands/database/db-status.ts` around lines 319 - 328, The code emits two consecutive blank lines via two log('') calls before and after the migrations header, which is redundant; remove one of the consecutive log('') calls (the extra blank line) so the block that prints the migrations section (the log calls around STATUS_INFO, INDENT, migrationsDirectory, projectRoot, relativePath, displayPath) has only a single blank line for separation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/commands/database/db-status.ts`:
- Around line 1-2: Run the project's formatter on the file to fix CI Prettier
violations: execute the configured format command (e.g., npx prettier --write
src/commands/database/db-status.ts), stage and commit the resulting changes;
ensure the import lines (import { readFile, readdir } from 'fs/promises' and
import { join, relative } from 'path') and surrounding whitespace are adjusted
to match project Prettier rules so the CI formatting check passes.
- Around line 242-244: STATUS_INFO has a trailing space causing double spacing
when rendered; remove the trailing space from the STATUS_INFO constant (change
STATUS_INFO = 'ℹ️ ' to 'ℹ️') so spacing is consistently handled by the primary()
formatting function (primary(…, emoji, text) which already inserts a space), and
scan other status constants (e.g., STATUS_NONE) to ensure none include extra
spaces.
---
Nitpick comments:
In `@src/commands/database/db-status.ts`:
- Around line 319-328: The code emits two consecutive blank lines via two
log('') calls before and after the migrations header, which is redundant; remove
one of the consecutive log('') calls (the extra blank line) so the block that
prints the migrations section (the log calls around STATUS_INFO, INDENT,
migrationsDirectory, projectRoot, relativePath, displayPath) has only a single
blank line for separation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d156e5f7-7b2a-46bb-b057-990ef277df31
📒 Files selected for processing (2)
src/commands/database/db-status.tstests/unit/commands/database/db-status.test.ts
| const STATUS_NONE = '⚪' | ||
| const STATUS_INFO = 'ℹ️ ' | ||
|
|
There was a problem hiding this comment.
Trailing space in STATUS_INFO causes double spacing.
STATUS_INFO includes a trailing space, but primary() on line 245-246 already adds a space between the emoji and text (\ ${emoji} ${text}`). This results in double spacing when STATUS_INFO` is used (e.g., lines 309, 326).
🔧 Suggested fix
const STATUS_NONE = '⚪'
-const STATUS_INFO = 'ℹ️ '
+const STATUS_INFO = 'ℹ️'📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const STATUS_NONE = '⚪' | |
| const STATUS_INFO = 'ℹ️ ' | |
| const STATUS_NONE = '⚪' | |
| const STATUS_INFO = 'ℹ️' | |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/commands/database/db-status.ts` around lines 242 - 244, STATUS_INFO has a
trailing space causing double spacing when rendered; remove the trailing space
from the STATUS_INFO constant (change STATUS_INFO = 'ℹ️ ' to 'ℹ️') so spacing is
consistently handled by the primary() formatting function (primary(…, emoji,
text) which already inserts a space), and scan other status constants (e.g.,
STATUS_NONE) to ensure none include extra spaces.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/commands/database/db-migration-pull.ts`:
- Around line 140-149: The current path traversal guard in the resolvedPaths
mapping (inside db-migration-pull.ts) uses
filePath.startsWith(canonicalMigrationsDir) which can be bypassed by sibling
prefixes; change the check to verify the resolved path is truly inside
canonicalMigrationsDir by computing the relative path (e.g.,
path.relative(canonicalMigrationsDir, filePath)) and ensuring it does not start
with '..' or contain path separators that move up, and keep the existing
isAbsolute and '..' segment checks; update the mapping that produces filePath so
the new validation uses canonicalMigrationsDir, filePath, resolve, and the
relative-path-based check instead of startsWith.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 6780400f-c22b-411c-8d31-bbe3404ef279
📒 Files selected for processing (6)
src/commands/database/db-migration-pull.tssrc/commands/database/db-status.tssrc/commands/database/util/api-errors.tssrc/commands/database/util/applied-migrations.tstests/unit/commands/database/db-migration-pull.test.tstests/unit/commands/database/util/api-errors.test.ts
| const resolvedPaths = migrations.map((migration) => { | ||
| if (isAbsolute(migration.path) || migration.path.split(/[/\\]/).includes('..')) { | ||
| throw new Error(`Migration path "${migration.path}" contains invalid path segments.`) | ||
| } | ||
| const filePath = resolve(canonicalMigrationsDir, migration.path) | ||
| if (!filePath.startsWith(canonicalMigrationsDir)) { | ||
| throw new Error(`Migration path "${migration.path}" resolves outside the migrations directory.`) | ||
| } | ||
| return filePath | ||
| }) |
There was a problem hiding this comment.
Path traversal check may allow escapes to sibling directories.
The startsWith check on line 145 can be bypassed when the resolved path shares a common prefix but escapes to a sibling directory. For example, if canonicalMigrationsDir is /project/migrations and filePath resolves to /project/migrations-evil/file.sql, the check passes because the string starts with /project/migrations.
🛡️ Proposed fix to ensure path separator boundary
const resolvedPaths = migrations.map((migration) => {
if (isAbsolute(migration.path) || migration.path.split(/[/\\]/).includes('..')) {
throw new Error(`Migration path "${migration.path}" contains invalid path segments.`)
}
const filePath = resolve(canonicalMigrationsDir, migration.path)
- if (!filePath.startsWith(canonicalMigrationsDir)) {
+ if (!filePath.startsWith(canonicalMigrationsDir + '/') && filePath !== canonicalMigrationsDir) {
throw new Error(`Migration path "${migration.path}" resolves outside the migrations directory.`)
}
return filePath
})🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/commands/database/db-migration-pull.ts` around lines 140 - 149, The
current path traversal guard in the resolvedPaths mapping (inside
db-migration-pull.ts) uses filePath.startsWith(canonicalMigrationsDir) which can
be bypassed by sibling prefixes; change the check to verify the resolved path is
truly inside canonicalMigrationsDir by computing the relative path (e.g.,
path.relative(canonicalMigrationsDir, filePath)) and ensuring it does not start
with '..' or contain path separators that move up, and keep the existing
isAbsolute and '..' segment checks; update the mapping that produces filePath so
the new validation uses canonicalMigrationsDir, filePath, resolve, and the
relative-path-based check instead of startsWith.
eduardoboucas
changed the title
database statusdatabase commands
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/commands/database/db-status.ts (1)
243-244:⚠️ Potential issue | 🟡 MinorTrailing space in
STATUS_INFOcauses double spacing.
STATUS_INFOincludes a trailing space ('ℹ️ '), but theprimary()function (line 247) already inserts a space between the emoji and text via the template literal` ${emoji} ${text}`. This results in double spacing whenSTATUS_INFOis used (e.g., lines 310, 328).🔧 Suggested fix
const STATUS_NONE = '⚪' -const STATUS_INFO = 'ℹ️ ' +const STATUS_INFO = 'ℹ️'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/commands/database/db-status.ts` around lines 243 - 244, STATUS_INFO has a trailing space causing double spacing because primary() already adds a space; remove the trailing space from the STATUS_INFO constant (change 'ℹ️ ' to 'ℹ️') or alternatively stop adding the extra space in the template inside primary(), ensuring occurrences that reference STATUS_INFO (e.g., lines that use STATUS_INFO in db-status.ts) render with a single space between emoji and text.
🧹 Nitpick comments (1)
src/commands/database/db-status.ts (1)
319-324: Consecutive empty lines between header and content.Lines 322 and 324 both output empty lines, creating double spacing between the "Migrations" header and the directory section. This is likely unintentional.
🧹 Remove duplicate empty line
log(chalk.bold('Migrations')) log(chalk.gray('Database migrations managed by Netlify')) log('') - - log('') const relativePath = relative(projectRoot, migrationsDirectory)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/commands/database/db-status.ts` around lines 319 - 324, Remove the duplicate blank line after the "Migrations" header by deleting one of the consecutive log('') calls in src/commands/database/db-status.ts (the two log('') calls surrounding the chalk.bold('Migrations') / chalk.gray('Database migrations managed by Netlify') block); leave a single log('') to keep one blank line between the header and the following content.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/commands/database/db-status.ts`:
- Around line 243-244: STATUS_INFO has a trailing space causing double spacing
because primary() already adds a space; remove the trailing space from the
STATUS_INFO constant (change 'ℹ️ ' to 'ℹ️') or alternatively stop adding the
extra space in the template inside primary(), ensuring occurrences that
reference STATUS_INFO (e.g., lines that use STATUS_INFO in db-status.ts) render
with a single space between emoji and text.
---
Nitpick comments:
In `@src/commands/database/db-status.ts`:
- Around line 319-324: Remove the duplicate blank line after the "Migrations"
header by deleting one of the consecutive log('') calls in
src/commands/database/db-status.ts (the two log('') calls surrounding the
chalk.bold('Migrations') / chalk.gray('Database migrations managed by Netlify')
block); leave a single log('') to keep one blank line between the header and the
following content.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 55a66b85-2008-48c4-a607-22182b38b40c
📒 Files selected for processing (1)
src/commands/database/db-status.ts
| if (canApplyLocally) { | ||
| log(chalk.gray(`${INDENT}Run ${formatCommand('database migrations apply')} to apply them to the local database.`)) | ||
| } else { | ||
| log(chalk.gray(`${INDENT}Deploy these files to apply the migrations.`)) |
There was a problem hiding this comment.
NIT: the wording might be confusing and what is meant by "deploy these files"
Not great but maybe:
| log(chalk.gray(`${INDENT}Deploy these files to apply the migrations.`)) | |
| log(chalk.gray(`${INDENT}Deploy your site to apply the migrations.`)) |
Also feel free to skip.
In any case - if any changes were applied, tests that look for this string would need to be updated
🤖 I have created a release *beep* *boop* --- ## [25.4.0](v25.3.0...v25.4.0) (2026-04-23) ### Features * show migrations directory path in `database status` ([#8187](#8187)) ([a378a02](a378a02)) * various improvements to `database` commands ([#8200](#8200)) ([2947a99](2947a99)) ### Bug Fixes * **deps:** update dependency @netlify/dev to v4.17.3 ([#8196](#8196)) ([0bc12bd](0bc12bd)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: token-generator-app[bot] <82042599+token-generator-app[bot]@users.noreply.github.com>
2 participants
fd77a17
Reworks the human-friendly output of
netlify database statusinto a single Migrations section (replacing the previous separate "Applied migrations" and "Migrations not applied" sections).Also fixed the apply-guidance: the
netlify database migrations applyhint only appears when truly targeting the local dev DB, and switches to "Deploy these files to apply the migrations." when using a remote database.The
--jsonoutput is now affected.60c162d
Fixed
netlify database migrations pullto match the server's new split API. The list endpoint (/database/migrations) is now metadata-only and a new per-migration endpoint (/database/migrations/{name}) returns the SQL content, so the command now makes a list call followed by parallel content fetches.9bb834f
Added a shared
readApiErrorMessagehelper used by every CLI call site that hits/database/*, which extracts the server's message field from JSON error bodies so users see clean messages instead of raw JSON.