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
6 changes: 6 additions & 0 deletions src/GenWave.Ads/AdRenderService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -349,6 +349,12 @@ async Task<AdPreviewOutcome> RenderPreviewCoreAsync(
return (null, $"render: stored script no longer parses ({reason})");
}

// SPEC F200.3 — every unknown-tag fold note the re-parse carried reaches the application log at
// INFO, structured (never interpolated): the "booth log" F200.3 names has no dedicated sink
// reachable from GenWave.Ads, only ILogger, so this IS that surface.
foreach (var note in script.Notes)
logger.LogInformation("Ad spot {Id} parse note {Note}", spot.Id, note);

var (bed, bedFailure) = await ResolveBedAsync(spot.BedMediaId, ct);
if (bedFailure is not null)
return (null, bedFailure);
Expand Down
11 changes: 9 additions & 2 deletions src/GenWave.Ads/AdScript.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,5 +5,12 @@ namespace GenWave.Ads;
/// cref="AdScriptValidator.Validate"/> hands back on <see cref="AdScriptValidationResult.Accepted"/>.
/// Render (PLAN T401) reads <see cref="Lines"/> directly for its cast-of-voices assembly.
/// </summary>
/// <param name="Lines">Every parsed line, in script order, each carrying its own voice tag.</param>
public sealed record AdScript(IReadOnlyList<AdScriptLine> Lines);
/// <param name="Lines">Every parsed line, in script order, each carrying its own voice tag — already
/// folded onto the known cast (SPEC F200.1): a line whose original tag was not <see
/// cref="AdScriptParser.AnnouncerTag"/>/<c>VOICE1</c>/<c>VOICE2</c> is attributed to <see
/// cref="AdScriptParser.AnnouncerTag"/> here, its copy kept verbatim.</param>
/// <param name="Notes">One entry per DISTINCT unknown tag the script carried (SPEC F200.1/F200.3), in
/// first-seen order — <c>"unknown-tag:{TAG}"</c>. Empty when every line's tag was already known. Never
/// a validator failure (STORY-467 AC6): <see cref="AdScriptValidator.Validate"/> returns this
/// <see cref="AdScript"/> unchanged on <see cref="AdScriptValidationResult.Accepted"/>.</param>
public sealed record AdScript(IReadOnlyList<AdScriptLine> Lines, IReadOnlyList<string> Notes);
27 changes: 27 additions & 0 deletions src/GenWave.Ads/AdScriptEcho.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
namespace GenWave.Ads;

/// <summary>
/// Bounds an untrusted string before it reaches a validation violation's <c>Reason</c> — the ONE place
/// the CWE-117 log-forging discipline lives, shared by <see cref="AdScriptParser"/> (its own tag
/// echoes) and <see cref="AdScriptValidator"/> (its own stage-direction echo), rather
/// than each duplicating the same const and one-liner (PLAN T552 review N1). Every violation Reason is
/// logged and surfaced verbatim (STORY-390 AC9's 400), so an untrusted script's own text must never
/// reach it unbounded.
/// </summary>
internal static class AdScriptEcho
{
/// <summary>Cap for a value echoed into a violation Reason (the original
/// <c>AdScriptParser.MaxEchoedChars</c>/<c>AdScriptValidator.MaxEchoedChars</c> precedent, PLAN
/// T399 review F6, CWE-117 log forging).</summary>
const int MaxEchoedChars = 120;

/// <summary>Truncates <paramref name="text"/> to <see cref="MaxEchoedChars"/>, appending an
/// ellipsis when it was cut, so a violation Reason never carries an unbounded echo of untrusted
/// text. Bounds LENGTH only — no control-character strip: <see cref="AdScriptParser"/>'s own call
/// sites pass a tag that already matched <c>TagPattern</c> (<c>^[A-Z][A-Z0-9]*$</c>, which admits no
/// control character), and <see cref="AdScriptValidator"/>'s own call sites echo a regex-matched
/// run (a phone-shaped digit group, a stage-direction shape) rather than a whole raw line — a future
/// call site that would echo genuinely unvalidated freeform text into a Reason must add its own
/// control-character strip first.</summary>
public static string ForReason(string text) => text.Length <= MaxEchoedChars ? text : text[..MaxEchoedChars] + "…";
}
21 changes: 21 additions & 0 deletions src/GenWave.Ads/AdScriptParseNotes.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
namespace GenWave.Ads;

