Skip to content

Reject out-of-range runtime ramps instead of activating cache layers #102

Description

@lan17

Priority

P1 — High — rollout-safety bug. A malformed runtime ramp above 100 silently activates the affected shared cache layer for every key, turning a gradual-rollout control-plane error into full cache enablement.

Baseline / current behavior

Static/default ramps are validated against the documented inclusive 0..100 domain and rejected outside it:

  • DialCache/src/dialcache.ts

    Lines 832 to 839 in e06d833

    const ramp = snapshot.ramp[layer];
    if (ramp !== undefined) {
    if (typeof ramp !== "number") {
    throw new TypeError(`DialCache defaultConfig ramp.${layer} must be a number`);
    }
    if (!Number.isFinite(ramp) || ramp < 0 || ramp > 100) {
    throw new RangeError(`DialCache defaultConfig ramp.${layer} must be between 0 and 100`);
    }

Runtime ramps are treated differently. Resolution rejects only non-finite values, then clamps every finite value below 0 to 0 and above 100 to 100:

  • const ttlSec = config.ttlSec[options.layer];
    if (ttlSec === undefined) {
    return { status: "disabled", reason: "policy_disabled" };
    }
    if (!Number.isSafeInteger(ttlSec) || ttlSec <= 0) {
    return { status: "disabled", reason: "invalid_ttl" };
    }
    const configuredRampValue = config.ramp[options.layer];
    const configuredRamp = configuredRampValue === undefined ? 100 : configuredRampValue;
    if (!Number.isFinite(configuredRamp)) {
    return { status: "disabled", reason: "invalid_ramp" };
    }
    const ramp = clampPercentage(configuredRamp);
    if (ramp <= 0) {
    return { status: "disabled", reason: "ramped_down" };
    }
    if (ramp >= 100) {
    return { status: "enabled", config: { ttlSec, ramp } };
    }
    const sample = deterministicRampSample(options.key, options.layer);
    return sample < ramp
    ? { status: "enabled", config: { ttlSec, ramp } }
    : { status: "disabled", reason: "ramped_down" };
  • function assertLayerConfig(config: LayerConfig | undefined, name: "ttlSec" | "ramp"): void {
    if (config !== undefined && (config === null || typeof config !== "object" || Array.isArray(config))) {
    throw new TypeError(`DialCache ${name} config must be a layer map`);
    }
    }
    function clampPercentage(value: number): number {
    if (value <= 0) {
    return 0;
    }
    if (value >= 100) {
    return 100;
    }
    return value;
  • DialCache/README.md

    Lines 215 to 223 in e06d833

    A provider result of `null` (or defensive `undefined`) applies no overrides. An empty `DialCacheKeyConfig` and omitted runtime fields also inherit the baseline. Use explicit values to override inherited policy: `requestLocal: false` disables request-local caching and a layer ramp of `0` disables that shared layer. `DialCacheKeyConfig.disabled()` is that explicit kill switch in one call: request-local off and both shared layers ramped to 0.
    DialCache validates `defaultConfig` when `cached()` registers a definition and whenever `getOrLoad()` is invoked: TTLs and remote-read deadlines must be positive safe integers, ramps must be finite percentages from 0 to 100, layer maps must be objects, and `requestLocal` must be a boolean when present. Invalid defaults are rejected immediately.
    Each registration or one-shot invocation captures an immutable internal snapshot of `defaultConfig`; mutating the supplied config or its maps later does not change that operation's baseline. Runtime policy changes belong in the provider's returned overlay.
    Runtime TTL and ramp leaves are used as supplied instead of falling back to valid default leaves. An invalid TTL disables that layer with `invalid_ttl`; a non-finite or nonnumeric ramp disables it with `invalid_ramp`; finite runtime ramps retain the defensive clamp to 0–100. Other layers can still run, and invalid leaves also record a `config_resolution` error so provider garbage is alertable separately from intentional ramp-downs. A malformed runtime config object, layer-map shape, `requestLocal` value, or explicit `remoteReadTimeoutMs` fails config resolution for the invocation, records `config_error`, and executes the fallback uncached without attempting Redis.
    `cacheConfigProvider` is called for every enabled cache invocation before DialCache performs any cache lookup. Keep it cheap, cache any remote/config-store reads inside the provider, and avoid work that would erase the benefit of a cache hit.

That makes 150 indistinguishable from the explicit, valid 100 control value. A typo, unit mismatch, or bad remote config can therefore enable caching at 100% instead of falling back to the source of truth. Negative values happen to disable the layer, but are mislabeled as an intentional ramped_down decision and do not emit the existing config_resolution error signal.

Reproduced evidence

The current test suite explicitly captures the unsafe behavior:

  • it("clamps finite runtime ramps and disables non-finite ramp config", async () => {
    // Given configured ramps can come from dynamic config and may be outside the normal 0-100 range.
    const clampedCache = new DialCache({
    cacheConfigProvider: async (key) => configFor(
    { [CacheLayer.LOCAL]: 60 },
    {
    [CacheLayer.LOCAL]: key.useCase === "NegativeConfiguredRamp"
    ? -10
    : key.useCase === "OverHundredConfiguredRamp"
    ? 150
    : Number.NaN,
    },
    ),
    });
    let negativeCalls = 0;
    const negativeRamp = clampedCache.cached(async (userId: string) => ({ userId, calls: ++negativeCalls }), {
    keyType: "user_id",
    useCase: "NegativeConfiguredRamp",
    cacheKey: (userId) => userId,
    });
    let overHundredCalls = 0;
    const overHundredRamp = clampedCache.cached(async (userId: string) => ({ userId, calls: ++overHundredCalls }), {
    keyType: "user_id",
    useCase: "OverHundredConfiguredRamp",
    cacheKey: (userId) => userId,
    });
    let nanConfiguredCalls = 0;
    const nanConfiguredRamp = clampedCache.cached(async (userId: string) => ({ userId, calls: ++nanConfiguredCalls }), {
    keyType: "user_id",
    useCase: "NanConfiguredRamp",
    cacheKey: (userId) => userId,
    });
    // When the same keys are read twice.
    const negativeFirst = await clampedCache.enable(async () => await negativeRamp("123"));
    const negativeSecond = await clampedCache.enable(async () => await negativeRamp("123"));
    const overHundredFirst = await clampedCache.enable(async () => await overHundredRamp("456"));
    const overHundredSecond = await clampedCache.enable(async () => await overHundredRamp("456"));
    const nanConfiguredFirst = await clampedCache.enable(async () => await nanConfiguredRamp("789"));
    const nanConfiguredSecond = await clampedCache.enable(async () => await nanConfiguredRamp("789"));
    // Then finite out-of-range ramps clamp to safe terminal behavior, and non-finite config disables caching.
    expect(negativeFirst).toEqual({ userId: "123", calls: 1 });
    expect(negativeSecond).toEqual({ userId: "123", calls: 2 });
    expect(overHundredFirst).toEqual({ userId: "456", calls: 1 });
    expect(overHundredSecond).toEqual({ userId: "456", calls: 1 });
    expect(nanConfiguredFirst).toEqual({ userId: "789", calls: 1 });
    expect(nanConfiguredSecond).toEqual({ userId: "789", calls: 2 });

With a runtime provider returning a local TTL and ramp: 150, two enabled calls for the same key execute the loader only once. The second call is a cache hit because 150 is clamped to full enablement:

{
  "first": "overHundred:1",
  "second": "overHundred:1",
  "loaderCalls": 1
}

The same test shows negative and NaN values execute the loader twice, but only NaN is classified as invalid_ramp; the negative value is silently treated as an intentional ramp-down.

Simplest implementation scope

  • In runtime leaf resolution, accept only numeric, finite ramps in the inclusive range 0..100.
  • Return the existing invalid_ramp disabled result for any other supplied value. Existing recordInvalidLeaf() behavior then emits the bounded config_resolution error metric.
  • Preserve undefined as the existing inherited/defaulted case, 0 as an intentional ramp-down, 100 as full enablement, and deterministic sampling for values strictly between them.
  • Remove clampPercentage() if it becomes unused.
  • Update the README and the existing clamp regression.
  • Do not add a public option, error class, disabled reason, metric label, or policy abstraction.

This disables only the invalid shared-cache layer; normal request-local behavior and any independently valid shared layer continue according to the resolved policy.

Acceptance criteria

  • Runtime ramp values 0 and 100 retain their current intentional boundary behavior.
  • Valid finite values strictly between 0 and 100 retain deterministic cohort behavior.
  • Supplied values below 0 or above 100, NaN, positive/negative infinity, and defensive non-number values disable the affected layer with invalid_ramp rather than being clamped.
  • Invalid ramps emit the existing bounded config_resolution error metric and do not read, populate, or write the affected shared cache layer.
  • An invalid leaf does not disable request-local caching or another independently valid shared layer.
  • Static/default and runtime ramps share the same accepted domain; static invalid values still throw at definition/invocation validation, while runtime invalid values fail open uncached for the affected layer.
  • Documentation no longer tells operators that out-of-range finite runtime ramps are clamped.
  • Add focused tests for -1, a finite value above 100, non-finite values, defensive wrong types, both valid boundaries, and continued operation of other layers.
  • No public API shape change.

Compatibility

This intentionally tightens behavior for invalid provider output. Any provider using values above 100 as “on” must return 100; any provider using negative values as “off” must return 0. Those are already the documented control values.

Related issues

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions