chore: low-severity sweep — import session cap, full compile-error lists, leaf-name-fallback validation warning
#05-T24. Three low-severity cleanups, each TDD'd: - TransportOptions.MaxConcurrentImportSessions (8): BundleSessionStore.Open rejects a new session past the cap (evicting expired entries first), bounding the N×~200 MB decrypted-content footprint. Soft cap (count/add not atomic). - ScriptCompiler.TryCompile now joins ALL forbidden-API violations / compile errors (string.Join instead of [0]) so an operator sees every problem at once; the T15 verdict cache stores the joined string. - SemanticValidator emits an advisory warning when a CallScript target resolves ONLY via the composed leaf-name fallback (dynamic child path, not statically verified) instead of silently accepting it. Claude-Session: https://claude.ai/code/session_01MtdgwpEeCUn6cUA5f1LMPj
This commit is contained in:
@@ -42,15 +42,17 @@ public class ScriptCompiler
|
||||
var (ok, error) = ScriptCompileVerdictCache.GetOrAdd(code, () =>
|
||||
{
|
||||
// Authoritative forbidden-API verdict first — a security violation must
|
||||
// gate the script regardless of whether it otherwise compiles.
|
||||
// gate the script regardless of whether it otherwise compiles. Report
|
||||
// ALL violations (#05-T24), not just the first, so an operator fixing
|
||||
// one forbidden API isn't surprised by a second on the next attempt.
|
||||
var violations = ScriptTrustValidator.FindViolations(code);
|
||||
if (violations.Count > 0)
|
||||
return (false, $"uses forbidden API: {violations[0]}");
|
||||
return (false, $"uses forbidden API: {string.Join("; ", violations)}");
|
||||
|
||||
// Real CSharpScript compile against the runtime-mirroring globals surface.
|
||||
var errors = RoslynScriptCompiler.Compile(code, typeof(ScriptCompileSurface));
|
||||
if (errors.Count > 0)
|
||||
return (false, $"failed to compile: {errors[0]}");
|
||||
return (false, $"failed to compile: {string.Join("; ", errors)}");
|
||||
|
||||
return (true, (string?)null);
|
||||
});
|
||||
|
||||
@@ -119,6 +119,18 @@ public class SemanticValidator
|
||||
}
|
||||
else
|
||||
{
|
||||
// #05-T24: if the target resolved ONLY via the composed
|
||||
// leaf-name fallback (not a same-scope sibling), the child
|
||||
// path is dynamic and cannot be statically verified — surface
|
||||
// an advisory warning instead of accepting it silently.
|
||||
if (!scriptNames.Contains(call.TargetName)
|
||||
&& composedLeafNames.Contains(call.TargetName))
|
||||
{
|
||||
warnings.Add(ValidationEntry.Warning(ValidationCategory.CallTargetNotFound,
|
||||
$"Script '{script.CanonicalName}' call target '{call.TargetName}' matched by composed leaf name only — child path not verified.",
|
||||
script.CanonicalName));
|
||||
}
|
||||
|
||||
ValidateCallParameters(script.CanonicalName, call, scriptParamMap, errors);
|
||||
ValidateCallReturnType(script.CanonicalName, call, scriptReturnMap, errors);
|
||||
|
||||
|
||||
@@ -73,6 +73,26 @@ public sealed class BundleSessionStore : IBundleSessionStore
|
||||
public BundleSession Open(BundleSession session)
|
||||
{
|
||||
ArgumentNullException.ThrowIfNull(session);
|
||||
// #05-T24: bound the number of concurrently-open sessions — each pins a
|
||||
// decrypted bundle (up to MaxBundleSizeMb of plaintext) in memory until
|
||||
// it expires or is applied/cancelled. Re-opening an existing id (overwrite)
|
||||
// never grows the count, so only a genuinely-new session is gated. A soft
|
||||
// cap: the count/add pair is not atomic, so a race may briefly exceed it
|
||||
// by one — acceptable for a memory-pressure guard, not a security invariant.
|
||||
if (!_sessions.ContainsKey(session.SessionId))
|
||||
{
|
||||
var cap = _options.Value.MaxConcurrentImportSessions;
|
||||
if (_sessions.Count >= cap)
|
||||
{
|
||||
// A full store may be full only of stale entries — reclaim first.
|
||||
EvictExpired();
|
||||
if (_sessions.Count >= cap)
|
||||
{
|
||||
throw new InvalidOperationException(
|
||||
$"Too many concurrent import sessions ({cap}). Complete or cancel an in-progress import before starting another.");
|
||||
}
|
||||
}
|
||||
}
|
||||
// Overwrite on collision is defensive: GUIDs are random so practical
|
||||
// collisions don't happen, but if a caller reuses an id we always
|
||||
// honor the latest Open call.
|
||||
|
||||
@@ -28,6 +28,13 @@ public sealed class TransportOptions
|
||||
/// generous headroom for unusually compressible bundles.
|
||||
/// </summary>
|
||||
public int MaxBundleEntryCompressionRatio { get; set; } = 50;
|
||||
/// <summary>
|
||||
/// #05-T24: maximum number of concurrently-open import sessions. Each open
|
||||
/// session pins a fully-decrypted bundle (up to <see cref="MaxBundleSizeMb"/>
|
||||
/// of plaintext) in memory until it expires or is applied/cancelled, so this
|
||||
/// bounds the N×~200 MB decrypted-content footprint the central node can hold.
|
||||
/// </summary>
|
||||
public int MaxConcurrentImportSessions { get; set; } = 8;
|
||||
/// <summary>Gets or sets the maximum number of failed passphrase unlock attempts before a session is locked.</summary>
|
||||
public int MaxUnlockAttemptsPerSession { get; set; } = 3;
|
||||
/// <summary>Gets or sets the maximum number of unlock attempts allowed per IP address per hour.</summary>
|
||||
|
||||
@@ -40,6 +40,19 @@ public class ScriptCompilerTests
|
||||
Assert.Contains("forbidden", result.Error, StringComparison.OrdinalIgnoreCase);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void TryCompile_MultipleForbiddenApis_ReportsAll()
|
||||
{
|
||||
// #05-T24: the failure message must enumerate EVERY forbidden API, not
|
||||
// just the first, so an operator fixing one isn't surprised by the next.
|
||||
var result = _sut.TryCompile(
|
||||
"System.IO.File.ReadAllText(\"x\"); System.Diagnostics.Process.Start(\"y\");",
|
||||
"Test");
|
||||
Assert.True(result.IsFailure);
|
||||
Assert.Contains("System.IO", result.Error, StringComparison.Ordinal);
|
||||
Assert.Contains("Process", result.Error, StringComparison.Ordinal);
|
||||
}
|
||||
|
||||
[Theory]
|
||||
// Namespace alias — the old substring scan never saw "System.IO." spelled out.
|
||||
[InlineData("using IO = System.IO; IO.File.ReadAllText(\"x\");")]
|
||||
|
||||
@@ -105,6 +105,50 @@ public class SemanticValidatorTests
|
||||
Assert.DoesNotContain(result.Errors, e => e.Category == ValidationCategory.CallTargetNotFound);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void Validate_CallScriptCompositionDelegatedByLeafName_EmitsAdvisoryWarning()
|
||||
{
|
||||
// #05-T24: a call target accepted ONLY by the composed leaf-name fallback
|
||||
// (dynamic child path, not statically verifiable) must surface an advisory
|
||||
// warning rather than being silently accepted.
|
||||
var config = new FlattenedConfiguration
|
||||
{
|
||||
InstanceUniqueName = "Instance1",
|
||||
Scripts =
|
||||
[
|
||||
new ResolvedScript { CanonicalName = "LeftMESReceiver.MoveIn", Code = "var x = 1;" },
|
||||
new ResolvedScript { CanonicalName = "MesMoveIn", Code = "await Children[side + \"MESReceiver\"].CallScript(\"MoveIn\", Parameters);" }
|
||||
]
|
||||
};
|
||||
|
||||
var result = _sut.Validate(config);
|
||||
|
||||
Assert.Contains(result.Warnings, w =>
|
||||
w.Category == ValidationCategory.CallTargetNotFound
|
||||
&& w.Message.Contains("leaf name only", StringComparison.Ordinal));
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void Validate_CallScriptSameScopeSibling_EmitsNoLeafWarning()
|
||||
{
|
||||
// A same-scope sibling resolves directly (not the leaf fallback), so no
|
||||
// advisory warning is emitted.
|
||||
var config = new FlattenedConfiguration
|
||||
{
|
||||
InstanceUniqueName = "Instance1",
|
||||
Scripts =
|
||||
[
|
||||
new ResolvedScript { CanonicalName = "Target", Code = "var x = 1;" },
|
||||
new ResolvedScript { CanonicalName = "Caller", Code = "CallScript(\"Target\");" }
|
||||
]
|
||||
};
|
||||
|
||||
var result = _sut.Validate(config);
|
||||
|
||||
Assert.DoesNotContain(result.Warnings, w =>
|
||||
w.Message.Contains("leaf name only", StringComparison.Ordinal));
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void Validate_CallScriptUnknownLeafName_StillReturnsError()
|
||||
{
|
||||
|
||||
@@ -29,6 +29,36 @@ public sealed class BundleSessionStoreTests
|
||||
Summary: new BundleSummary(0, 0, 0, 0, 0, 0, 0, 0),
|
||||
Contents: Array.Empty<ManifestContentEntry>());
|
||||
|
||||
[Fact]
|
||||
public void Open_beyond_MaxConcurrentImportSessions_throws_and_removing_one_admits_a_new_session()
|
||||
{
|
||||
// #05-T24: the store bounds concurrently-open sessions (each pins a
|
||||
// decrypted bundle in memory). The (cap+1)th Open is rejected; freeing a
|
||||
// slot re-admits a new session.
|
||||
var clock = new TestTimeProvider(DateTimeOffset.UtcNow);
|
||||
var opts = Options();
|
||||
var cap = opts.Value.MaxConcurrentImportSessions;
|
||||
var sut = new BundleSessionStore(opts, clock);
|
||||
var future = clock.GetUtcNow().AddMinutes(30);
|
||||
|
||||
var sessions = new List<BundleSession>();
|
||||
for (var i = 0; i < cap; i++)
|
||||
{
|
||||
var s = SessionExpiringAt(future);
|
||||
sut.Open(s);
|
||||
sessions.Add(s);
|
||||
}
|
||||
|
||||
var overflow = SessionExpiringAt(future);
|
||||
var ex = Assert.Throws<InvalidOperationException>(() => sut.Open(overflow));
|
||||
Assert.Contains("Too many concurrent import sessions", ex.Message, StringComparison.Ordinal);
|
||||
|
||||
// Free a slot → the previously-rejected session is admitted.
|
||||
sut.Remove(sessions[0].SessionId);
|
||||
sut.Open(overflow);
|
||||
Assert.NotNull(sut.Get(overflow.SessionId));
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void Open_then_Get_returns_session()
|
||||
{
|
||||
|
||||
Reference in New Issue
Block a user