/// <summary>
/// The one PUBLIC seam a stored spot's parse notes reach the Host wire through (SPEC F200.3,
/// STORY-467; PLAN T551) — <see cref="AdScriptParser"/> itself stays <see langword="internal"/> (no
/// <c>InternalsVisibleTo</c> widened for GenWave.Host just to reach this one read), so
/// <c>AdsController.ToDto</c> calls <see cref="For"/> instead of the parser directly.
/// </summary>
public static class AdScriptParseNotes
{
/// <summary>Re-parses <paramref name="script"/> structurally — the SAME
/// <c>AdScriptParser.Parse(script, int.MaxValue)</c> re-parse <c>AdRenderService</c> already runs
/// (never a re-validation: the per-line length rule was already enforced at write time) — and
/// returns its <see cref="AdScript.Notes"/>. Empty, never a throw, when <paramref name="script"/>
/// is <see langword="null"/> or no longer parses (e.g. hand-edited since it was saved).</summary>
public static IReadOnlyList<string> For(string? script)
{
var parsed = AdScriptParser.Parse(script ?? "", int.MaxValue);
return parsed is AdScriptValidationResult.Accepted(var parsedScript) ? parsedScript.Notes : [];
}
}
79 changes: 60 additions & 19 deletions src/GenWave.Ads/AdScriptParser.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,9 @@ namespace GenWave.Ads;
/// <summary>
/// The format stage of <see cref="AdScriptValidator"/> (SPEC F160.3, STORY-390 AC1/AC8) — the
/// <c>CrosstalkScriptParser</c> shape narrowed to the ad wire format: <c>TAG: line</c>, 1-3 DISTINCT
/// uppercase-alphanumeric voice tags, <see cref="AnnouncerTag"/> required, each line's text bounded by
/// the caller's per-line char ceiling. Fail-closed, first-rule-wins: the first line/rule that breaks
/// uppercase-alphanumeric voice tags AFTER FOLDING (SPEC F200.1/F200.2 — see this class's own "not
/// every tag is a voice" remarks below), <see cref="AnnouncerTag"/> required, each line's text bounded
/// by the caller's per-line char ceiling. Fail-closed, first-rule-wins: the first line/rule that breaks
/// the shape is the reason returned, never a full list.
///
/// <para>
Expand All @@ -23,20 +24,37 @@ namespace GenWave.Ads;
/// FIRST untagged line among the NON-BLANK lines (blanks are dropped before this numbering runs, so a
/// blank interior line never shifts it).
/// </para>
///
/// <para>
/// <b>Not every <see cref="TagPattern"/>-shaped tag is a voice</b> (SPEC F200.1, STORY-467): only
/// <see cref="KnownTags"/> — <see cref="AnnouncerTag"/>, <c>VOICE1</c>, <c>VOICE2</c> — cast a distinct
/// voice. A tag that matched <see cref="TagPattern"/> but is not in <see cref="KnownTags"/> (e.g.
/// <c>NARRATOR</c>, <c>VOICE 2</c> once its space fails the pattern) folds onto
/// <see cref="AnnouncerTag"/> instead of refusing — its copy is kept, attributed to the announcer — and
/// <see cref="Parse"/> records one <see cref="AdScript.Notes"/> entry per DISTINCT unknown tag
/// (<c>FoldUnknownTags</c>). <see cref="MinVoiceTags"/>/<see cref="MaxVoiceTags"/> and the "no
/// <see cref="AnnouncerTag"/> line" rule both run over the FOLDED lines (SPEC F200.2), so a script that
/// is entirely unknown tags is a valid one-voice, all-<see cref="AnnouncerTag"/> script.
/// </para>
/// </summary>
internal static partial class AdScriptParser
{
/// <summary>The one voice tag every script must carry (SPEC F160.3).</summary>
public const string AnnouncerTag = "ANNOUNCER";

const int MinVoiceTags = 1;
/// <summary>F160.3's "1–3" upper bound, kept as a documented invariant: with <see cref="KnownTags"/>
/// at exactly three and every other tag folding (F200.2), the <c>&gt; MaxVoiceTags</c> arm is only
/// reachable if the known cast ever grows past three.</summary>
const int MaxVoiceTags = 3;

/// <summary>Cap for a tag echoed into a violation reason (the CrosstalkScriptParser
/// <c>MaxEchoedLineChars</c> precedent, F127.11, PLAN T399 review F6) — an untrusted script's tag
/// text reaches a Reason that is logged and surfaced verbatim (STORY-390 AC9's 400), never an
/// unbounded echo.</summary>
const int MaxEchoedChars = 120;
/// <summary>The full known cast (SPEC F200.1) — <see cref="AdCastPicker.Voice1Tag"/>/
/// <see cref="AdCastPicker.Voice2Tag"/> are the SAME two tags <c>AdScriptPromptBuilder</c> tells the
/// writing model about, duplicated here as the one place the parser itself needs to know which tags
/// are cast, never merely shape-matched. Any other <see cref="TagPattern"/>-shaped tag folds onto
/// <see cref="AnnouncerTag"/> (<c>FoldUnknownTags</c>) rather than refusing.</summary>
static readonly IReadOnlySet<string> KnownTags =
new HashSet<string>(StringComparer.Ordinal) { AnnouncerTag, AdCastPicker.Voice1Tag, AdCastPicker.Voice2Tag };

public static AdScriptValidationResult Parse(string rawScript, int maxLineChars)
{
Expand Down Expand Up @@ -65,14 +83,45 @@ public static AdScriptValidationResult Parse(string rawScript, int maxLineChars)
return new AdScriptValidationResult.Refused(violation);
}

var distinctTags = lines.Select(line => line.Tag).Distinct(StringComparer.Ordinal).ToList();
var (foldedLines, notes) = FoldUnknownTags(lines);

var distinctTags = foldedLines.Select(line => line.Tag).Distinct(StringComparer.Ordinal).ToList();
if (distinctTags.Count is < MinVoiceTags or > MaxVoiceTags)
return Refused($"expected {MinVoiceTags}-{MaxVoiceTags} distinct voice tags, got {distinctTags.Count}");

if (!distinctTags.Contains(AnnouncerTag, StringComparer.Ordinal))
return Refused($"no {AnnouncerTag} line appeared — every spot needs the {AnnouncerTag} voice");

return new AdScriptValidationResult.Accepted(new AdScript(lines));
return new AdScriptValidationResult.Accepted(new AdScript(foldedLines, notes));
}

