Skip to content
Merged
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
74 changes: 74 additions & 0 deletions .changeset/cli-sms-provider-unknown-tag-throws.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
---
"@objectstack/service-sms": minor
"@objectstack/cli": major
---

fix(cli,service-sms)!: `OS_SMS_PROVIDER=twilo` now fails the boot instead of silently becoming the log transport (#5713)

**BREAKING for one configuration: a provider tag no SMS transport can build.**
`os serve` used to hand `OS_SMS_PROVIDER` (or `config.sms.provider`) straight to
`SmsServicePlugin` with nothing to compare it against. The plugin then caught the
`makeSmsTransport: unknown provider 'twilo'` throw, substituted `LogSmsTransport`,
and booted normally — measured, not inferred:

```
new SmsServicePlugin({ provider: 'twilo' }).init(ctx)
booted_without_throw: true transport_class: 'LogSmsTransport'
isConfigured(): false logger.warn × 1, logger.error × 0
service.send(…) → { status: 'sent', messageId: 'dev-sms-…' }
```

So a phone-OTP sign-in answered "code sent", the user waited for an SMS that was
never dispatched, and the one `warn` line scrolled past in the boot log. That is
the declared-but-not-delivered shape of Prime Directive #10, and the same one
#5132 closed for **mail** in the neighbouring arm of the very same capability
loop.

Three gates already guard the `sms` provider value and none of them could see
this path: the `sms` settings namespace declares `provider` as a `select` with an
options table, #5131 enforces that table on the write path, and #5204 closed the
`SettingsService` env-override branch. All three live behind `SettingsService` —
this read happens while the kernel is being assembled, *before* a settings
service exists.

**`resolveSmsCapabilityArg` now refuses a provider tag outside
`log` / `aliyun` / `twilio`**, the way its neighbouring `resolveEmailCapabilityArg`
already did, and the capability loop turns that into the loud failure it should
be — a hard boot error when the app declared `requires: ['sms']`, otherwise a
`console.error` and no SMS service.

**What it deliberately does NOT do:** demand credentials. Unlike mail, SMS
provider credentials are not a boot-time input — the `sms` settings namespace
binds them at `kernel:ready`, and that is their documented home. A bare
`OS_SMS_PROVIDER=twilio` on a host whose Twilio keys live in Settings is a
complete configuration and passes through untouched. `SmsServicePlugin`'s own
fallback is likewise untouched: for a *known* provider with incomplete
constructor credentials it is correct (the settings bind can still swap in a
working transport), and it remains the last line of defence for hosts that
construct the plugin themselves. `os serve` simply stops feeding it input it can
never use.

**Who is affected:** deployments that set `OS_SMS_PROVIDER` (or
`config.sms.provider`) to a value outside the supported three — in practice a
typo, or a provider that was never implemented — and relied on the fallback to
boot. An unset `OS_SMS_PROVIDER` still defaults to `log`; every supported tag
still boots with or without credentials.

**Migration — one line, either direction:**

- the environment is *not* meant to send SMS → `OS_SMS_PROVIDER=log` (that
explicit value is the supported way to say so, and why refusing the others is
fair);
- the environment *is* meant to send SMS → fix the tag to `aliyun` or `twilio`
and put the credentials in Settings → SMS Delivery (or
`config.sms.providerOptions`).

The error names the consequence and both fixes, per AGENTS.md's
degradation-log-level rule.

`@objectstack/service-sms` gains the vocabulary the CLI reads instead of
restating: `SMS_TRANSPORT_PROVIDERS` and `isSmsTransportProvider()`, with
`SmsProviderTag` now derived from the array rather than declared beside it. One
vocabulary, two consumers — a second literal list in the CLI is how the mail
settings dropdown and the mail transports drifted apart in the first place
(#5094).
13 changes: 13 additions & 0 deletions content/docs/kernel/runtime-services/sms-service.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,19 @@ Every key accepts the standard settings env override (`OS_SMS_PROVIDER`,
`OS_SMS_ALIYUN_ACCESS_KEY_ID`, …). The **Send test SMS** action exercises the
live (or unsaved) provider configuration.

`OS_SMS_PROVIDER` (and `config.sms.provider`) is read twice, at two different
moments: once by `os serve` while it assembles the kernel — before any settings
service exists — to pick the plugin's initial transport, and again by the
settings namespace once that binds at `kernel:ready`. **A provider value outside
`log` / `aliyun` / `twilio` is refused at the first of those** (#5713): the boot
fails when the app declares `requires: ['sms']`, and otherwise logs an error and
starts without an SMS service. It used to become the `log` transport silently, so
a typo like `OS_SMS_PROVIDER=twilo` produced a server that answered every send
`status: 'sent'` and delivered nothing. Credentials are *not* required at boot —
only the provider tag is checked, because the credentials legitimately arrive
later, from this namespace. An environment that is not meant to send SMS says so
with `OS_SMS_PROVIDER=log`, which is the default.

## Consumers

- **Phone-number OTP auth** — sign-in verification codes and self-service
Expand Down
131 changes: 131 additions & 0 deletions packages/cli/src/commands/serve-sms-capability.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,131 @@
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.

/**
* framework#5713 — what `SmsServicePlugin` is constructed with on the
* `os serve` path, and what happens when the provider tag cannot deliver.
*
* The `sms` settings namespace declares `provider` as a `select` with an
* options table (`log` / `aliyun` / `twilio`), #5131 enforces that table on the
* write path, and #5204 closed the `SettingsService` env-override branch that
* bypassed it. None of those three gates can see this path: `os serve` reads
* `OS_SMS_PROVIDER` while assembling the kernel, *before* a settings service
* exists, and handed the string straight to the plugin.
*
* Measured on `origin/main` before this change, `provider: 'twilo'` (a plausible
* misspelling of `twilio`) reached `SmsServicePlugin.init`, threw inside
* `makeSmsTransport`, was caught, and became `LogSmsTransport`:
*
* booted_without_throw: true transport_class: 'LogSmsTransport'
* isConfigured(): false send() → { status: 'sent', messageId: 'dev-sms-…' }
*
* — a server that answers every OTP send "sent" and delivers nothing. That is
* the declared-but-not-delivered shape of Prime Directive #10, and the same one
* #5132 closed for mail in the neighbouring arm of this very loop.
*
* These pin the invariant in one piece: a configuration this server can deliver
* through reaches the plugin unchanged (credentials included — they legitimately
* arrive later, from the settings namespace at `kernel:ready`), and a provider
* tag it cannot deliver through throws. The counterpart the throw depends on is
* pinned too: an operator who does not want SMS sent says so with
* `OS_SMS_PROVIDER=log`, and that still boots.
*/

import { describe, it, expect } from 'vitest';
import { SMS_TRANSPORT_PROVIDERS } from '@objectstack/service-sms';
import { resolveSmsCapabilityArg } from './serve.js';

describe('resolveSmsCapabilityArg', () => {
it('defaults to the log provider when nothing is configured', () => {
const { options } = resolveSmsCapabilityArg({}, {});
expect(options).toMatchObject({ provider: 'log' });
expect(options).not.toHaveProperty('providerOptions');
expect(options).not.toHaveProperty('retries');
});

it('boots on an EXPLICIT provider=log — the way to say "this environment does not send SMS"', () => {
// The premise of every throw below: refusing an undeliverable provider is
// only fair because "no SMS from here" has its own spelling. If this ever
// stops booting, the errors elsewhere in this file stop being actionable.
expect(() => resolveSmsCapabilityArg({}, { OS_SMS_PROVIDER: 'log' })).not.toThrow();
expect(resolveSmsCapabilityArg({}, { OS_SMS_PROVIDER: 'log' }).options)
.toMatchObject({ provider: 'log' });
// …including from objectstack.config.ts.
expect(resolveSmsCapabilityArg({ provider: 'log' }, {}).options).toMatchObject({ provider: 'log' });
});

it('lets env beat config, and normalizes the case', () => {
expect(resolveSmsCapabilityArg({ provider: 'aliyun' }, { OS_SMS_PROVIDER: 'twilio' }).options)
.toMatchObject({ provider: 'twilio' });
// `OS_SMS_PROVIDER=Twilio` is the same declaration — the guard runs on the
// lower-cased value, never on the raw env string.
expect(resolveSmsCapabilityArg({}, { OS_SMS_PROVIDER: 'Twilio' }).options)
.toMatchObject({ provider: 'twilio' });
expect(resolveSmsCapabilityArg({ provider: 'ALIYUN' }, {}).options)
.toMatchObject({ provider: 'aliyun' });
});

it('passes a deliverable provider through WITHOUT demanding credentials', () => {
// Unlike mail, SMS credentials are not a boot-time input: the `sms`
// settings namespace binds them at kernel:ready. A bare provider tag is a
// complete configuration here, so this arm refuses the tag and nothing
// else — demanding keys would break every host that stores them in
// Settings, which is the documented home for them.
for (const provider of SMS_TRANSPORT_PROVIDERS) {
expect(() => resolveSmsCapabilityArg({}, { OS_SMS_PROVIDER: provider }), provider).not.toThrow();
expect(resolveSmsCapabilityArg({}, { OS_SMS_PROVIDER: provider }).options, provider)
.toMatchObject({ provider });
}
});

it('carries config.sms.providerOptions and retries through untouched', () => {
const { options } = resolveSmsCapabilityArg(
{ provider: 'aliyun', providerOptions: { accessKeyId: 'ak', signName: '签名' }, retries: 2 },
{},
);
expect(options).toMatchObject({
provider: 'aliyun',
providerOptions: { accessKeyId: 'ak', signName: '签名' },
retries: 2,
});
// `retries: 0` is a real declaration (no retry), not an absence.
expect(resolveSmsCapabilityArg({ retries: 0 }, {}).options).toMatchObject({ retries: 0 });
});

it('THROWS on a provider tag no transport can deliver — no silent LogSmsTransport (#5713)', () => {
const boot = () => resolveSmsCapabilityArg({}, { OS_SMS_PROVIDER: 'twilo' });
expect(boot).toThrow(/provider='twilo'/);
// Consequence AND fix in the one message (AGENTS.md degradation-log-level).
expect(boot).toThrow(/nothing would leave the box/);
expect(boot).toThrow(/log \/ aliyun \/ twilio/);
expect(boot).toThrow(/OS_SMS_PROVIDER=log/);
// …and never the old silent rewrite.
expect(boot).not.toThrow(/falling back to LogSmsTransport/);
});

it('refuses the same tag declared through config.sms.provider', () => {
// A typo in objectstack.config.ts is the same declaration by another
// channel — and the one an operator cannot fix with an env var.
expect(() => resolveSmsCapabilityArg({ provider: 'aliyn' }, {}))
.toThrow(/provider='aliyn'/);
// Retired-looking and never-supported tags land in the same arm, and the
// message names the vocabulary rather than guessing at an intent.
for (const tag of ['sendgrid', 'aws-sns', 'tencent', 'smtp']) {
expect(() => resolveSmsCapabilityArg({}, { OS_SMS_PROVIDER: tag }), tag)
.toThrow(/log \/ aliyun \/ twilio/);
}
});

it('reads its vocabulary from @objectstack/service-sms, not a second literal', () => {
// #5094's lesson, pinned: if a transport is added to service-sms and this
// file kept its own list, the new provider would be refused at boot while
// the plugin could build it. The error message enumerates the exported
// vocabulary, so this assertion goes red the day the two diverge.
let message = '';
try {
resolveSmsCapabilityArg({}, { OS_SMS_PROVIDER: 'definitely-not-a-provider' });
} catch (err) {
message = (err as Error).message;
}
expect(message).toContain(SMS_TRANSPORT_PROVIDERS.join(' / '));
});
});
85 changes: 78 additions & 7 deletions packages/cli/src/commands/serve.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,13 @@ import { missingProviderMessage } from '../utils/capability-preflight.js';
// only (no plugin class): `os serve` loads `EmailServicePlugin` itself through
// the capability loop's dynamic import, host copy first.
import { isEmailTransportProvider, emailProviderRequiresApiKey, unsupportedProviderFix } from '@objectstack/plugin-email';
// The SMS provider vocabulary, read from the package that materialises the
// transports, for the same reason and by the same rule as the mail one above
// (#5713). `resolveSmsCapabilityArg` has to refuse exactly the tags
// `makeSmsTransport` cannot build — restating `log`/`aliyun`/`twilio` here would
// be the second literal #5094 was filed for. Values only (no plugin class): the
// capability loop dynamic-imports `SmsServicePlugin` itself, host copy first.
import { isSmsTransportProvider, SMS_TRANSPORT_PROVIDERS } from '@objectstack/service-sms';
import { resolveObjectStackHome } from '@objectstack/runtime';
import { LOG_LEVELS, resolveLogLevel, readLogLevelEnv } from '../utils/log-level.js';
import { BootLogCapture, isVerboseBootLevel } from '../utils/boot-log-capture.js';
Expand Down Expand Up @@ -2342,13 +2349,12 @@ export default class Serve extends Command {
// credentials normally live in the `sms` settings namespace
// (bound at kernel:ready); constructor opts cover pre-settings
// boot and hosts without the settings service.
const cfgSms = (config as any).sms ?? {};
const provider = (process.env.OS_SMS_PROVIDER || cfgSms.provider || 'log').toLowerCase();
arg = {
provider,
...(cfgSms.providerOptions ? { providerOptions: cfgSms.providerOptions } : {}),
...(cfgSms.retries != null ? { retries: cfgSms.retries } : {}),
};
//
// Throws on a provider tag no transport can deliver (#5713) — the
// catch below turns that into the boot failure / loud error it
// should be, never a LogSmsTransport substituted behind the
// operator's back. Same shape as the `email` arm above.
arg = resolveSmsCapabilityArg((config as any).sms ?? {}, process.env).options;
} else if (cap === 'storage') {
// Storage is now in the default capability slate. If the host
// hasn't configured a backend explicitly we fall back to the
Expand Down Expand Up @@ -3231,6 +3237,71 @@ export function resolveEmailCapabilityArg(
return { options };
}

/** Constructor options for `SmsServicePlugin`, as the capability loop builds them. */
export interface SmsCapabilityArg {
options: Record<string, unknown>;
}

/**
* Resolve `SmsServicePlugin` constructor options from `config.sms` + `OS_SMS_*`
* env, and **refuse a provider tag no transport can deliver through** (#5713).
*
* The refusal is the point. Credentials for a real provider normally arrive from
* the `sms` settings namespace at `kernel:ready`, so this function deliberately
* does NOT demand them — a bare `OS_SMS_PROVIDER=twilio` on a host whose Twilio
* keys are stored in Settings is a complete, working configuration and passes
* through untouched. What it refuses is the one thing settings can never repair:
* a provider *tag* outside `SMS_TRANSPORT_PROVIDERS`.
*
* That tag used to travel all the way into the plugin, which caught the
* `makeSmsTransport: unknown provider 'twilo'` throw and substituted
* `LogSmsTransport` behind the operator's back. Measured on `origin/main` before
* this change, `new SmsServicePlugin({ provider: 'twilo' }).init(ctx)`:
*
* - boots without throwing, registers the `sms` service;
* - transport = `LogSmsTransport`, `isConfigured() === false`;
* - one `logger.warn` line, then `send()` answers
* `{ status: 'sent', messageId: 'dev-sms-…' }`.
*
* So a phone-OTP sign-in tells the user "code sent" and nothing leaves the box —
* the same declared-but-not-delivered shape #5132 closed for mail one layer up,
* and the same door #5204 closed on the `SettingsService` env branch. This path
* never reaches `SettingsService`: it runs at kernel-assembly time, before the
* settings service exists, which is exactly why the `sms` namespace's `select`
* options table (`sms.manifest.ts`) could not see it.
*
* The plugin's fallback is left alone on purpose. For a *known* provider with
* incomplete constructor credentials it is correct — the settings bind can still
* swap in a working transport — and it stays the last line of defence for hosts
* that construct `SmsServicePlugin` themselves. `os serve` simply stops handing
* it input it cannot use.
*
* `OS_SMS_PROVIDER=log` (the default) is how an environment says "this box does
* not send SMS", which is what makes refusing the rest fair.
*/
export function resolveSmsCapabilityArg(
cfgSms: Record<string, any> = {},
env: NodeJS.ProcessEnv = process.env,
): SmsCapabilityArg {
const provider = String(env.OS_SMS_PROVIDER || cfgSms.provider || 'log').toLowerCase();
if (!isSmsTransportProvider(provider)) {
throw new Error(
`provider='${provider}' is not a transport this server can deliver through, so every OTP and `
+ "notification SMS would be answered status: 'sent' and nothing would leave the box — "
+ `pick one of ${SMS_TRANSPORT_PROVIDERS.join(' / ')} (Settings → SMS Delivery → Provider). `
+ 'On this boot path the provider is OS_SMS_PROVIDER or config.sms.provider; set '
+ 'OS_SMS_PROVIDER=log if this environment is not meant to send SMS.',
);
}
return {
options: {
provider,
...(cfgSms.providerOptions ? { providerOptions: cfgSms.providerOptions } : {}),
...(cfgSms.retries != null ? { retries: cfgSms.retries } : {}),
},
};
}

/**
* Best-effort driver introspection.
*
Expand Down
2 changes: 2 additions & 0 deletions packages/services/service-sms/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ export {
export { SmsServicePlugin, type SmsServicePluginOptions } from './sms-plugin.js';
export {
makeSmsTransport,
SMS_TRANSPORT_PROVIDERS,
isSmsTransportProvider,
AliyunSmsTransport,
TwilioSmsTransport,
type SmsProviderTag,
Expand Down
Loading
Loading