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

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 25 additions & 1 deletion packages/targets/chat-telegram/src/index.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
import { afterEach, describe, expect, it, vi } from 'vitest';
import { contractTestTarget, fakeShipContext } from '@profullstack/sh1pt-core/testing';
import { contractTestTarget, fakeBuildContext, fakeShipContext } from '@profullstack/sh1pt-core/testing';
import { mkdtemp, rm } from 'node:fs/promises';
import { tmpdir } from 'node:os';
import { join } from 'node:path';
import adapter from './index.js';

contractTestTarget(adapter, {
Expand All @@ -14,7 +17,28 @@ afterEach(() => {
vi.restoreAllMocks();
});

const tempDirs: string[] = [];

afterEach(async () => {
await Promise.all(tempDirs.splice(0).map((dir) => rm(dir, { recursive: true, force: true })));
});

describe('chat-telegram API calls', () => {
it('keeps bot usernames with path separators inside the output directory', async () => {
const outDir = await mkdtemp(join(tmpdir(), 'sh1pt-telegram-out-'));
tempDirs.push(outDir);

const result = await adapter.build(fakeBuildContext({
outDir,
dryRun: true,
}) as any, {
botUsername: '../demo_bot',
webhookUrl: 'https://example.com/telegram',
});

expect(result.artifact).toBe(join(outDir, 'telegram-demo_bot.json'));
});

it('sets webhook, commands, and bot descriptions', async () => {
const fetchMock = vi.spyOn(globalThis, 'fetch').mockResolvedValue({
ok: true,
Expand Down
10 changes: 9 additions & 1 deletion packages/targets/chat-telegram/src/index.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { defineTarget, manualSetup } from '@profullstack/sh1pt-core';
import { join } from 'node:path';

// Telegram bots. No "store" — a bot is just a token + webhook URL. This
// adapter registers the webhook with Telegram, sets commands/description/
Expand All @@ -25,13 +26,20 @@ interface TelegramResponse<T> {

type TelegramCommand = { command: string; description: string };

function safeFileStem(value: string): string {
return value
.replace(/[^a-zA-Z0-9._-]+/g, '-')
.replace(/^\.+|\.+$/g, '')
.replace(/^-+|-+$/g, '') || 'telegram-bot';
}

export default defineTarget<Config>({
id: 'chat-telegram',
kind: 'chat',
label: 'Telegram Bot',
async build(ctx, config) {
ctx.log(`telegram · prepare bot manifest for @${config.botUsername}`);
return { artifact: `${ctx.outDir}/telegram-${config.botUsername}.json` };
return { artifact: join(ctx.outDir, `telegram-${safeFileStem(config.botUsername)}.json`) };
},
async ship(ctx, config) {
const username = normalizeUsername(config.botUsername);
Expand Down
24 changes: 24 additions & 0 deletions packages/targets/pkg-flatpak/src/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,4 +62,28 @@ describe('Flatpak manifest generation', () => {
appId: 'com.example.MyApp',
})).resolves.toEqual({ id: 'dry-run' });
});

it('rejects invalid app IDs before manifest generation', async () => {
const outDir = await mkdtemp(join(tmpdir(), 'sh1pt-flatpak-'));
tempDirs.push(outDir);
const ctx = fakeBuildContext({
outDir,
projectDir: '/repo/myapp',
version: '1.2.3',
channel: 'stable',
}) as any;

for (const appId of ['../escape', 'com.example', 'com.example.App-', 'com.123.App']) {
await expect(adapter.build(ctx, { appId })).rejects.toThrow('appId');
}
});

it('rejects invalid app IDs before shipping', async () => {
await expect(adapter.ship(fakeShipContext({
version: '1.2.3',
dryRun: true,
}) as any, {
appId: '../escape',
})).rejects.toThrow('appId');
});
});
24 changes: 2 additions & 22 deletions packages/targets/pkg-flatpak/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ function renderList(values: string[], indent: string): string[] {
/**
* Validate a Flatpak application ID.
* Must be a reverse-DNS string with at least 3 dot-separated segments,
* each non-empty and containing only alphanumeric characters or hyphens/underscores.
* each starting with a letter and containing alphanumeric characters, hyphens, or underscores.
* Must not contain path traversal characters.
*/
function validateAppId(appId: string): void {
Expand All @@ -49,7 +49,7 @@ function validateAppId(appId: string): void {
if (!seg) {
throw new Error(`pkg-flatpak: invalid appId "${appId}" — segments must be non-empty`);
}
if (!/^[A-Za-z0-9_-]+$/.test(seg)) {
if (!/^[A-Za-z][A-Za-z0-9_]*(?:-[A-Za-z0-9_]+)*$/.test(seg)) {
throw new Error(`pkg-flatpak: invalid appId "${appId}" — segment "${seg}" contains invalid characters`);
}
}
Expand Down Expand Up @@ -107,26 +107,6 @@ function renderFlatpakManifest(ctx: { projectDir: string; version: string; chann
return lines.join('\n');
}

/**
* Validate a Flatpak app ID (reverse-DNS format): at least 3 dot-separated segments,
* each containing alphanumeric or hyphens. e.g. "com.example.MyApp"
*/
function validateAppId(appId: string): void {
if (!appId) throw new Error('pkg-flatpak: appId is required');
const segments = appId.split('.');
if (segments.length < 3) {
throw new Error(`pkg-flatpak: invalid appId "${appId}". Must have at least 3 reverse-DNS segments (e.g. "com.example.MyApp").`);
}
if (appId.includes('..') || appId.includes('/') || appId.includes('\\')) {
throw new Error(`pkg-flatpak: appId "${appId}" contains path traversal characters.`);
}
for (const seg of segments) {
if (!seg || !/^[A-Za-z0-9_-]+$/.test(seg)) {
throw new Error(`pkg-flatpak: invalid segment "${seg}" in appId "${appId}".`);
}
}
}

export default defineTarget<Config>({
id: 'pkg-flatpak',
kind: 'package-manager',
Expand Down
44 changes: 43 additions & 1 deletion packages/targets/pkg-snap/src/index.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { fakeBuildContext, fakeShipContext, smokeTest } from '@profullstack/sh1pt-core/testing';
import { mkdtemp, readFile, rm } from 'node:fs/promises';
import { mkdir, mkdtemp, readFile, readdir, rm } from 'node:fs/promises';
import { tmpdir } from 'node:os';
import { join } from 'node:path';
import { afterEach, describe, expect, it } from 'vitest';
Expand Down Expand Up @@ -60,4 +60,46 @@ describe('snapcraft manifest generation', () => {
snapName: 'myapp',
})).resolves.toEqual({ id: 'dry-run' });
});

it('rejects invalid snap names before generating manifests', async () => {
const outDir = await mkdtemp(join(tmpdir(), 'sh1pt-snap-'));
tempDirs.push(outDir);

const ctx = fakeBuildContext({
outDir,
projectDir: '/repo/myapp',
version: '1.2.3',
channel: 'stable',
}) as any;

for (const snapName of ['Bad: Name', '-myapp', 'myapp-', 'my--app', '1234', 'a'.repeat(41)]) {
await expect(adapter.build(ctx, { snapName })).rejects.toThrow('snapName');
}
});

it('rejects invalid snap names before touching the output directory', async () => {
const outDir = await mkdtemp(join(tmpdir(), 'sh1pt-snap-'));
tempDirs.push(outDir);
await mkdir(outDir, { recursive: true });

await expect(adapter.build(fakeBuildContext({
outDir,
projectDir: '/repo/myapp',
version: '1.2.3',
channel: 'stable',
}) as any, {
snapName: 'my--app',
})).rejects.toThrow('snapName');

await expect(readdir(outDir)).resolves.toEqual([]);
});

it('rejects invalid snap names before shipping', async () => {
await expect(adapter.ship(fakeShipContext({
version: '1.2.3',
dryRun: true,
}) as any, {
snapName: 'Bad: Name',
})).rejects.toThrow('snapName');
});
});
13 changes: 4 additions & 9 deletions packages/targets/pkg-snap/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,9 @@ function validateSnapName(snapName: string): void {
if (snapName.startsWith('-') || snapName.endsWith('-')) {
throw new Error(`pkg-snap: snapName "${snapName}" must not start or end with a hyphen`);
}
if (snapName.includes('--')) {
throw new Error(`pkg-snap: snapName "${snapName}" must not contain consecutive hyphens`);
}
if (!/^[a-z0-9-]+$/.test(snapName)) {
throw new Error(`pkg-snap: snapName "${snapName}" must contain only lowercase letters, digits, and hyphens`);
}
Expand All @@ -57,6 +60,7 @@ function validateSnapName(snapName: string): void {
}

function renderSnapcraftYaml(ctx: { projectDir: string; version: string; channel: string }, config: Config): string {
validateSnapName(config.snapName);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 Double validation in build() path

validateSnapName is called inside renderSnapcraftYaml (line 63) AND again explicitly at the start of build() (line 114) before the same renderSnapcraftYaml is invoked. When build() runs, validation fires twice. The explicit guard at line 114 is the one that matters for the "no filesystem side-effects on bad input" guarantee — the call inside renderSnapcraftYaml is redundant and should be removed from there (keeping only the call sites in build and ship directly).

const grade = config.grade ?? (ctx.channel === 'stable' ? 'stable' : 'devel');
const confinement = config.confinement ?? 'strict';
const base = config.base ?? 'core22';
Expand Down Expand Up @@ -102,15 +106,6 @@ function renderSnapcraftYaml(ctx: { projectDir: string; version: string; channel
return lines.join('\n');
}

/** Validate a snap package name: lowercase, alphanumeric, hyphens only (no leading/trailing hyphen). */
function validateSnapName(name: string): void {
if (!name || !/^[a-z0-9]([a-z0-9-]*[a-z0-9])?$/.test(name)) {
throw new Error(
`pkg-snap: invalid snapName "${name}". Snap names must be lowercase alphanumeric with optional hyphens (no leading/trailing hyphen, no uppercase, no underscore).`,
);
}
}

export default defineTarget<Config>({
id: 'pkg-snap',
kind: 'package-manager',
Expand Down
36 changes: 36 additions & 0 deletions packages/targets/pkg-winget/src/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -82,4 +82,40 @@ describe('winget manifest generation', () => {
],
})).resolves.toEqual({ id: 'dry-run' });
});

it('rejects invalid package IDs before manifest generation', async () => {
const outDir = await mkdtemp(join(tmpdir(), 'sh1pt-winget-'));
tempDirs.push(outDir);
const ctx = fakeBuildContext({
outDir,
version: '1.2.3',
}) as any;
const installers = [
{
architecture: 'x64' as const,
url: 'https://downloads.example.com/my-tool-1.2.3-x64.exe',
sha256: 'c'.repeat(64),
},
];

for (const packageId of ['NoDot', '.Acme.MyTool', 'Acme..MyTool', 'Acme/MyTool']) {
await expect(adapter.build(ctx, { packageId, installers })).rejects.toThrow('packageId');
}
});

it('rejects invalid package IDs before shipping', async () => {
await expect(adapter.ship(fakeShipContext({
version: '1.2.3',
dryRun: true,
}) as any, {
packageId: 'Acme/MyTool',
installers: [
{
architecture: 'x64',
url: 'https://downloads.example.com/my-tool-1.2.3-x64.exe',
sha256: 'c'.repeat(64),
},
],
})).rejects.toThrow('packageId');
});
});
22 changes: 0 additions & 22 deletions packages/targets/pkg-winget/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -124,28 +124,6 @@ function renderLocaleManifest(config: Config, version: string): string {
return lines.join('\n');
}

/**
* Validate a winget package identifier: must contain at least one dot,
* each segment must be non-empty alphanumeric+hyphens/underscores/dots.
* e.g. "Microsoft.WindowsTerminal"
*/
function validatePackageId(packageId: string): void {
if (!packageId) throw new Error('pkg-winget: packageId is required');
const segments = packageId.split('.');
if (segments.length < 2) {
throw new Error(`pkg-winget: invalid packageId "${packageId}". Must be "Publisher.AppName" format with at least one dot.`);
}
for (const seg of segments) {
if (!seg) throw new Error(`pkg-winget: empty segment in packageId "${packageId}".`);
if (!/^[A-Za-z0-9_\-]+$/.test(seg)) {
throw new Error(`pkg-winget: invalid segment "${seg}" in packageId "${packageId}". Segments must be alphanumeric with hyphens or underscores.`);
}
}
if (packageId.includes('..') || packageId.includes('/') || packageId.includes('\\')) {
throw new Error(`pkg-winget: packageId "${packageId}" contains path traversal characters.`);
}
}

export default defineTarget<Config>({
id: 'pkg-winget',
kind: 'package-manager',
Expand Down
Loading