/// <summary>Folds every line whose tag is not in <see cref="KnownTags"/> onto
/// <see cref="AnnouncerTag"/> (SPEC F200.1) — the line's own <see cref="AdScriptLine.Text"/> is kept
/// verbatim, only its <see cref="AdScriptLine.Tag"/> changes. One note per DISTINCT original tag, in
/// first-seen order (SPEC F200.1: "a tag appearing on three lines yields one note"); the tag echoed
/// through <see cref="AdScriptEcho.ForReason"/>, the same untrusted-echo bound every other
/// logged/surfaced tag in this class goes through.</summary>
static (IReadOnlyList<AdScriptLine> Lines, IReadOnlyList<string> Notes) FoldUnknownTags(
IReadOnlyList<AdScriptLine> lines)
{
var foldedLines = new List<AdScriptLine>(lines.Count);
var notes = new List<string>();
var seenUnknownTags = new HashSet<string>(StringComparer.Ordinal);

foreach (var line in lines)
{
if (KnownTags.Contains(line.Tag))
{
foldedLines.Add(line);
continue;
}

foldedLines.Add(line with { Tag = AnnouncerTag });
if (seenUnknownTags.Add(line.Tag))
notes.Add($"unknown-tag:{AdScriptEcho.ForReason(line.Tag)}");
}

return (foldedLines, notes);
}

