Skip to content

Validate Signal runtime config #617

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
rissrice2105-agent wants to merge 1 commit into from
Closed

Validate Signal runtime config #617

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
22 changes: 21 additions & 1 deletion packages/targets/chat-signal/src/index.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { contractTestTarget, fakeBuildContext, smokeTest } from '@profullstack/sh1pt-core/testing';
import { contractTestTarget, fakeBuildContext, fakeShipContext, smokeTest } from '@profullstack/sh1pt-core/testing';
import { mkdtemp, readFile, rm } from 'node:fs/promises';
import { tmpdir } from 'node:os';
import { join } from 'node:path';
Expand Down Expand Up @@ -38,4 +38,24 @@ describe('Signal target planning', () => {
captchaTokenPresent: true,
});
});

it('rejects non-E.164 phone numbers before writing runtime plans', async () => {
const outDir = await mkdtemp(join(tmpdir(), 'sh1pt-signal-'));
tempDirs.push(outDir);

await expect(adapter.build(fakeBuildContext({ outDir }) as any, {
...sampleConfig,
phoneNumber: '415-555-1234',
})).rejects.toThrow('E.164');
});

it('rejects unsupported runtimes before dry-run shipping', async () => {
await expect(adapter.ship(fakeShipContext({
version: '1.2.3',
dryRun: true,
}) as any, {
...sampleConfig,
runtime: 'signal-web',
} as any)).rejects.toThrow('runtime must be one of');
});
Comment on lines +52 to +60

@greptile-apps greptile-apps Bot Jun 5, 2026

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 Missing coverage: build with invalid runtime. The new runtime-rejection test only exercises ship. Since build also calls requireRuntime directly, a counterpart test that passes an invalid runtime to build would confirm the guard fires on both entry points. Without it, a future refactor that accidentally drops the requireRuntime call from build would go undetected.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

});
32 changes: 27 additions & 5 deletions packages/targets/chat-signal/src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,31 +19,53 @@ interface Config {
deviceName?: string;
}

const RUNTIMES = ['signal-cli', 'signald'] as const;

function requirePhoneNumber(config: Config): string {
const phoneNumber = config.phoneNumber?.trim();
if (!/^\+[1-9]\d{7,14}$/.test(phoneNumber)) {

@greptile-apps greptile-apps Bot Jun 5, 2026

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 The E.164 regex requires at minimum 9 characters after + ([1-9] plus \d{7,14}), but the ITU-T standard allows as few as 4 total digits for some assigned country codes (e.g., Niue +683XXXX = 7 digits). Any phone number with fewer than 8 digits after + will be incorrectly rejected. Lowering the lower-bound to {6,14} brings coverage down to the smallest real-world allocations while staying well within E.164's 15-digit ceiling.

Suggested change
if (!/^\+[1-9]\d{7,14}$/.test(phoneNumber)) {
if (!/^\+[1-9]\d{6,14}$/.test(phoneNumber)) {

throw new Error('chat-signal phoneNumber must be an E.164 number such as +14155551234');
}
return phoneNumber;
}

function requireRuntime(config: Config): Config['runtime'] {
const runtime = String(config.runtime ?? '').trim();
if (!RUNTIMES.includes(runtime as Config['runtime'])) {
throw new Error(`chat-signal runtime must be one of: ${RUNTIMES.join(', ')}`);
}
return runtime as Config['runtime'];
}

export default defineTarget<Config>({
id: 'chat-signal',
kind: 'chat',
label: 'Signal (signal-cli / signald)',
async build(ctx, config) {
ctx.log(`prepare ${config.runtime} config for ${config.phoneNumber}`);
const phoneNumber = requirePhoneNumber(config);
const runtime = requireRuntime(config);
ctx.log(`prepare ${runtime} config for ${phoneNumber}`);
const artifactDir = join(ctx.outDir, 'signal-runtime');
const planPath = join(artifactDir, 'signal-runtime-plan.json');
await mkdir(artifactDir, { recursive: true });
await writeFile(planPath, `${JSON.stringify({
phoneNumber: config.phoneNumber,
runtime: config.runtime,
phoneNumber,
runtime,
deviceName: config.deviceName,
captchaTokenPresent: !!config.captchaToken,
}, null, 2)}\n`, 'utf-8');
return { artifact: planPath };
},
async ship(ctx, config) {
ctx.log(`register Signal number ${config.phoneNumber} (${config.runtime})`);
const phoneNumber = requirePhoneNumber(config);
const runtime = requireRuntime(config);
ctx.log(`register Signal number ${phoneNumber} (${runtime})`);
if (ctx.dryRun) return { id: 'dry-run' };
// TODO:
// - signal-cli register -v <phone> (requires captchaToken)
// - verify with SMS/voice code (human step unless using a SIP gateway)
// - store runtime secrets (identity keys) in secrets vault
return { id: `signal:${config.phoneNumber}@${ctx.version}` };
return { id: `signal:${phoneNumber}@${ctx.version}` };
},
async status(id) {
return { state: 'live', version: id };
Expand Down
Loading