From 1d0c3cfc8e843ffedef30ea9ffcbd1153c44726b Mon Sep 17 00:00:00 2001 From: Victor Ukam Date: Wed, 5 Aug 2026 08:53:58 +0100 Subject: [PATCH 1/3] fix: harden PromptManager against tracking failures and unsafe input MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Tracking defaulted to on whenever APP_DEBUG was false, but its tables are published in a separate opt-in step. The natural production install therefore threw "no such table: prompt_versions" on the first Deck::get(). Caching gave no protection, because the active version is resolved before the cache is consulted, and prompt:activate failed the same way. Tracking now defaults to off. Every database interaction is guarded and degrades to metadata.json with a single logged warning: - The connection is resolved through a latched helper, so a broken database is attempted once per instance rather than once per prompt load. An unreachable host would otherwise cost a connect timeout on every render, which is worse than the fast crash it replaces. Throwable is caught there because an undefined DECK_DB_CONNECTION raises InvalidArgumentException from the connection resolver, not QueryException. - track() catches Throwable and never rethrows. It runs after a completed, paid-for AI call, and no analytics failure is worth discarding that. - activate() degrades only when the table is genuinely absent, and rethrows anything else. Swallowing a deadlock or constraint violation would write metadata.json, return true, and leave the database — which getActiveVersion() prefers — still pointing at the previous version. activate() also now upserts rather than issuing an UPDATE that matched no rows because nothing ever inserted them. The table was permanently empty, so the lookup that crashed installs could fail but never succeed. Recording a version requires user_prompt to be nullable, since Deck keeps content on disk. Also fixes two unrelated defects in the same class: - Prompt names were interpolated into filesystem paths unvalidated, so a name containing '..' or a separator could read any file on disk. - The version directory pattern was unanchored and applied to the full path, so rev2, dev3 and archive-v9 registered as versions 2, 3 and 9 — listed by prompt:list --all, then failing to load. PromptManager and make:prompt must agree, so both are anchored against the directory name. --- config/deck.php | 12 +- src/Console/Commands/MakePromptCommand.php | 5 +- ...5_02_28_0000001_create_prompt_versions.php | 2 +- ..._make_prompt_versions_content_nullable.php | 34 +++ src/Exceptions/InvalidPromptNameException.php | 24 ++ src/PromptManager.php | 237 ++++++++++++++---- tests/Feature/DeckServiceProviderTest.php | 22 +- tests/Feature/MigrationTest.php | 18 ++ tests/Unit/PromptManagerTest.php | 229 +++++++++++++++++ 9 files changed, 534 insertions(+), 49 deletions(-) create mode 100644 src/Database/migrations/2026_08_04_000001_make_prompt_versions_content_nullable.php create mode 100644 src/Exceptions/InvalidPromptNameException.php diff --git a/config/deck.php b/config/deck.php index 40a7d54..6a64136 100644 --- a/config/deck.php +++ b/config/deck.php @@ -57,9 +57,19 @@ | If enabled, prompt versions and executions will be logged to the database, | enabling performance tracking and audit trails. | + | Tracking is off by default because it requires the package migrations to + | have been published and run. Prompt rendering never depends on it: if the + | tables are missing or the database is unreachable, Deck falls back to + | metadata.json and logs a warning rather than failing. + | + | To enable it: + | 1. php artisan vendor:publish --tag=deck-migrations + | 2. php artisan migrate + | 3. DECK_TRACKING_ENABLED=true + | */ 'tracking' => [ - 'enabled' => env('DECK_TRACKING_ENABLED', env('APP_DEBUG', false) ? false : true), + 'enabled' => env('DECK_TRACKING_ENABLED', false), 'connection' => env('DECK_DB_CONNECTION'), // null for default ], diff --git a/src/Console/Commands/MakePromptCommand.php b/src/Console/Commands/MakePromptCommand.php index 3a55c06..584219c 100644 --- a/src/Console/Commands/MakePromptCommand.php +++ b/src/Console/Commands/MakePromptCommand.php @@ -209,7 +209,10 @@ protected function detectLatestVersion(string $promptPath): int $latest = 0; foreach ($this->files->directories($promptPath) as $dir) { - if (preg_match('/v(\d+)$/', $dir, $matches)) { + // Anchored against the directory name alone, matching + // PromptManager::versions(). The two must agree on what counts as + // a version, or the command scaffolds over an existing one. + if (preg_match('/^v(\d+)$/', basename($dir), $matches)) { $latest = max($latest, (int) $matches[1]); } } diff --git a/src/Database/migrations/2025_02_28_0000001_create_prompt_versions.php b/src/Database/migrations/2025_02_28_0000001_create_prompt_versions.php index acd57ed..966d515 100644 --- a/src/Database/migrations/2025_02_28_0000001_create_prompt_versions.php +++ b/src/Database/migrations/2025_02_28_0000001_create_prompt_versions.php @@ -15,7 +15,7 @@ public function up() $table->string('name'); $table->unsignedInteger('version'); $table->text('system_prompt')->nullable(); - $table->text('user_prompt'); + $table->text('user_prompt')->nullable(); $table->json('metadata')->nullable(); $table->boolean('is_active')->default(false); $table->timestamps(); diff --git a/src/Database/migrations/2026_08_04_000001_make_prompt_versions_content_nullable.php b/src/Database/migrations/2026_08_04_000001_make_prompt_versions_content_nullable.php new file mode 100644 index 0000000..3485478 --- /dev/null +++ b/src/Database/migrations/2026_08_04_000001_make_prompt_versions_content_nullable.php @@ -0,0 +1,34 @@ +text('user_prompt')->nullable()->change(); + }); + } + + public function down(): void + { + // Rows written by activate() carry no content, so they must go before + // the column can be NOT NULL again. + Schema::table('prompt_versions', function (Blueprint $table) { + $table->text('user_prompt')->nullable(false)->default('')->change(); + }); + } +}; diff --git a/src/Exceptions/InvalidPromptNameException.php b/src/Exceptions/InvalidPromptNameException.php new file mode 100644 index 0000000..7e81a08 --- /dev/null +++ b/src/Exceptions/InvalidPromptNameException.php @@ -0,0 +1,24 @@ +files = new Filesystem; @@ -50,6 +78,8 @@ public function __construct(string $basePath, string $extension, Cache $cache, C */ public function get(string $name, string|int|null $version = null): PromptTemplate { + $this->assertValidName($name); + if ($version === null) { $version = $this->getActiveVersion($name); } else { @@ -94,6 +124,8 @@ public function get(string $name, string|int|null $version = null): PromptTempla */ public function active(string $name): PromptTemplate { + $this->assertValidName($name); + return $this->get($name, $this->getActiveVersion($name)); } @@ -102,6 +134,8 @@ public function active(string $name): PromptTemplate */ public function versions(string $name): array { + $this->assertValidName($name); + $promptPath = "{$this->basePath}/{$name}"; if (! $this->files->isDirectory($promptPath)) { @@ -110,11 +144,14 @@ public function versions(string $name): array $versions = []; - // Scan for version directories (v1, v2, etc.) or version files. + // Scan for version directories (v1, v2, etc.). $items = $this->files->directories($promptPath); foreach ($items as $dir) { - if (preg_match('/v(\d+)$/', $dir, $matches)) { + // Anchored against the directory name alone: an unanchored match + // on the full path treats 'rev2', 'dev3' and 'archive-v9' as + // versions 2, 3 and 9. + if (preg_match('/^v(\d+)$/', basename($dir), $matches)) { $version = (int) $matches[1]; $versions[] = [ 'version' => $version, @@ -146,29 +183,44 @@ public function versions(string $name): array */ public function activate(string $name, string|int $version): bool { + $this->assertValidName($name); + $version = $this->parseVersion((string) $version) ?? throw InvalidVersionException::unparseable($name, $version); $this->ensureVersionExists($name, $version); - if ($this->trackingConfig['enabled'] ?? false) { - $connection = DB::connection( - $this->trackingConfig['connection'] ?? config('database.default') - ); - - // Update database. - $connection->transaction(function () use ($connection, $name, $version): void { - $connection - ->table('prompt_versions') - ->where('name', $name) - ->update(['is_active' => false]); - - $connection - ->table('prompt_versions') - ->where('name', $name) - ->where('version', $version) - ->update(['is_active' => true]); - }); + if ($connection = $this->trackingConnection()) { + try { + $connection->transaction(function () use ($connection, $name, $version): void { + // Deactivate every other version of this prompt. + $connection + ->table('prompt_versions') + ->where('name', $name) + ->update(['is_active' => false]); + + // Record this one, inserting it if it has not been activated + // before. Timestamps are passed explicitly: this is a query + // builder call, so Eloquent does not maintain them. + $connection + ->table('prompt_versions') + ->updateOrInsert( + ['name' => $name, 'version' => $version], + ['is_active' => true, 'updated_at' => now(), 'created_at' => now()], + ); + }); + } catch (QueryException $e) { + // Only a missing table is a degradable condition. A deadlock, a + // constraint violation from concurrent activations, or a full + // disk must surface: swallowing them would write metadata.json, + // return true, and leave the database — which getActiveVersion() + // prefers — still pointing at the previous version. + if (! $this->trackingTableMissing($connection, 'prompt_versions')) { + throw $e; + } + + $this->markTrackingUnavailable($e); + } } // Fallback: store in a JSON file in the prompt directory. @@ -185,45 +237,68 @@ public function activate(string $name, string|int $version): bool /** * Track an execution for performance monitoring. + * + * Never throws. This runs after a completed — and paid for — AI call, so + * no analytics failure is worth destroying the response that triggered it. */ public function track(string $promptName, int $version, array $data): void { - if (! ($this->trackingConfig['enabled'] ?? false)) { + $this->assertValidName($promptName); + + if (! ($connection = $this->trackingConnection())) { return; } - DB::connection($this->trackingConfig['connection'] ?? config('database.default')) - ->table('prompt_executions') - ->insert([ - 'prompt_name' => $promptName, - 'prompt_version' => $version, - 'input' => json_encode($data['input'] ?? null), - 'output' => $data['output'] ?? null, - 'tokens' => $data['tokens'] ?? null, - 'latency_ms' => $data['latency'] ?? null, - 'cost' => $data['cost'] ?? null, - 'model' => $data['model'] ?? null, - 'provider' => $data['provider'] ?? null, - 'feedback' => isset($data['feedback']) ? json_encode($data['feedback']) : null, - 'created_at' => now(), - ]); + try { + $connection + ->table('prompt_executions') + ->insert([ + 'prompt_name' => $promptName, + 'prompt_version' => $version, + 'input' => json_encode($data['input'] ?? null), + 'output' => $data['output'] ?? null, + 'tokens' => $data['tokens'] ?? null, + 'latency_ms' => $data['latency'] ?? null, + 'cost' => $data['cost'] ?? null, + 'model' => $data['model'] ?? null, + 'provider' => $data['provider'] ?? null, + 'feedback' => isset($data['feedback']) ? json_encode($data['feedback']) : null, + 'created_at' => now(), + ]); + } catch (Throwable $e) { + // Deliberately broad: a missing table, an unreachable host, or a + // json_encode failure on non-UTF-8 input must all be swallowed. + $this->markTrackingUnavailable($e); + } } /** * Get the active version number for a prompt. + * + * The database wins when it holds a record, so a version activated at + * runtime takes precedence over the active_version committed alongside + * the prompt files. */ protected function getActiveVersion(string $name): int { // Check database first if tracking enabled. - if ($this->trackingConfig['enabled'] ?? false) { - $record = DB::connection($this->trackingConfig['connection'] ?? config('database.default')) - ->table('prompt_versions') - ->where('name', $name) - ->where('is_active', true) - ->first(); - - if ($record) { - return $record->version; + if ($connection = $this->trackingConnection()) { + try { + $record = $connection + ->table('prompt_versions') + ->where('name', $name) + ->where('is_active', true) + ->orderByDesc('version') + ->first(); + + if ($record) { + return (int) $record->version; + } + } catch (QueryException $e) { + // Rendering must survive any database problem, not only a + // missing table: serving the version on disk beats not serving + // at all. Latched, so this is attempted once per instance. + $this->markTrackingUnavailable($e); } } @@ -244,6 +319,78 @@ protected function getActiveVersion(string $name): int return max(array_column($versions, 'version')); } + /** + * Resolve the connection tracking should write to, or null when tracking + * is disabled or the database has already been found unusable. + * + * Catching Throwable is deliberate: an undefined DECK_DB_CONNECTION throws + * InvalidArgumentException from the connection resolver rather than a + * QueryException, and is a likely misconfiguration. + */ + protected function trackingConnection(): ?Connection + { + if (! ($this->trackingConfig['enabled'] ?? false) || $this->trackingUnavailable) { + return null; + } + + try { + return DB::connection($this->trackingConfig['connection'] ?? config('database.default')); + } catch (Throwable $e) { + $this->markTrackingUnavailable($e); + + return null; + } + } + + /** + * Record that tracking is unusable, warning once for this instance. + * + * The latch keeps a broken database from being retried on every prompt + * load, and doubles as the guard against flooding the log. + */ + protected function markTrackingUnavailable(Throwable $e): void + { + if ($this->trackingUnavailable) { + return; + } + + $this->trackingUnavailable = true; + + Log::warning( + 'Deck tracking is enabled but the tracking database is unavailable. ' + .'Prompt rendering has fallen back to metadata.json. Publish and run ' + .'the Deck migrations, or set DECK_TRACKING_ENABLED=false. ' + .$e->getMessage() + ); + } + + /** + * Determine whether a tracking table is genuinely absent, as opposed to + * present but erroring for some other reason. + */ + protected function trackingTableMissing(Connection $connection, string $table): bool + { + try { + return ! Schema::connection($connection->getName())->hasTable($table); + } catch (Throwable) { + // If the schema cannot be inspected either, the database is not in + // a usable state — treat it as absent so callers degrade. + return true; + } + } + + /** + * Ensure a prompt name is safe to interpolate into a filesystem path. + * + * @throws InvalidPromptNameException + */ + protected function assertValidName(string $name): void + { + if (preg_match(self::NAME_PATTERN, $name) !== 1) { + throw InvalidPromptNameException::named($name); + } + } + /** * Load prompt data from filesystem for a given name and version. * diff --git a/tests/Feature/DeckServiceProviderTest.php b/tests/Feature/DeckServiceProviderTest.php index c99f5d3..7bb1006 100644 --- a/tests/Feature/DeckServiceProviderTest.php +++ b/tests/Feature/DeckServiceProviderTest.php @@ -83,8 +83,14 @@ $source = array_key_first($publishes); + // Asserted by name rather than by count, so adding a migration does not + // break the guard. What matters is that the directory really resolves and + // really holds the table definitions. + $migrations = array_map('basename', glob(rtrim($source, '/').'/*.php')); + expect(is_dir($source))->toBeTrue() - ->and(glob(rtrim($source, '/').'/*.php'))->toHaveCount(2); + ->and($migrations)->toContain('2025_02_28_0000001_create_prompt_versions.php') + ->and($migrations)->toContain('2025_02_28_0000001_create_prompt_executions.php'); }); test('stubs are not included in default provider publishing', function () { @@ -97,3 +103,17 @@ expect($stubPaths)->toBeEmpty(); }); + +test('tracking defaults to off in the package config', function () { + // Tracking needs migrations that are published in a separate, opt-in step, + // so it must not default on: prompt rendering would depend on tables that + // may not exist. + // + // Asserted against the source rather than the evaluated value, because + // phpunit.xml pins DECK_TRACKING_ENABLED for the whole suite, which would + // mask the default either way. + $source = file_get_contents(__DIR__.'/../../config/deck.php'); + + expect($source)->toContain("'enabled' => env('DECK_TRACKING_ENABLED', false)") + ->and($source)->not->toContain("env('DECK_TRACKING_ENABLED', env('APP_DEBUG'"); +}); diff --git a/tests/Feature/MigrationTest.php b/tests/Feature/MigrationTest.php index 1d0bfe9..9be634d 100644 --- a/tests/Feature/MigrationTest.php +++ b/tests/Feature/MigrationTest.php @@ -557,3 +557,21 @@ function rollbackMigrations(): void expect($execution->provider)->toBeIn(['openai', 'anthropic']); }); + +test('prompt_versions user_prompt is nullable so activate() can record a version', function () { + runMigrations(); + + DB::connection('testing')->table('prompt_versions')->insert([ + 'name' => 'content-free', + 'version' => 1, + 'is_active' => true, + 'created_at' => now(), + 'updated_at' => now(), + ]); + + $record = DB::connection('testing')->table('prompt_versions') + ->where('name', 'content-free')->first(); + + expect($record->user_prompt)->toBeNull() + ->and($record->system_prompt)->toBeNull(); +}); diff --git a/tests/Unit/PromptManagerTest.php b/tests/Unit/PromptManagerTest.php index 139e330..ea4725e 100644 --- a/tests/Unit/PromptManagerTest.php +++ b/tests/Unit/PromptManagerTest.php @@ -3,6 +3,8 @@ declare(strict_types=1); use Illuminate\Support\Facades\DB; +use Illuminate\Support\Facades\Log; +use PromptPHP\Deck\Exceptions\InvalidPromptNameException; use PromptPHP\Deck\Exceptions\InvalidVersionException; use PromptPHP\Deck\Exceptions\PromptNotFoundException; use PromptPHP\Deck\PromptManager; @@ -641,3 +643,230 @@ function freshManager(?array $configOverrides = []): PromptManager ->and($manager->activate('parity', $input))->toBeTrue(); } }); + +// ===================================================================== +// Tracking enabled without its tables — must degrade, never throw +// ===================================================================== + +test('get() falls back to metadata.json when the tracking tables are absent', function () { + // Deliberately does NOT call setUpTrackingTables(). + config()->set('deck.tracking.enabled', true); + + $this->createPromptFixture('no-tables', 1, 'sys v1', null, null, ['active_version' => 1]); + $this->createPromptFixture('no-tables', 2, 'sys v2'); + + $prompt = freshManager()->get('no-tables'); + + expect($prompt->version())->toBe(1) + ->and($prompt->system())->toBe('sys v1'); +}); + +test('track() is a silent no-op when the tracking table is absent', function () { + config()->set('deck.tracking.enabled', true); + + freshManager()->track('anything', 1, ['output' => 'hello']); +})->throwsNoExceptions(); + +test('activate() still writes metadata.json when the tracking table is absent', function () { + config()->set('deck.tracking.enabled', true); + + $this->createPromptFixture('no-tables-activate', 1, 'sys'); + $this->createPromptFixture('no-tables-activate', 2, 'sys'); + + expect(freshManager()->activate('no-tables-activate', 2))->toBeTrue(); + + $meta = json_decode(file_get_contents("{$this->tempDir}/no-tables-activate/metadata.json"), true); + expect($meta['active_version'])->toBe(2); +}); + +test('an undefined tracking connection degrades instead of throwing', function () { + // Throws InvalidArgumentException from the connection resolver rather + // than a QueryException, so a QueryException-only guard would miss it. + config()->set('deck.tracking.enabled', true); + config()->set('deck.tracking.connection', 'does-not-exist'); + + $this->createPromptFixture('bad-connection', 1, 'sys', null, null, ['active_version' => 1]); + + expect(freshManager()->get('bad-connection')->version())->toBe(1); +}); + +test('an unusable tracking database is attempted once, not on every load', function () { + config()->set('deck.tracking.enabled', true); + Log::spy(); + + $this->createPromptFixture('latched', 1, 'sys', null, null, ['active_version' => 1]); + + $manager = freshManager(); + $manager->get('latched'); + $manager->get('latched'); + $manager->get('latched'); + + Log::shouldHaveReceived('warning')->once(); +}); + +// ===================================================================== +// Tracking enabled with its tables — activation is recorded +// ===================================================================== + +test('activate() inserts a prompt_versions row with timestamps', function () { + $this->setUpTrackingTables(); + config()->set('deck.tracking.enabled', true); + + $this->createPromptFixture('db-activate', 1, 'sys v1'); + $this->createPromptFixture('db-activate', 2, 'sys v2'); + + freshManager()->activate('db-activate', 2); + + $row = DB::connection('testing')->table('prompt_versions') + ->where('name', 'db-activate')->where('version', 2)->first(); + + expect($row)->not->toBeNull() + ->and((bool) $row->is_active)->toBeTrue() + ->and($row->created_at)->not->toBeNull() + ->and($row->updated_at)->not->toBeNull(); +}); + +test('getActiveVersion() reads the version back from the database', function () { + $this->setUpTrackingTables(); + config()->set('deck.tracking.enabled', true); + + $this->createPromptFixture('db-roundtrip', 1, 'sys v1'); + $this->createPromptFixture('db-roundtrip', 2, 'sys v2'); + + $manager = freshManager(); + $manager->activate('db-roundtrip', 2); + + expect($manager->get('db-roundtrip')->version())->toBe(2) + ->and($manager->get('db-roundtrip')->system())->toBe('sys v2'); +}); + +test('activate() deactivates sibling versions of the same prompt', function () { + $this->setUpTrackingTables(); + config()->set('deck.tracking.enabled', true); + + $this->createPromptFixture('siblings', 1, 'sys'); + $this->createPromptFixture('siblings', 2, 'sys'); + + $manager = freshManager(); + $manager->activate('siblings', 1); + $manager->activate('siblings', 2); + + $active = DB::connection('testing')->table('prompt_versions') + ->where('name', 'siblings')->where('is_active', true)->pluck('version')->all(); + + expect($active)->toBe([2]); +}); + +test('activating the same version twice updates rather than duplicating', function () { + $this->setUpTrackingTables(); + config()->set('deck.tracking.enabled', true); + + $this->createPromptFixture('twice', 1, 'sys'); + + $manager = freshManager(); + $manager->activate('twice', 1); + $manager->activate('twice', 1); + + $count = DB::connection('testing')->table('prompt_versions')->where('name', 'twice')->count(); + + expect($count)->toBe(1); +}); + +test('the database wins over metadata.json once a version has been activated', function () { + // Activation is environment state; the file is the bootstrap default. + $this->setUpTrackingTables(); + config()->set('deck.tracking.enabled', true); + + $this->createPromptFixture('precedence', 1, 'sys v1'); + $this->createPromptFixture('precedence', 2, 'sys v2'); + + $manager = freshManager(); + $manager->activate('precedence', 2); + + // Simulate someone editing active_version in git and deploying. + file_put_contents( + "{$this->tempDir}/precedence/metadata.json", + json_encode(['active_version' => 1]), + ); + + expect($manager->get('precedence')->version())->toBe(2); +}); + +test('get() with an explicit version performs no database query', function () { + $this->setUpTrackingTables(); + config()->set('deck.tracking.enabled', true); + + $this->createPromptFixture('hot-path', 2, 'sys v2'); + + $manager = freshManager(); + + DB::connection('testing')->enableQueryLog(); + $manager->get('hot-path', 2); + + expect(DB::connection('testing')->getQueryLog())->toBe([]); +}); + +// ===================================================================== +// Prompt name validation +// ===================================================================== + +test('get() rejects a traversing prompt name', function () { + freshManager()->get('../elsewhere', 1); +})->throws(InvalidPromptNameException::class); + +test('traversal cannot read a prompt outside the configured path', function () { + $outside = dirname($this->tempDir).'/deck-outside-'.uniqid(); + mkdir($outside.'/v1', 0755, true); + file_put_contents($outside.'/v1/system.md', 'SECRET'); + + try { + freshManager()->get('../'.basename($outside), 1); + $leaked = true; + } catch (InvalidPromptNameException) { + $leaked = false; + } finally { + unlink($outside.'/v1/system.md'); + rmdir($outside.'/v1'); + rmdir($outside); + } + + expect($leaked)->toBeFalse(); +}); + +test('name validation rejects separators and leading dots but allows ordinary names', function () { + $manager = freshManager(); + + foreach (['../secrets', '..', 'a/b', 'a\\b', '.hidden', ''] as $bad) { + expect(fn () => $manager->versions($bad))->toThrow(InvalidPromptNameException::class); + } + + // Kebab, snake, and dotted names must keep working. + foreach (['order-summary', 'order_summary', 'order.summary.v2', 'OrderSummary'] as $good) { + $this->createPromptFixture($good, 1, 'sys'); + expect($manager->versions($good))->toHaveCount(1); + } +}); + +test('activate() and track() reject invalid names too', function () { + $manager = freshManager(); + + expect(fn () => $manager->activate('../evil', 1))->toThrow(InvalidPromptNameException::class) + ->and(fn () => $manager->track('../evil', 1, []))->toThrow(InvalidPromptNameException::class); +}); + +// ===================================================================== +// Version directory detection +// ===================================================================== + +test('versions() ignores directories that merely end in a version-like suffix', function () { + $this->createPromptFixture('phantom', 1, 'sys'); + $this->createPromptFixture('phantom', 10, 'sys'); + + foreach (['rev2', 'dev3', 'archive-v9', 'drafts', 'v'] as $decoy) { + mkdir("{$this->tempDir}/phantom/{$decoy}", 0755, true); + } + + $found = array_column(freshManager()->versions('phantom'), 'version'); + + expect($found)->toBe([1, 10]); +}); From 4c61344af57f0691cf6842ab7e88b929cb5158fe Mon Sep 17 00:00:00 2001 From: Victor Ukam Date: Wed, 5 Aug 2026 08:54:16 +0100 Subject: [PATCH 2/3] docs: document the 0.4.6 tracking, naming and precedence changes Records that tracking is now opt-in and how to turn it on, that prompt rendering never depends on it, and the constraints on prompt names. Documents the behaviour change with the widest reach: now that activate() genuinely populates prompt_versions, the database takes precedence over metadata.json, so editing active_version in git and deploying no longer changes what is served. Activation is environment state; the file is the bootstrap default. Adds an UPGRADE section covering the default flip, the new migration, and that anyone who published config/deck.php is unaffected. --- CHANGELOG.md | 18 ++++++++++ UPGRADE.md | 47 ++++++++++++++++++++++-- docs/advanced/tracking.mdx | 50 +++++++++++++++----------- docs/changelog.mdx | 24 +++++++++++++ docs/core/prompts.mdx | 12 +++++++ docs/getting-started/configuration.mdx | 40 ++++++++++++++------- 6 files changed, 156 insertions(+), 35 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9a1182c..828071d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,24 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Removed +## [0.4.6] - 2026-08-04 + +### Fixed + +- Fixed tracking making prompt rendering fail. Tracking defaulted to on whenever `APP_DEBUG` was false, but its tables are published in a separate opt-in step, so the natural production install threw `no such table: prompt_versions` on the first `Deck::get()`. Caching gave no protection, because the active version is resolved before the cache is consulted. Tracking now defaults to off, and every database interaction degrades to `metadata.json` with a single logged warning rather than throwing. `Deck::track()` never throws at all — it runs after a completed, paid-for AI call. +- Fixed `Deck::activate()` never recording anything in `prompt_versions`. It only ever issued an `UPDATE`, which matched no rows because nothing inserted them, so the table stayed empty and the lookup in `getActiveVersion()` could fail but never succeed. It now upserts, with explicit timestamps, making runtime version switching work as documented. +- Fixed prompt names being interpolated into filesystem paths without validation. A name containing `..` or a directory separator could read files outside the configured prompts path. Names are now validated and `InvalidPromptNameException` is thrown otherwise. +- Fixed the version directory pattern matching far more than intended. Being unanchored and applied to the full path, `/v(\d+)$/` treated directories such as `rev2`, `dev3`, and `archive-v9` as versions 2, 3, and 9 — advertised by `prompt:list --all` and then failing to load. Both `PromptManager` and `make:prompt` now anchor the match against the directory name. +- Fixed `getActiveVersion()` returning the version column unordered and uncast, which picked arbitrarily between multiple active rows and could raise a `TypeError` on drivers that return integers as strings. + +### Changed + +- **Tracking now defaults to off.** If you published `config/deck.php` you are unaffected. If you did not and you rely on tracking, set `DECK_TRACKING_ENABLED=true`. See [UPGRADE.md](UPGRADE.md). +- `prompt_versions.user_prompt` is now nullable, so activation can be recorded without prompt content. Existing tracking installs must re-publish and run the migrations. +- Once a version has been activated with tracking enabled, the database takes precedence over `metadata.json`. Editing `active_version` in the file and deploying no longer changes what is served. Activation is environment state; the file is the bootstrap default. + +### Removed + ## [0.4.5] - 2026-08-03 ### Added diff --git a/UPGRADE.md b/UPGRADE.md index e25a65a..a004116 100644 --- a/UPGRADE.md +++ b/UPGRADE.md @@ -1,5 +1,50 @@ # Upgrade Guide +## Upgrading to `v0.4.6` + +`v0.4.6` is not a breaking release, but two changes need your attention if you use database tracking. + +### Tracking now defaults to off + +Tracking previously defaulted to **on** whenever `APP_DEBUG` was false, which meant a production deploy would enable it before its tables existed — and every prompt load then threw `no such table: prompt_versions`. It now defaults to off: + +```php +'enabled' => env('DECK_TRACKING_ENABLED', false), +``` + +**If you published `config/deck.php`**, your file is unchanged and so is your behaviour. Nothing to do. + +**If you did not publish it and you rely on tracking**, it will switch off silently on upgrade. Turn it back on explicitly: + +```dotenv +DECK_TRACKING_ENABLED=true +``` + +Deck no longer fails when tracking is enabled without its tables. It falls back to `metadata.json` and logs a warning once, so check your logs if tracking data stops appearing. + +### A new migration, if you use tracking + +`activate()` now records the active version in `prompt_versions` — previously it only ever ran an `UPDATE` that matched no rows, so the table was never populated. Inserting requires `user_prompt` to be nullable: + +```bash +php artisan vendor:publish --tag=deck-migrations +php artisan migrate +``` + +Skip this if you do not use tracking. + +### Activation precedence changed in practice + +Because the table is now genuinely populated, the documented rule that the database takes precedence over `metadata.json` starts to have an effect. After the first `Deck::activate()` in an environment, editing `active_version` in `metadata.json` and deploying will no longer change which version is served. Activation is environment state; the file is the bootstrap default. To return a prompt to file-based control, delete its rows from `prompt_versions`. + +### Prompt names are validated + +Names may contain letters, numbers, dots, dashes, and underscores, and may not begin with a dot. Anything else throws `InvalidPromptNameException`. Names generated by `make:prompt` have always satisfied this; only hand-created directories with unusual names are affected. + +--- + +## Upgrading from Prompt Deck `v0.3.x` to Deck `v0.4.0` + - [Update Composer](#update-composer) - [Update namespaces](#update-namespaces) - [Update Laravel AI SDK trait imports](#update-laravel-ai-sdk-trait-imports) @@ -8,8 +53,6 @@ - [Clear Laravel caches](#clear-laravel-caches) - [Database notes](#database-notes) -## Upgrading from Prompt Deck `v0.3.x` to Deck `v0.4.0` - Deck `v0.4.0` is a breaking release. The package has moved from `veeqtoh/prompt-deck` to `promptphp/deck`, and the PHP namespace has changed from `Veeqtoh\PromptDeck` to `PromptPHP\Deck`. diff --git a/docs/advanced/tracking.mdx b/docs/advanced/tracking.mdx index 6b50aba..a2781c8 100644 --- a/docs/advanced/tracking.mdx +++ b/docs/advanced/tracking.mdx @@ -14,36 +14,36 @@ Deck by PromptPHP includes an optional database tracking system that logs prompt - **Cost analysis** — Monitor API spending per prompt, model, and provider. - **User feedback** — Attach ratings and comments to individual executions. -Tracking is entirely optional. Deck works fully without it. +Tracking is entirely optional and **off by default**. Deck works fully without it. + + + Prompt rendering never depends on tracking. If you enable it without running the migrations, or the database becomes unreachable, Deck falls back to `metadata.json` and logs a warning once rather than failing. `Deck::track()` never throws under any circumstances — it runs after a completed, paid-for AI call. + ## Setup -### Enable tracking +Publish and run the migrations **before** enabling the flag, or tracking has no tables to write to. -Set the tracking configuration in `config/deck.php`: +### Publish and run the migrations -```php -'tracking' => [ - 'enabled' => env('DECK_TRACKING_ENABLED', true), - 'connection' => env('DECK_DB_CONNECTION'), -], +```bash +php artisan vendor:publish --tag=deck-migrations +php artisan migrate ``` -Or via environment variable: +### Enable tracking ```dotenv DECK_TRACKING_ENABLED=true ``` -By default, tracking is **disabled** when `APP_DEBUG=true` and **enabled** in production. - -### Publish migrations - -Publish and run the migrations to create the required tables: +Or set it directly in `config/deck.php`: -```bash -php artisan vendor:publish --tag=deck-migrations -php artisan migrate +```php +'tracking' => [ + 'enabled' => env('DECK_TRACKING_ENABLED', false), + 'connection' => env('DECK_DB_CONNECTION'), +], ``` ### Database connection @@ -54,7 +54,7 @@ By default, tracking uses your application's default database connection. To sto DECK_DB_CONNECTION=analytics ``` -The connection name must match a connection defined in `config/database.php`. +The connection name must match a connection defined in `config/database.php`. If it does not, Deck logs a warning and falls back to `metadata.json` rather than failing. ## Database schema @@ -381,9 +381,19 @@ $dailyCost = PromptExecution::where('prompt_name', 'order-summary') ## Version management via database -When tracking is enabled, version activation is managed through the `prompt_versions` table instead of `metadata.json` files. This provides a centralised, queryable record of version history. +When tracking is enabled, `Deck::activate()` and `prompt:activate` record the active version in the `prompt_versions` table **in addition to** `metadata.json`, giving you a centralised, queryable record of version history. -The `Deck::activate()` method and `prompt:activate` command automatically use the appropriate storage (database or file) based on your tracking configuration. +### Activation precedence + +Once a version has been activated with tracking enabled, **the database wins**. `metadata.json` is consulted only when the table holds no record for that prompt. + +The practical consequence: editing `active_version` in `metadata.json`, committing it, and deploying **will not change which version is served**. Activation is environment state — it is how you promote a version in staging without touching production, and how you roll back without a deploy. The file is the bootstrap default, used until the first activation in that environment. + +To go back to file-based control for a prompt, delete its rows from `prompt_versions`. + + + The table only ever holds versions that have been **activated**. It is not a mirror of your prompt library — a version you have created but never activated has no row. The `system_prompt` and `user_prompt` columns are nullable and are written as `NULL`: Deck is file-based, and content lives on disk. + ```php // Activate programmatically diff --git a/docs/changelog.mdx b/docs/changelog.mdx index acae31a..c280411 100644 --- a/docs/changelog.mdx +++ b/docs/changelog.mdx @@ -6,6 +6,30 @@ rss: true Deck follows [Semantic Versioning](https://semver.org/spec/v2.0.0.html). Subscribe to the [RSS feed](https://deck.promptphp.com/changelog/rss.xml) to be notified of new releases, or browse the [full history on GitHub](https://github.com/promptphp/deck/blob/0.x/CHANGELOG.md). + + + + Tracking now defaults to **off**. If you published `config/deck.php` you are unaffected. If you did not and you rely on tracking, set `DECK_TRACKING_ENABLED=true`. If you use tracking, re-publish and run the migrations. See the [upgrade guide](https://github.com/promptphp/deck/blob/0.x/UPGRADE.md). + + +**Fixed** + +- **Tracking no longer breaks prompt rendering.** Tracking defaulted to on whenever `APP_DEBUG` was false, but its tables are published in a separate opt-in step — so the natural production install (`composer require`, publish config, deploy) threw `no such table: prompt_versions` on the very first `Deck::get()`. Caching gave no protection, because the active version is resolved before the cache is consulted. + + Tracking now defaults to off, and every database interaction degrades to `metadata.json` with a single logged warning instead of throwing. `Deck::track()` never throws under any circumstances: it runs after a completed, paid-for AI call, and no analytics failure is worth discarding that response. + +- **`Deck::activate()` now records the version it activated.** It only ever issued an `UPDATE`, which matched no rows because nothing ever inserted them — so `prompt_versions` stayed empty and the lookup that read it could fail but never succeed. Activation is now an upsert, making runtime version switching work as documented. +- **Prompt names are validated.** A name was interpolated straight into a filesystem path, so one containing `..` or a directory separator could read files outside your prompts directory. Names now throw `InvalidPromptNameException` unless they contain only letters, numbers, dots, dashes, and underscores. +- **Version directories are matched precisely.** The pattern was unanchored and applied to the full path, so `rev2`, `dev3`, and `archive-v9` registered as versions 2, 3, and 9 — advertised by `prompt:list --all`, then failing to load. +- `getActiveVersion()` no longer picks arbitrarily between multiple active rows, and casts the version it reads. + +**Changed** + +- `prompt_versions.user_prompt` is nullable, so activation can be recorded without prompt content. Deck is file-based; content lives on disk. +- Once a version has been activated with tracking enabled, the database takes precedence over `metadata.json`. Editing `active_version` in the file and deploying no longer changes what is served — activation is environment state, the file is the bootstrap default. See [Tracking — Activation precedence](/advanced/tracking#activation-precedence). + + + **Added** diff --git a/docs/core/prompts.mdx b/docs/core/prompts.mdx index c0971fa..a3bfb04 100644 --- a/docs/core/prompts.mdx +++ b/docs/core/prompts.mdx @@ -39,6 +39,18 @@ $prompt = Deck::get('order-summary', 2); The `get` method returns a `PromptTemplate` instance. If no version is specified, the [active version](#version-resolution-order) is resolved automatically. +### Prompt names + +A prompt name becomes a directory path, so names are validated before they are resolved. They may contain letters, numbers, dots, dashes, and underscores, and may not begin with a dot. Anything else — most importantly a name containing `/`, `\`, or `..` — throws `InvalidPromptNameException`. + +```php +Deck::get('order-summary'); // fine +Deck::get('order_summary'); // fine +Deck::get('../../.env'); // InvalidPromptNameException +``` + +This matters if a prompt name ever comes from user input, where an unvalidated name could otherwise read files outside your prompts directory. + ### Dependency injection You can also inject the `PromptManager` directly via Laravel's service container: diff --git a/docs/getting-started/configuration.mdx b/docs/getting-started/configuration.mdx index 65118ac..a2e99b8 100644 --- a/docs/getting-started/configuration.mdx +++ b/docs/getting-started/configuration.mdx @@ -148,27 +148,41 @@ The `tracking` section controls whether prompt versions and executions are logge ```php 'tracking' => [ - 'enabled' => env('DECK_TRACKING_ENABLED', env('APP_DEBUG', false) ? false : true), + 'enabled' => env('DECK_TRACKING_ENABLED', false), 'connection' => env('DECK_DB_CONNECTION'), ], ``` ### Enabling / disabling -```php -'enabled' => env('DECK_TRACKING_ENABLED', env('APP_DEBUG', false) ? false : true), -``` - -Like caching, tracking is **disabled** in debug mode and **enabled** in production by default. When enabled: - -- **Version activation** is stored in the `prompt_versions` database table (instead of `metadata.json`). +Tracking is **off by default**, because it needs database tables that are published in a separate step. Turning it on takes three: + + + + ```bash + php artisan vendor:publish --tag=deck-migrations + ``` + + + ```bash + php artisan migrate + ``` + + + ```dotenv + DECK_TRACKING_ENABLED=true + ``` + + + +When enabled: + +- **Version activation** is recorded in the `prompt_versions` table *as well as* `metadata.json`, and the table takes precedence. See [Tracking — Activation precedence](/advanced/tracking#activation-precedence). - **Execution tracking** via `Deck::track()` inserts records into the `prompt_executions` table. - - You must publish and run the migrations before enabling tracking. See - [Installation — Publishing - migrations](/getting-started/installation#publishing-migrations). - + + Prompt rendering never depends on tracking. If the tables are missing or the database is unreachable, Deck falls back to `metadata.json` and logs a warning once, rather than failing. `Deck::track()` never throws at all — it runs after a completed AI call, and no analytics failure is worth discarding that response. + ### Database connection From ab9f74f0f881ada40218537565c844dd4c3a2689 Mon Sep 17 00:00:00 2001 From: Victor Ukam Date: Wed, 5 Aug 2026 10:00:02 +0100 Subject: [PATCH 3/3] fix: close three gaps found reviewing the hardening changes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit track() validated the prompt name before its try block, so it threw on the one input the release had just taught it to reject — contradicting the promise in its own docblock two lines above. It builds no path; the name is only a column value, so the guard protected nothing. make:prompt still created names the manager refuses. Kebab-casing passes path separators through, so `make:prompt Support/Reply` scaffolded a nested support/reply that PromptManager rejects and prompt:list renders as a broken `support` entry at v0. Nested prompts never worked — prompt:list only ever scanned the top level — so the generator now fails cleanly rather than writing something unreadable. That is the same generator/loader disagreement this release already fixed for the version directory pattern, one file over, so the name pattern moves into a ValidatesPromptNames concern that both share. Two drifts of the same shape is enough to stop relying on the two staying in step by hand. The pattern is also re-anchored with \A and \z: $ matches before a trailing newline, so "order-summary\n" was accepted. Adds an UPGRADE note for anyone who organised prompts into subdirectories, which worked in 0.4.5 and does not now. --- CHANGELOG.md | 3 ++ UPGRADE.md | 12 ++++++- docs/changelog.mdx | 7 +++- src/Concerns/ValidatesPromptNames.php | 35 +++++++++++++++++++ src/Console/Commands/MakePromptCommand.php | 15 +++++++- src/PromptManager.php | 18 ++++------ .../Commands/MakePromptCommandTest.php | 34 ++++++++++++++++++ tests/Unit/PromptManagerTest.php | 29 +++++++++++++-- 8 files changed, 136 insertions(+), 17 deletions(-) create mode 100644 src/Concerns/ValidatesPromptNames.php diff --git a/CHANGELOG.md b/CHANGELOG.md index 828071d..63351fa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Fixed prompt names being interpolated into filesystem paths without validation. A name containing `..` or a directory separator could read files outside the configured prompts path. Names are now validated and `InvalidPromptNameException` is thrown otherwise. - Fixed the version directory pattern matching far more than intended. Being unanchored and applied to the full path, `/v(\d+)$/` treated directories such as `rev2`, `dev3`, and `archive-v9` as versions 2, 3, and 9 — advertised by `prompt:list --all` and then failing to load. Both `PromptManager` and `make:prompt` now anchor the match against the directory name. - Fixed `getActiveVersion()` returning the version column unordered and uncast, which picked arbitrarily between multiple active rows and could raise a `TypeError` on drivers that return integers as strings. +- Fixed `make:prompt` creating prompts that could not be loaded back. Kebab-casing passed path separators straight through, so `make:prompt Support/Reply` scaffolded a nested `support/reply` that `PromptManager` refuses to resolve and `prompt:list` shows as a broken `support` entry. The command now validates the name it generated against the same pattern the loader applies, sharing one definition so the two cannot drift apart. +- Fixed `Deck::track()` throwing on an invalid prompt name, contradicting its own documented promise never to throw. It builds no path — the name is only a column value — so the guard protected nothing. +- Fixed the prompt name pattern accepting a trailing newline, since `$` also matches before one. It is now anchored with `\z`. ### Changed diff --git a/UPGRADE.md b/UPGRADE.md index a004116..c755cfc 100644 --- a/UPGRADE.md +++ b/UPGRADE.md @@ -39,7 +39,17 @@ Because the table is now genuinely populated, the documented rule that the datab ### Prompt names are validated -Names may contain letters, numbers, dots, dashes, and underscores, and may not begin with a dot. Anything else throws `InvalidPromptNameException`. Names generated by `make:prompt` have always satisfied this; only hand-created directories with unusual names are affected. +Names may contain letters, numbers, dots, dashes, and underscores, and may not begin with a dot. Anything else throws `InvalidPromptNameException`. + +**If you organised prompts into subdirectories**, such as `support/reply`, this is a breaking change: those names no longer load. `make:prompt` now refuses to create them too, rather than writing a prompt the package cannot read back. + +Nested prompts never worked properly. `prompt:list` scans only the top level, so `support/reply` was listed as a broken `support` entry at `v0` and never as itself. Flatten the names to restore them: + +```bash +mv resources/prompts/support/reply resources/prompts/support-reply +``` + +Then update any `Deck::get('support/reply')` call to `Deck::get('support-reply')`. --- diff --git a/docs/changelog.mdx b/docs/changelog.mdx index c280411..0af3718 100644 --- a/docs/changelog.mdx +++ b/docs/changelog.mdx @@ -9,7 +9,9 @@ Deck follows [Semantic Versioning](https://semver.org/spec/v2.0.0.html). Subscri - Tracking now defaults to **off**. If you published `config/deck.php` you are unaffected. If you did not and you rely on tracking, set `DECK_TRACKING_ENABLED=true`. If you use tracking, re-publish and run the migrations. See the [upgrade guide](https://github.com/promptphp/deck/blob/0.x/UPGRADE.md). + Tracking now defaults to **off**. If you published `config/deck.php` you are unaffected. If you did not and you rely on tracking, set `DECK_TRACKING_ENABLED=true`. If you use tracking, re-publish and run the migrations. + + If you organised prompts into subdirectories such as `support/reply`, those names no longer load — flatten them to `support-reply`. Nested prompts never worked properly: `prompt:list` scans only the top level and showed them as a broken entry. See the [upgrade guide](https://github.com/promptphp/deck/blob/0.x/UPGRADE.md). **Fixed** @@ -21,7 +23,10 @@ Deck follows [Semantic Versioning](https://semver.org/spec/v2.0.0.html). Subscri - **`Deck::activate()` now records the version it activated.** It only ever issued an `UPDATE`, which matched no rows because nothing ever inserted them — so `prompt_versions` stayed empty and the lookup that read it could fail but never succeed. Activation is now an upsert, making runtime version switching work as documented. - **Prompt names are validated.** A name was interpolated straight into a filesystem path, so one containing `..` or a directory separator could read files outside your prompts directory. Names now throw `InvalidPromptNameException` unless they contain only letters, numbers, dots, dashes, and underscores. - **Version directories are matched precisely.** The pattern was unanchored and applied to the full path, so `rev2`, `dev3`, and `archive-v9` registered as versions 2, 3, and 9 — advertised by `prompt:list --all`, then failing to load. +- **`make:prompt` can no longer create a prompt that will not load.** Kebab-casing passed path separators through, so `make:prompt Support/Reply` scaffolded a nested `support/reply` that the manager refuses to resolve and `prompt:list` displays as a broken `support` entry. The generator and the loader now share one definition of a valid name, so they cannot drift apart. +- **`Deck::track()` no longer throws on an invalid name**, which contradicted its own documented promise. It builds no path — the name is only a column value — so the guard protected nothing. - `getActiveVersion()` no longer picks arbitrarily between multiple active rows, and casts the version it reads. +- The name pattern no longer accepts a trailing newline, since `$` matches before one. **Changed** diff --git a/src/Concerns/ValidatesPromptNames.php b/src/Concerns/ValidatesPromptNames.php new file mode 100644 index 0000000..00d1eec --- /dev/null +++ b/src/Concerns/ValidatesPromptNames.php @@ -0,0 +1,35 @@ +toKebabCase($rawName); + $name = $this->toKebabCase($rawName); + + // toKebabCase() passes separators straight through, so 'Support/Reply' + // became 'support/reply' — scaffolded into a nested directory that + // PromptManager then refuses to resolve, and that prompt:list cannot + // display. Fail here rather than writing a prompt nothing can read. + if (! $this->isValidPromptName($name)) { + $this->error("Prompt name [{$name}] is not valid. Names may contain letters, numbers, dots, dashes, and underscores, and may not begin with a dot."); + + return Command::FAILURE; + } + $basePath = config('deck.path'); // Resolve description. diff --git a/src/PromptManager.php b/src/PromptManager.php index 181946c..873b059 100644 --- a/src/PromptManager.php +++ b/src/PromptManager.php @@ -14,6 +14,7 @@ use Illuminate\Support\Facades\Schema; use PromptPHP\Deck\Concerns\ReadsJsonFiles; use PromptPHP\Deck\Concerns\ResolvesVersion; +use PromptPHP\Deck\Concerns\ValidatesPromptNames; use PromptPHP\Deck\Exceptions\InvalidPromptNameException; use PromptPHP\Deck\Exceptions\InvalidVersionException; use PromptPHP\Deck\Exceptions\PromptNotFoundException; @@ -23,15 +24,7 @@ class PromptManager { use ReadsJsonFiles; use ResolvesVersion; - - /** - * Characters permitted in a prompt name. - * - * Names are interpolated into filesystem paths, so anything that could - * escape the prompts directory is rejected. A leading dot is disallowed, - * which also rules out '..'. - */ - protected const NAME_PATTERN = '/^[A-Za-z0-9_-][A-Za-z0-9._-]*$/'; + use ValidatesPromptNames; protected Filesystem $files; @@ -243,8 +236,9 @@ public function activate(string $name, string|int $version): bool */ public function track(string $promptName, int $version, array $data): void { - $this->assertValidName($promptName); - + // Deliberately not validated: track() builds no path, the name is only + // a column value, so a guard here would protect nothing while breaking + // the promise above. if (! ($connection = $this->trackingConnection())) { return; } @@ -386,7 +380,7 @@ protected function trackingTableMissing(Connection $connection, string $table): */ protected function assertValidName(string $name): void { - if (preg_match(self::NAME_PATTERN, $name) !== 1) { + if (! $this->isValidPromptName($name)) { throw InvalidPromptNameException::named($name); } } diff --git a/tests/Feature/Commands/MakePromptCommandTest.php b/tests/Feature/Commands/MakePromptCommandTest.php index c713911..21a78f6 100644 --- a/tests/Feature/Commands/MakePromptCommandTest.php +++ b/tests/Feature/Commands/MakePromptCommandTest.php @@ -754,3 +754,37 @@ ->doesntExpectOutputToContain('is still the active version') ->assertSuccessful(); }); + +// ===================================================================== +// The generator may only create names the loader can resolve +// ===================================================================== + +test('make:prompt refuses a name containing a path separator', function () { + // kebab-casing passed '/' straight through, scaffolding a nested prompt + // that PromptManager refuses to load and prompt:list cannot display. + $this->artisan('make:prompt', ['name' => 'Support/Reply']) + ->expectsOutputToContain('is not valid') + ->assertFailed(); + + expect(is_dir("{$this->tempDir}/support"))->toBeFalse(); +}); + +test('make:prompt refuses a traversing name', function () { + $this->artisan('make:prompt', ['name' => '../escape']) + ->expectsOutputToContain('is not valid') + ->assertFailed(); +}); + +test('every name make:prompt accepts can be loaded back', function () { + // Distinct after kebab-casing, so none collides with an earlier one and + // triggers the interactive "already exists" prompt. + foreach (['order-summary', 'BillingReminder', 'ops_alert', 'weekly.digest'] as $input) { + $this->artisan('make:prompt', ['name' => $input])->assertSuccessful(); + } + + // Whatever the generator wrote, the manager must resolve. + foreach (glob("{$this->tempDir}/*", GLOB_ONLYDIR) as $dir) { + $name = basename($dir); + expect(app(PromptManager::class)->get($name)->version())->toBe(1); + } +}); diff --git a/tests/Unit/PromptManagerTest.php b/tests/Unit/PromptManagerTest.php index ea4725e..1fbfc4d 100644 --- a/tests/Unit/PromptManagerTest.php +++ b/tests/Unit/PromptManagerTest.php @@ -847,11 +847,13 @@ function freshManager(?array $configOverrides = []): PromptManager } }); -test('activate() and track() reject invalid names too', function () { +test('activate() rejects invalid names, track() does not', function () { + // activate() resolves a path and so must validate. track() only writes a + // column value, and is documented never to throw. $manager = freshManager(); expect(fn () => $manager->activate('../evil', 1))->toThrow(InvalidPromptNameException::class) - ->and(fn () => $manager->track('../evil', 1, []))->toThrow(InvalidPromptNameException::class); + ->and(fn () => $manager->track('../evil', 1, []))->not->toThrow(InvalidPromptNameException::class); }); // ===================================================================== @@ -870,3 +872,26 @@ function freshManager(?array $configOverrides = []): PromptManager expect($found)->toBe([1, 10]); }); + +// ===================================================================== +// track() must never throw — it runs after a paid-for AI call +// ===================================================================== + +test('track() does not throw on a name it would otherwise reject', function () { + // track() builds no path; the name is only a column value. Validating it + // here would guard nothing while breaking the never-throws promise. + config()->set('deck.tracking.enabled', true); + + freshManager()->track('../not/a/real/name', 1, ['output' => 'hi']); +})->throwsNoExceptions(); + +test('track() does not throw when the name is valid but the table is absent', function () { + config()->set('deck.tracking.enabled', true); + + freshManager()->track('order-summary', 1, ['output' => 'hi']); +})->throwsNoExceptions(); + +test('prompt names are rejected when padded with a trailing newline', function () { + // $ matches before a final newline, so the pattern is \z-anchored. + freshManager()->versions("order-summary\n"); +})->throws(InvalidPromptNameException::class);