/// <summary>The plain-sentence pre-pass itself (SPEC F174.6, PLAN T444 ruling): classifies every
Expand Down Expand Up @@ -136,10 +185,10 @@ public static AdScriptValidationResult Parse(string rawScript, int maxLineChars)
static (AdScriptLine? Line, AdScriptViolation? Violation) CheckLine(string tag, string text, int maxLineChars)
{
if (text.Length == 0)
return (null, FormatViolation($"the {EchoForReason(tag)} line has no spoken text"));
return (null, FormatViolation($"the {AdScriptEcho.ForReason(tag)} line has no spoken text"));

if (text.Length > maxLineChars)
return (null, FormatViolation($"the {EchoForReason(tag)} line ({text.Length} chars) exceeds the {maxLineChars}-char per-line budget"));
return (null, FormatViolation($"the {AdScriptEcho.ForReason(tag)} line ({text.Length} chars) exceeds the {maxLineChars}-char per-line budget"));

return (new AdScriptLine(tag, text), null);
}
Expand All @@ -148,14 +197,6 @@ public static AdScriptValidationResult Parse(string rawScript, int maxLineChars)

static AdScriptViolation FormatViolation(string reason) => new(AdScriptRuleIds.Format, reason);

/// <summary>Bounds an untrusted tag echoed into a violation Reason to <see cref="MaxEchoedChars"/>
/// (CWE-117 log forging — PLAN T399 review F6). PLAN T444 ruling: every call site passes a
/// <paramref name="tag"/> that already matched <see cref="TagPattern"/>
/// (<c>^[A-Z][A-Z0-9]*$</c>, which admits no control character), so the control-character strip
/// this method carried is unreachable and has been removed — a future call site that echoes
/// raw, unvalidated text into a Reason must bring that strip back, pinned by a fact.</summary>
static string EchoForReason(string tag) => tag.Length <= MaxEchoedChars ? tag : tag[..MaxEchoedChars] + "…";

// Must start with a letter (PLAN T399 review N4) — a digits-only tag ("12") is not a plausible
// voice name, so a line whose would-be tag is pure digits reads as malformed FORMAT rather than
// silently accepting a nonsense tag. A digit sequence appearing later, inside a line's spoken
Expand Down
7 changes: 6 additions & 1 deletion src/GenWave.Ads/AdScriptRuleIds.cs
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,14 @@ public static class AdScriptRuleIds
/// <summary>The script named a blocklisted real-world brand.</summary>
public const string BrandCollision = "brand_collision";

/// <summary>A phone-shaped digit run does not contain 555.</summary>
/// <summary>A phone-shaped digit run does not contain 555 (no sponsor phone on file), or does not
/// match the sponsor's own number (one is on file).</summary>
public const string PhoneShape = "phone_shape";

/// <summary>A profane word under the <c>everyone</c> audience posture.</summary>
public const string AudiencePosture = "audience_posture";

/// <summary>A parenthetical, bracketed beat, or asterisked aside survived hygiene and still
/// appears in a line's spoken text (SPEC F201.2, STORY-468).</summary>
public const string StageDirection = "stage_direction";
}
4 changes: 3 additions & 1 deletion src/GenWave.Ads/AdScriptValidationRequest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,9 @@ namespace GenWave.Ads;
/// an owner sponsor with a non-blank value here, <see cref="AdScriptValidator"/>'s 555 rule (SPEC
/// F172.5) treats a phone-shaped run whose DIGITS equal this value's digits as allowed — the
/// comparison is formatting-insensitive (punctuation stripped from both sides), the surrounding script
/// text is still read raw. Every OTHER non-555 phone-shaped run still refuses.</param>
/// text is still read raw. Every OTHER phone-shaped run refuses (SPEC F199.3) — including one that
/// itself contains 555 — once a real sponsor phone is on file; only with no phone on file does the
/// looser "contains 555" rule apply.</param>
/// <param name="IsPackOwned">Whether this script belongs to a pack-owned sponsor — the parody posture
/// SPEC F172.5 keeps unchanged for packs (neither skip above ever runs). Defaults <see
/// langword="true"/> (PLAN T438 ruling: fail closed, not fail open) so every caller that predates
Expand Down
Loading
Loading