From 11c33035d173e18dc3002e2b9edd56c79dcf4e8a Mon Sep 17 00:00:00 2001 From: Clint Andrew Hall Date: Tue, 9 Jun 2026 15:45:02 +0200 Subject: [PATCH 1/2] Resolve env-dependent storybook.registry via allow-listed interpolation docset.yml is committed and static, but storybook.registry is environment-dependent (local, per-PR preview, main). Resolving it from an allow-listed environment variable lets one committed value serve every environment without per-env edits. The allow-list is required because docs-builder renders untrusted PR branches, so unrestricted env access would be a secret-exfiltration vector. Co-Authored-By: Claude Opus 4.8 --- docs/syntax/storybook.md | 19 +++ .../BuildContext.cs | 11 +- .../Builder/ConfigurationFile.cs | 20 ++- .../EnvironmentInterpolation.cs | 94 ++++++++++++++ .../IDocumentationContext.cs | 3 + .../Directives/Storybook/StorybookBlock.cs | 34 ++++- .../ApiConfigurationTests.cs | 3 +- .../ConfigurationFileExcludeTests.cs | 1 + ...ConfigurationFileStorybookRegistryTests.cs | 116 ++++++++++++++++++ .../CrossLinkRegistryTests.cs | 1 + .../EnvironmentInterpolationTests.cs | 116 ++++++++++++++++++ .../Directives/DirectiveBaseTests.cs | 8 +- .../Directives/StorybookTests.cs | 62 ++++++++++ .../TestDocumentationSetContext.cs | 1 + .../Framework/CrossLinkResolverAssertions.fs | 1 + 15 files changed, 481 insertions(+), 9 deletions(-) create mode 100644 src/Elastic.Documentation.Configuration/EnvironmentInterpolation.cs create mode 100644 tests/Elastic.Documentation.Configuration.Tests/ConfigurationFileStorybookRegistryTests.cs create mode 100644 tests/Elastic.Documentation.Configuration.Tests/EnvironmentInterpolationTests.cs diff --git a/docs/syntax/storybook.md b/docs/syntax/storybook.md index 71cf58436f..7e7e8f4564 100644 --- a/docs/syntax/storybook.md +++ b/docs/syntax/storybook.md @@ -17,6 +17,25 @@ For local Kibana testing, `yarn storybook_docs shared_ux --serve` serves the reg http://127.0.0.1:6007/storybook-docs/docs_registry.json ``` +## Environment-dependent registry + +The registry URL is often environment-dependent (a local server, a per-PR preview bucket, or the published `main` artifact). Rather than hand-editing `docset.yml` per environment, `registry` supports shell-style environment-variable interpolation with a committed default: + +```yaml +storybook: + registry: ${KIBANA_STORYBOOK_REGISTRY:-https://ci-artifacts.kibana.dev/storybooks/main/storybook-docs/docs_registry.json} +``` + +The committed value is then identical across all environments: + +- `${VAR}` resolves to the value of `VAR`, or empty when unset. +- `${VAR:-default}` resolves to `VAR` when set and non-empty, otherwise to `default`. +- With no environment variable set (for example a `main` build), the committed `default` is used. To target a different registry, export the variable before building, for example `KIBANA_STORYBOOK_REGISTRY=http://127.0.0.1:6007/storybook-docs/docs_registry.json docs-builder serve`. + +Because docs-builder renders untrusted branches, only an explicit allow-list of variable names is interpolated. Currently that is `KIBANA_STORYBOOK_REGISTRY`. Any other variable is left literal and a warning is emitted, so a `docset.yml` can never read arbitrary build secrets such as `${AWS_SECRET_ACCESS_KEY}`. + +When the interpolated registry is unreachable (for example an ephemeral per-PR registry that has not been published yet), docs-builder falls back to the committed default. If the committed default cannot be read either, the build reports an error. + ## Usage Use a registry ID directly: diff --git a/src/Elastic.Documentation.Configuration/BuildContext.cs b/src/Elastic.Documentation.Configuration/BuildContext.cs index 5ea7d914a9..2200c21943 100644 --- a/src/Elastic.Documentation.Configuration/BuildContext.cs +++ b/src/Elastic.Documentation.Configuration/BuildContext.cs @@ -46,6 +46,8 @@ public record BuildContext : IDocumentationSetContext, IDocumentationConfigurati public GitCheckoutInformation Git { get; } + public IEnvironmentVariables Environment { get; } + public IDiagnosticsCollector Collector { get; } public bool Force { get; init; } @@ -74,9 +76,10 @@ public string? UrlPathPrefix public BuildContext( IDiagnosticsCollector collector, ScopedFileSystem fileSystem, - IConfigurationContext configurationContext + IConfigurationContext configurationContext, + IEnvironmentVariables? environment = null ) - : this(collector, fileSystem, fileSystem, configurationContext, ExportOptions.Default, null, null) + : this(collector, fileSystem, fileSystem, configurationContext, ExportOptions.Default, null, null, environment: environment) { } @@ -88,13 +91,15 @@ public BuildContext( IReadOnlySet availableExporters, string? source = null, string? output = null, - GitCheckoutInformation? gitCheckoutInformation = null + GitCheckoutInformation? gitCheckoutInformation = null, + IEnvironmentVariables? environment = null ) { Collector = collector; ReadFileSystem = readFileSystem; WriteFileSystem = writeFileSystem; AvailableExporters = availableExporters; + Environment = environment ?? SystemEnvironmentVariables.Instance; SearchConfiguration = configurationContext.SearchConfiguration; VersionsConfiguration = configurationContext.VersionsConfiguration; ConfigurationFileProvider = configurationContext.ConfigurationFileProvider; diff --git a/src/Elastic.Documentation.Configuration/Builder/ConfigurationFile.cs b/src/Elastic.Documentation.Configuration/Builder/ConfigurationFile.cs index 8a94b087b7..44cdefc879 100644 --- a/src/Elastic.Documentation.Configuration/Builder/ConfigurationFile.cs +++ b/src/Elastic.Documentation.Configuration/Builder/ConfigurationFile.cs @@ -61,6 +61,13 @@ public record ConfigurationFile public string? StorybookRegistry { get; } + /// + /// Environment-independent storybook.registry value (committed default). Non-null only when an allow-listed + /// environment variable changed , so the directive can degrade to the committed + /// registry when the environment-supplied one is unreachable (e.g. an ephemeral PR registry that is not yet published). + /// + public string? StorybookRegistryFallback { get; } + /// /// Resolved API configurations with template and specification file information. /// @@ -248,7 +255,18 @@ public ConfigurationFile(DocumentationSetFile docSetFile, IDocumentationSetConte } if (docSetFile.Storybook is not null) - StorybookRegistry = docSetFile.Storybook.Registry?.Trim(); + { + var interpolated = EnvironmentInterpolation.Interpolate( + docSetFile.Storybook.Registry?.Trim(), + context.Environment, + name => context.EmitWarning( + context.ConfigurationPath, + $"'storybook.registry' references environment variable '{name}' which is not allow-listed for interpolation and is left literal. Allowed: {string.Join(", ", EnvironmentInterpolation.AllowedVariables)}." + ) + ); + StorybookRegistry = interpolated.Value; + StorybookRegistryFallback = interpolated.Fallback; + } // Process products from docset - resolve ProductLinks to Product objects if (docSetFile.Products.Count > 0) diff --git a/src/Elastic.Documentation.Configuration/EnvironmentInterpolation.cs b/src/Elastic.Documentation.Configuration/EnvironmentInterpolation.cs new file mode 100644 index 0000000000..f9e9f1f994 --- /dev/null +++ b/src/Elastic.Documentation.Configuration/EnvironmentInterpolation.cs @@ -0,0 +1,94 @@ +// Licensed to Elasticsearch B.V under one or more agreements. +// Elasticsearch B.V licenses this file to you under the Apache 2.0 License. +// See the LICENSE file in the project root for more information + +using System.Collections.Frozen; +using System.Text; +using System.Text.RegularExpressions; +using Elastic.Documentation; + +namespace Elastic.Documentation.Configuration; + +/// +/// Outcome of interpolating shell-style ${VAR} / ${VAR:-default} expressions into a config value. +/// +/// The resolved value, using environment variables where set and defaults otherwise. +/// +/// The environment-independent value (every expression replaced by its committed default). Non-null only when an +/// allow-listed environment variable actually changed the result, so consumers can degrade to the committed default +/// if the environment-supplied value turns out to be unusable (e.g. an ephemeral PR registry that 404s). +/// +public sealed record InterpolatedValue(string? Value, string? Fallback); + +/// +/// Resolves shell-style ${VAR} and ${VAR:-default} expressions in committed config values. +/// docs-builder renders untrusted PR branches, so interpolation is restricted to an explicit allow-list — naive access +/// to the full process environment would let a malicious docset.yml exfiltrate CI secrets (e.g. ${AWS_SECRET_ACCESS_KEY}). +/// +public static partial class EnvironmentInterpolation +{ + /// Environment variable names that may be interpolated into committed config values. + public static readonly FrozenSet AllowedVariables = + new HashSet(StringComparer.Ordinal) { "KIBANA_STORYBOOK_REGISTRY" }.ToFrozenSet(StringComparer.Ordinal); + + [GeneratedRegex(@"\$\{(?[A-Za-z_][A-Za-z0-9_]*)(?::-(?[^}]*))?\}", RegexOptions.CultureInvariant)] + private static partial Regex ExpressionRegex(); + + /// + /// Interpolates allow-listed environment variables into . Non-allow-listed expressions are + /// left literal (never read from the environment) and reported via . + /// + public static InterpolatedValue Interpolate(string? raw, IEnvironmentVariables environment, Action? onDisallowed = null) + { + if (string.IsNullOrEmpty(raw) || !raw.Contains("${", StringComparison.Ordinal)) + return new InterpolatedValue(raw, null); + + var resolved = new StringBuilder(raw.Length); + var committed = new StringBuilder(raw.Length); + var lastIndex = 0; + var environmentChangedValue = false; + + foreach (Match match in ExpressionRegex().Matches(raw)) + { + var literal = raw[lastIndex..match.Index]; + _ = resolved.Append(literal); + _ = committed.Append(literal); + lastIndex = match.Index + match.Length; + + var name = match.Groups["name"].Value; + var defaultGroup = match.Groups["default"]; + var defaultValue = defaultGroup.Success ? defaultGroup.Value : string.Empty; + + if (!AllowedVariables.Contains(name)) + { + onDisallowed?.Invoke(name); + _ = resolved.Append(match.Value); + _ = committed.Append(match.Value); + continue; + } + + var environmentValue = environment.GetEnvironmentVariable(name); + if (!string.IsNullOrEmpty(environmentValue)) + { + _ = resolved.Append(environmentValue); + environmentChangedValue = true; + } + else + _ = resolved.Append(defaultValue); + + _ = committed.Append(defaultValue); + } + + var tail = raw[lastIndex..]; + _ = resolved.Append(tail); + _ = committed.Append(tail); + + var resolvedValue = resolved.ToString(); + var committedValue = committed.ToString(); + var fallback = environmentChangedValue && !string.Equals(resolvedValue, committedValue, StringComparison.Ordinal) + ? committedValue + : null; + + return new InterpolatedValue(resolvedValue, fallback); + } +} diff --git a/src/Elastic.Documentation/IDocumentationContext.cs b/src/Elastic.Documentation/IDocumentationContext.cs index 72512d4c09..9bc24b4f72 100644 --- a/src/Elastic.Documentation/IDocumentationContext.cs +++ b/src/Elastic.Documentation/IDocumentationContext.cs @@ -22,6 +22,9 @@ public interface IDocumentationSetContext : IDocumentationContext { IDirectoryInfo DocumentationSourceDirectory { get; } GitCheckoutInformation Git { get; } + + /// Environment variables used to resolve env-dependent config values; injectable so tests are deterministic. + IEnvironmentVariables Environment { get; } } public static class DocumentationContextExtensions diff --git a/src/Elastic.Markdown/Myst/Directives/Storybook/StorybookBlock.cs b/src/Elastic.Markdown/Myst/Directives/Storybook/StorybookBlock.cs index 3b37474c8c..fd9e9cf93c 100644 --- a/src/Elastic.Markdown/Myst/Directives/Storybook/StorybookBlock.cs +++ b/src/Elastic.Markdown/Myst/Directives/Storybook/StorybookBlock.cs @@ -155,14 +155,42 @@ private bool TryLoadRegistry(out StorybookRegistry registry) return false; } + if (TryReadRegistry(rawRegistry, out var registryJson, out var error)) + return TryDeserializeRegistry(rawRegistry, registryJson, out registry); + + // Only an environment-supplied registry is treated as best-effort: an ephemeral per-PR URL may not be published + // yet, so degrade to the committed default. A committed/static registry (no env fallback) that fails to read is + // an authoring error and stays a hard error so typos and broken paths don't silently drop every embed. + var fallback = Build.Configuration.StorybookRegistryFallback; + var hasEnvironmentFallback = !string.IsNullOrWhiteSpace(fallback) + && !string.Equals(fallback, rawRegistry, StringComparison.Ordinal); + if (!hasEnvironmentFallback) + { + this.EmitError($"storybook registry could not be read: {rawRegistry}", error); + return false; + } + + this.EmitWarning($"storybook registry '{rawRegistry}' could not be read; falling back to the committed default '{fallback}'."); + + if (TryReadRegistry(fallback!, out registryJson, out error)) + return TryDeserializeRegistry(fallback!, registryJson, out registry); + + this.EmitError($"storybook registry could not be read: {fallback}", error); + return false; + } + + private bool TryReadRegistry(string rawRegistry, out string registryJson, out Exception? error) + { try { - var registryJson = ReadRegistry(rawRegistry); - return TryDeserializeRegistry(rawRegistry, registryJson, out registry); + registryJson = ReadRegistry(rawRegistry); + error = null; + return true; } catch (Exception e) { - this.EmitError($"storybook registry could not be read: {rawRegistry}", e); + registryJson = string.Empty; + error = e; return false; } } diff --git a/tests/Elastic.Documentation.Configuration.Tests/ApiConfigurationTests.cs b/tests/Elastic.Documentation.Configuration.Tests/ApiConfigurationTests.cs index 7f27e0838a..c2193fc514 100644 --- a/tests/Elastic.Documentation.Configuration.Tests/ApiConfigurationTests.cs +++ b/tests/Elastic.Documentation.Configuration.Tests/ApiConfigurationTests.cs @@ -375,7 +375,7 @@ public class ConfigurationFileApiTests [Fact] public void ConfigurationFile_ProcessesNewApiSequenceConfiguration() { - // Arrange + // Arrange var docSetFile = new DocumentationSetFile { Api = new Dictionary @@ -459,5 +459,6 @@ private sealed class MockDocumentationSetContext( public BuildType BuildType => BuildType.Isolated; public IDirectoryInfo DocumentationSourceDirectory => documentationSourceDirectory; public GitCheckoutInformation Git => GitCheckoutInformationFactory.Create(documentationSourceDirectory, fileSystem); + public IEnvironmentVariables Environment => SystemEnvironmentVariables.Instance; } } diff --git a/tests/Elastic.Documentation.Configuration.Tests/ConfigurationFileExcludeTests.cs b/tests/Elastic.Documentation.Configuration.Tests/ConfigurationFileExcludeTests.cs index 1cb21c1e3e..f3582b6e3e 100644 --- a/tests/Elastic.Documentation.Configuration.Tests/ConfigurationFileExcludeTests.cs +++ b/tests/Elastic.Documentation.Configuration.Tests/ConfigurationFileExcludeTests.cs @@ -94,5 +94,6 @@ private sealed class MockDocumentationSetContext( public BuildType BuildType => BuildType.Isolated; public IDirectoryInfo DocumentationSourceDirectory => documentationSourceDirectory; public GitCheckoutInformation Git => GitCheckoutInformationFactory.Create(documentationSourceDirectory, fileSystem); + public IEnvironmentVariables Environment => SystemEnvironmentVariables.Instance; } } diff --git a/tests/Elastic.Documentation.Configuration.Tests/ConfigurationFileStorybookRegistryTests.cs b/tests/Elastic.Documentation.Configuration.Tests/ConfigurationFileStorybookRegistryTests.cs new file mode 100644 index 0000000000..7bc9291f6a --- /dev/null +++ b/tests/Elastic.Documentation.Configuration.Tests/ConfigurationFileStorybookRegistryTests.cs @@ -0,0 +1,116 @@ +// Licensed to Elasticsearch B.V under one or more agreements. +// Elasticsearch B.V licenses this file to you under the Apache 2.0 License. +// See the LICENSE file in the project root for more information + +using System.Collections.Frozen; +using System.IO.Abstractions; +using System.IO.Abstractions.TestingHelpers; +using AwesomeAssertions; +using Elastic.Documentation.Configuration.Builder; +using Elastic.Documentation.Configuration.Products; +using Elastic.Documentation.Configuration.Toc; +using Elastic.Documentation.Configuration.Versions; +using Elastic.Documentation.Diagnostics; +using Nullean.ScopedFileSystem; + +namespace Elastic.Documentation.Configuration.Tests; + +public class ConfigurationFileStorybookRegistryTests +{ + private const string Default = "https://ci-artifacts.kibana.dev/storybooks/main/storybook-docs/docs_registry.json"; + private const string Expression = $"${{KIBANA_STORYBOOK_REGISTRY:-{Default}}}"; + + [Fact] + public void UnsetVariable_ResolvesToCommittedDefault_WithNoFallback() + { + var config = CreateConfiguration(Expression, new MockEnvironment()); + + config.StorybookRegistry.Should().Be(Default); + config.StorybookRegistryFallback.Should().BeNull(); + } + + [Fact] + public void SetVariable_ResolvesToEnvironmentValue_AndExposesDefaultAsFallback() + { + const string prRegistry = "https://ci-artifacts.kibana.dev/storybooks/pr-42/storybook-docs/docs_registry.json"; + var config = CreateConfiguration(Expression, new MockEnvironment { ["KIBANA_STORYBOOK_REGISTRY"] = prRegistry }); + + config.StorybookRegistry.Should().Be(prRegistry); + config.StorybookRegistryFallback.Should().Be(Default); + } + + [Fact] + public void DisallowedVariable_IsLeftLiteral_AndWarns() + { + var collector = new DiagnosticsCollector([]); + var config = CreateConfiguration("${AWS_SECRET_ACCESS_KEY:-fallback}", new MockEnvironment { ["AWS_SECRET_ACCESS_KEY"] = "super-secret" }, collector); + + config.StorybookRegistry.Should().Be("${AWS_SECRET_ACCESS_KEY:-fallback}"); + config.StorybookRegistry.Should().NotContain("super-secret"); + } + + private static ConfigurationFile CreateConfiguration(string registry, IEnvironmentVariables environment, DiagnosticsCollector? collector = null) + { + collector ??= new DiagnosticsCollector([]); + var root = Paths.WorkingDirectoryRoot.FullName; + var configFilePath = Path.Join(root, "docs", "_docset.yml"); + var fileSystem = new MockFileSystem(new Dictionary + { + { configFilePath, new MockFileData("") } + }, root); + + var configPath = fileSystem.FileInfo.New(configFilePath); + var docsDir = fileSystem.DirectoryInfo.New(Path.Join(root, "docs")); + + var context = new MockDocumentationSetContext(collector, fileSystem, configPath, docsDir, environment); + var versionsConfig = new VersionsConfiguration { VersioningSystems = new Dictionary() }; + var productsConfig = new ProductsConfiguration + { + Products = new Dictionary().ToFrozenDictionary(), + PublicReferenceProducts = new Dictionary().ToFrozenDictionary(), + ProductDisplayNames = new Dictionary().ToFrozenDictionary() + }; + + var docSet = new DocumentationSetFile + { + Project = "test", + TableOfContents = [], + Storybook = new DocumentationSetStorybook { Registry = registry } + }; + + return new ConfigurationFile(docSet, context, versionsConfig, productsConfig); + } + + private sealed class MockEnvironment : IEnvironmentVariables + { + private readonly Dictionary _variables = [with(StringComparer.Ordinal)]; + + public string? this[string name] + { + set => _variables[name] = value; + } + + public string? GetEnvironmentVariable(string name) => _variables.GetValueOrDefault(name); + + public bool IsRunningOnCI => false; + } + + private sealed class MockDocumentationSetContext( + IDiagnosticsCollector collector, + IFileSystem fileSystem, + IFileInfo configurationPath, + IDirectoryInfo documentationSourceDirectory, + IEnvironmentVariables environment) + : IDocumentationSetContext + { + public IDiagnosticsCollector Collector => collector; + public ScopedFileSystem ReadFileSystem => WriteFileSystem; + public ScopedFileSystem WriteFileSystem { get; } = FileSystemFactory.ScopeCurrentWorkingDirectoryForWrite(fileSystem); + public IDirectoryInfo OutputDirectory => fileSystem.DirectoryInfo.New(Path.Join(Paths.WorkingDirectoryRoot.FullName, ".artifacts")); + public IFileInfo ConfigurationPath => configurationPath; + public BuildType BuildType => BuildType.Isolated; + public IDirectoryInfo DocumentationSourceDirectory => documentationSourceDirectory; + public GitCheckoutInformation Git => GitCheckoutInformationFactory.Create(documentationSourceDirectory, fileSystem); + public IEnvironmentVariables Environment => environment; + } +} diff --git a/tests/Elastic.Documentation.Configuration.Tests/CrossLinkRegistryTests.cs b/tests/Elastic.Documentation.Configuration.Tests/CrossLinkRegistryTests.cs index 5e34366c80..b259fdff5a 100644 --- a/tests/Elastic.Documentation.Configuration.Tests/CrossLinkRegistryTests.cs +++ b/tests/Elastic.Documentation.Configuration.Tests/CrossLinkRegistryTests.cs @@ -140,5 +140,6 @@ private sealed class MockDocumentationSetContext( public BuildType BuildType => BuildType.Isolated; public IDirectoryInfo DocumentationSourceDirectory => documentationSourceDirectory; public GitCheckoutInformation Git => GitCheckoutInformationFactory.Create(documentationSourceDirectory, fileSystem); + public IEnvironmentVariables Environment => SystemEnvironmentVariables.Instance; } } diff --git a/tests/Elastic.Documentation.Configuration.Tests/EnvironmentInterpolationTests.cs b/tests/Elastic.Documentation.Configuration.Tests/EnvironmentInterpolationTests.cs new file mode 100644 index 0000000000..45e6e970e8 --- /dev/null +++ b/tests/Elastic.Documentation.Configuration.Tests/EnvironmentInterpolationTests.cs @@ -0,0 +1,116 @@ +// Licensed to Elasticsearch B.V under one or more agreements. +// Elasticsearch B.V licenses this file to you under the Apache 2.0 License. +// See the LICENSE file in the project root for more information + +using AwesomeAssertions; + +namespace Elastic.Documentation.Configuration.Tests; + +public class EnvironmentInterpolationTests +{ + private const string AllowedVar = "KIBANA_STORYBOOK_REGISTRY"; + private const string DefaultRegistry = "https://ci-artifacts.kibana.dev/storybooks/main/storybook-docs/docs_registry.json"; + + [Fact] + public void DefaultIsUsed_WhenAllowedVariableUnset() + { + var environment = new MockEnvironment(); + + var result = EnvironmentInterpolation.Interpolate($"${{{AllowedVar}:-{DefaultRegistry}}}", environment); + + result.Value.Should().Be(DefaultRegistry); + result.Fallback.Should().BeNull(); + } + + [Fact] + public void EnvironmentValueIsUsed_WhenAllowedVariableSet() + { + const string prRegistry = "https://ci-artifacts.kibana.dev/storybooks/pr-42/storybook-docs/docs_registry.json"; + var environment = new MockEnvironment { [AllowedVar] = prRegistry }; + + var result = EnvironmentInterpolation.Interpolate($"${{{AllowedVar}:-{DefaultRegistry}}}", environment); + + result.Value.Should().Be(prRegistry); + result.Fallback.Should().Be(DefaultRegistry); + } + + [Fact] + public void EmptyEnvironmentValue_FallsBackToDefault() + { + var environment = new MockEnvironment { [AllowedVar] = "" }; + + var result = EnvironmentInterpolation.Interpolate($"${{{AllowedVar}:-{DefaultRegistry}}}", environment); + + result.Value.Should().Be(DefaultRegistry); + result.Fallback.Should().BeNull(); + } + + [Fact] + public void DisallowedVariable_IsNotReadFromEnvironment_AndLeftLiteral() + { + var environment = new MockEnvironment { ["AWS_SECRET_ACCESS_KEY"] = "super-secret" }; + string? reported = null; + + var result = EnvironmentInterpolation.Interpolate("${AWS_SECRET_ACCESS_KEY}", environment, name => reported = name); + + result.Value.Should().Be("${AWS_SECRET_ACCESS_KEY}"); + result.Value.Should().NotContain("super-secret"); + result.Fallback.Should().BeNull(); + reported.Should().Be("AWS_SECRET_ACCESS_KEY"); + } + + [Fact] + public void DisallowedVariableWithDefault_DoesNotResolveToDefault() + { + var environment = new MockEnvironment(); + + var result = EnvironmentInterpolation.Interpolate("${SECRET:-fallback}", environment); + + result.Value.Should().Be("${SECRET:-fallback}"); + } + + [Fact] + public void AllowedVariableWithoutDefault_Unset_ResolvesToEmpty() + { + var environment = new MockEnvironment(); + + var result = EnvironmentInterpolation.Interpolate($"prefix-${{{AllowedVar}}}-suffix", environment); + + result.Value.Should().Be("prefix--suffix"); + } + + [Fact] + public void NoExpression_ReturnsRawUnchanged() + { + var environment = new MockEnvironment { [AllowedVar] = "ignored" }; + + var result = EnvironmentInterpolation.Interpolate(DefaultRegistry, environment); + + result.Value.Should().Be(DefaultRegistry); + result.Fallback.Should().BeNull(); + } + + [Fact] + public void NullInput_ReturnsNull() + { + var result = EnvironmentInterpolation.Interpolate(null, new MockEnvironment()); + + result.Value.Should().BeNull(); + result.Fallback.Should().BeNull(); + } + + private sealed class MockEnvironment : IEnvironmentVariables + { + private readonly Dictionary _variables = [with(StringComparer.Ordinal)]; + + public string? this[string name] + { + set => _variables[name] = value; + } + + public string? GetEnvironmentVariable(string name) => + _variables.GetValueOrDefault(name); + + public bool IsRunningOnCI => false; + } +} diff --git a/tests/Elastic.Markdown.Tests/Directives/DirectiveBaseTests.cs b/tests/Elastic.Markdown.Tests/Directives/DirectiveBaseTests.cs index 48edbc964b..2a1f7691df 100644 --- a/tests/Elastic.Markdown.Tests/Directives/DirectiveBaseTests.cs +++ b/tests/Elastic.Markdown.Tests/Directives/DirectiveBaseTests.cs @@ -3,6 +3,7 @@ // See the LICENSE file in the project root for more information using System.IO.Abstractions.TestingHelpers; using AwesomeAssertions; +using Elastic.Documentation; using Elastic.Documentation.Configuration; using Elastic.Markdown.IO; using Elastic.Markdown.Myst.Directives; @@ -71,7 +72,9 @@ protected DirectiveTest(ITestOutputHelper output, [LanguageInjection("markdown") Collector = new TestDiagnosticsCollector(output); var configurationContext = TestHelpers.CreateConfigurationContext(FileSystem); - var context = new BuildContext(Collector, FileSystemFactory.ScopeCurrentWorkingDirectory(FileSystem), configurationContext); + // ReSharper disable once VirtualMemberCallInConstructor + var environment = GetEnvironment(); + var context = new BuildContext(Collector, FileSystemFactory.ScopeCurrentWorkingDirectory(FileSystem), configurationContext, environment); var linkResolver = new TestCrossLinkResolver(); Set = new DocumentationSet(context, logger, linkResolver); File = Set.TryFindDocument(FileSystem.FileInfo.New("docs/index.md")) as MarkdownFile ?? throw new NullReferenceException(); @@ -89,6 +92,9 @@ protected virtual void AddToFileSystem(MockFileSystem fileSystem) { } protected virtual string? GetDocsetExtraYaml() => null; + /// Override to inject a deterministic environment for env-dependent config (e.g. storybook.registry). + protected virtual IEnvironmentVariables? GetEnvironment() => null; + public virtual async ValueTask InitializeAsync() { _ = Collector.StartAsync(TestContext.Current.CancellationToken); diff --git a/tests/Elastic.Markdown.Tests/Directives/StorybookTests.cs b/tests/Elastic.Markdown.Tests/Directives/StorybookTests.cs index 9180a132a2..4e380ca6ad 100644 --- a/tests/Elastic.Markdown.Tests/Directives/StorybookTests.cs +++ b/tests/Elastic.Markdown.Tests/Directives/StorybookTests.cs @@ -4,6 +4,7 @@ using System.IO.Abstractions.TestingHelpers; using AwesomeAssertions; +using Elastic.Documentation; using Elastic.Documentation.Diagnostics; using Elastic.Markdown.Myst.Directives.Storybook; @@ -110,6 +111,67 @@ public void RendersInlineStory() } } +/// Deterministic so storybook interpolation tests don't depend on the host shell. +internal sealed class TestEnvironmentVariables : IEnvironmentVariables +{ + private readonly Dictionary _variables = [with(StringComparer.Ordinal)]; + + public string? this[string name] + { + set => _variables[name] = value; + } + + public string? GetEnvironmentVariable(string name) => _variables.GetValueOrDefault(name); + + public bool IsRunningOnCI => false; +} + +public class StorybookInterpolatedRegistryTests(ITestOutputHelper output) : StorybookRegistryTest(output, +""" +:::{storybook} +:id: kibana:shared_ux:ai-components-aibutton--default +::: +""" +) +{ + protected override IEnvironmentVariables? GetEnvironment() => new TestEnvironmentVariables(); + + protected override string? GetDocsetExtraYaml() => +""" +storybook: + registry: ${KIBANA_STORYBOOK_REGISTRY:-docs_registry.json} +"""; + + [Fact] + public void ResolvesDefaultWhenEnvironmentVariableUnset() => + Block!.StoryId.Should().Be("ai-components-aibutton--default"); +} + +public class StorybookDisallowedRegistryVariableTests(ITestOutputHelper output) : StorybookRegistryTest(output, +""" +:::{storybook} +:id: kibana:shared_ux:ai-components-aibutton--default +::: +""" +) +{ + // A disallowed variable must not be read even when present in the environment. + protected override IEnvironmentVariables? GetEnvironment() => + new TestEnvironmentVariables { ["AWS_SECRET_ACCESS_KEY"] = "super-secret" }; + + protected override string? GetDocsetExtraYaml() => +""" +storybook: + registry: ${AWS_SECRET_ACCESS_KEY:-docs_registry.json} +"""; + + [Fact] + public void WarnsAndLeavesExpressionLiteral() => + Collector.Diagnostics.Should().Contain(d => + d.Severity == Severity.Warning + && d.Message.Contains("not allow-listed for interpolation")); +} + public class StorybookStructuredReferenceTests(ITestOutputHelper output) : StorybookRegistryTest(output, """ :::{storybook} diff --git a/tests/Navigation.Tests/TestDocumentationSetContext.cs b/tests/Navigation.Tests/TestDocumentationSetContext.cs index 288dae691f..80c1192b06 100644 --- a/tests/Navigation.Tests/TestDocumentationSetContext.cs +++ b/tests/Navigation.Tests/TestDocumentationSetContext.cs @@ -108,6 +108,7 @@ public TestDocumentationSetContext(IFileSystem fileSystem, public IDirectoryInfo DocumentationSourceDirectory { get; } public GitCheckoutInformation Git { get; } public IFileInfo ConfigurationPath { get; } + public IEnvironmentVariables Environment { get; init; } = SystemEnvironmentVariables.Instance; /// public BuildType BuildType { get; set; } diff --git a/tests/authoring/Framework/CrossLinkResolverAssertions.fs b/tests/authoring/Framework/CrossLinkResolverAssertions.fs index 0171ace235..53b55f939f 100644 --- a/tests/authoring/Framework/CrossLinkResolverAssertions.fs +++ b/tests/authoring/Framework/CrossLinkResolverAssertions.fs @@ -36,6 +36,7 @@ module CrossLinkResolverAssertions = member _.ConfigurationPath = mockFileSystem.FileInfo.New("mock_docset.yml") member _.OutputDirectory = mockFileSystem.DirectoryInfo.New(".artifacts") member _.BuildType = BuildType.Isolated + member _.Environment = SystemEnvironmentVariables.Instance } let redirectFileParser = RedirectFile(docContext, mockRedirectsFile) redirectFileParser.Redirects From 08611cd3f280f1d8f4099d21bd1cba22c61c45e6 Mon Sep 17 00:00:00 2001 From: Clint Andrew Hall Date: Tue, 9 Jun 2026 16:19:55 +0200 Subject: [PATCH 2/2] Assert warning is emitted for disallowed interpolation variable The DisallowedVariable_IsLeftLiteral_AndWarns test named the warning path but never verified it. Co-Authored-By: Claude Opus 4.8 --- .../ConfigurationFileStorybookRegistryTests.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/Elastic.Documentation.Configuration.Tests/ConfigurationFileStorybookRegistryTests.cs b/tests/Elastic.Documentation.Configuration.Tests/ConfigurationFileStorybookRegistryTests.cs index 7bc9291f6a..d86c7f1f90 100644 --- a/tests/Elastic.Documentation.Configuration.Tests/ConfigurationFileStorybookRegistryTests.cs +++ b/tests/Elastic.Documentation.Configuration.Tests/ConfigurationFileStorybookRegistryTests.cs @@ -47,6 +47,7 @@ public void DisallowedVariable_IsLeftLiteral_AndWarns() config.StorybookRegistry.Should().Be("${AWS_SECRET_ACCESS_KEY:-fallback}"); config.StorybookRegistry.Should().NotContain("super-secret"); + collector.Warnings.Should().Be(1, "a disallowed interpolation variable must emit exactly one warning"); } private static ConfigurationFile CreateConfiguration(string registry, IEnvironmentVariables environment, DiagnosticsCollector? collector = null)