diff --git a/COMPATIBILITY.md b/COMPATIBILITY.md index d9cfba82..3996d934 100644 --- a/COMPATIBILITY.md +++ b/COMPATIBILITY.md @@ -553,7 +553,7 @@ The walk goes through open shadow roots and closed ones that page script attache - A runner calls `WebEngine.PrepareAsync`, `Init`, `FinishAsync`, and `DisposeAsync` as upstream's `prepare`, `init`, `finish`, and `dispose`, with `EnginePrepareInfo`, `EnginePrepareResult`, `EngineInitInfo`, and `EngineFinishInfo`; an attempt is `StartAsync` and the dispose of its session (`startAttempt` and `endAttempt`). The hand-off to a worker travels in the same `E2E_WEB_BROWSERS__` environment variable, capped at 16384 bytes. The port has no runner, so an engine whose `Init` never ran leases for the one attempt it starts, as upstream's standalone run does: it prepares one slot of a run of its own, reading `EngineStartOptions.Env` (the process environment by default), and gives the lease back, and sweeps, when the session is disposed, with the default cleanup timeout for each step (the engine does not see the session's). `EngineStartOptions.AttemptId` names the attempt to a per-attempt lease - Differences from upstream's lease flow, all from the port's engine. It keeps no browser connection between attempts, so a worker dials the endpoint of the lease it holds at every attempt, and leases a replacement only after an attempt attached to a browser that was no longer connected when the attempt ended (upstream: when the next attempt finds the connection dropped). A start that fails gives its lease back itself, since there is no `endAttempt` to call after it. A lease that arrives too late, after the attempt was cancelled or the worker disposed, is given back under a cleanup budget of its own (upstream passes the signal that already aborted). Upstream's `state` capability, which an attempt-scoped provider drops, does not exist in the port, so only `app.clearState()` is refused (`UNSUPPORTED_CAPABILITY`, naming the scope) - The lease tests are `BrowserProvider/` in `E2E.Tests`, from `browser-provider.test.ts`. Not ported: "rejects a sweep that is not a function" and "rejects a record that is not a function" (both are methods of the interface), "releases the replacement on dispose even when ending the attempt failed" (the port's `DisposeAsync` does not end an attempt), and "refuses, at the start and named, a recording without a start time and a stop" (a recording is typed). The other tests of the recording group are ported in full, on a real Chromium the provider leases (upstream drives a scripted page), through `StartVideoAsync` and `StopVideoAsync` of the engine session. Also not ported: "knows a recording by a parseable start time and a stop" of `provider-recording.test.ts`. The rest of that file is `ProviderRecording/`, and `provider-downloads.test.ts` is `ProviderDownloads/`, on a real Chrome over CDP, with the `Browser` fixture over the engine session -- `Browser.OnDialogAsync` and `Browser.WaitForDownloadAsync` are upstream's `onDialog` and `waitForDownload`. `DialogHandler` is upstream's `DialogHandler` (`DialogHandler.Accept` and `Dismiss` are its `'accept'` and `'dismiss'`, and a `Func` converts to it), `Dialog.Type` is a `DialogType` (`Alert`, `Confirm`, `Prompt`, `BeforeUnload`) where upstream's is a string union, and `OnDialogAsync` returns `Func` for upstream's unsubscribe. The engine side is `DialogRouter` (`dialogs.ts`) and `ErrorLatch` and `WebSupport.SanitizeFilename` (`support.ts`), and `WebSession.ArtifactPath` is the surface's `artifactPath`. Differences: the engine session has `OnDialogAsync` and `WaitForDownloadAsync` (default members of `IBrowserSession`), and the `Browser` fixture attaches the file to the step (`attachArtifact`), since the port's fixture holds no context; `BrowserDownload` is the result upstream types inline. A dialog handler's failure is latched and fails the next step, as a route handler's does, but the port has no `settleAttempt`, so one that no step follows is dropped (upstream fails the attempt with it). A test framework's assertion is not an `E2EException`, so one thrown in a handler fails the next step as `ENGINE_FAILURE`, `dialog handler failed: ...`, where upstream's `expect` failure keeps its `ASSERTION_FAILED`. A download is a `download` artifact of the `browser.waitForDownload` step, with the media type of its extension. The port has no artifact store, so upstream's rewrite of a text-like download through the secret ledger has no port: the record says `incomplete`, as upstream's does for a session without secrecy. Not ported: "a dialog handler assertion that no step follows" and "fails a %s handler assertion at the attempt end when no step follows it" of `web-platform.test.ts` (no `settleAttempt`), and nothing else. The frame tests of `dialogs-frames.e2e.ts` are `DialogsFrames/FramesTests`, and the download tests trigger the link with `browser.locator('a[download]')` as upstream does +- `Browser.OnDialogAsync` and `Browser.WaitForDownloadAsync` are upstream's `onDialog` and `waitForDownload`. `DialogHandler` is upstream's `DialogHandler` (`DialogHandler.Accept` and `Dismiss` are its `'accept'` and `'dismiss'`, and a `Func` converts to it), `Dialog.Type` is a `DialogType` (`Alert`, `Confirm`, `Prompt`, `BeforeUnload`) where upstream's is a string union, and `OnDialogAsync` returns `Func` for upstream's unsubscribe. The engine side is `DialogRouter` (`dialogs.ts`) and `ErrorLatch` and `WebSupport.SanitizeFilename` (`support.ts`), and `WebSession.ArtifactPath` is the surface's `artifactPath`. Differences: the engine session has `OnDialogAsync` and `WaitForDownloadAsync` (default members of `IBrowserSession`), and the `Browser` fixture attaches the file to the step (`attachArtifact`), since the port's fixture holds no context; `BrowserDownload` is the result upstream types inline. A dialog handler's failure is latched and fails the next step, as a route handler's does, but the port has no `settleAttempt`, so one that no step follows is dropped (upstream fails the attempt with it). A test framework's assertion is not an `E2EException`, so one thrown in a handler fails the next step as `ENGINE_FAILURE`, `dialog handler failed: ...`, where upstream's `expect` failure keeps its `ASSERTION_FAILED`. A download is a `download` artifact of the `browser.waitForDownload` step, with the media type of its extension. The port has no artifact store (`ArtifactStore`, `createAttemptArtifacts`), so `AttemptScope.AttachArtifact` does what upstream's `sink.register` does for a download: when the session has a secret value and the media type is text-like (`text/*` or `application/json`), it rewrites the file through the redactor (`ArtifactPaths.RedactDownload`, upstream's `redactDownload`) and the record says `complete`; any other download, one it cannot decode as UTF-8, one containing NUL bytes, or a session without a secret value stays as served and the record says `incomplete`. Unlike upstream, the port rejects NUL-containing text conservatively so BOM-less UTF-16 cannot hide secrets between NULs and still be marked `complete`. `DownloadRedactionTests` extends the binary-download case with a BOM-less UTF-16LE CSV regression. `AttemptScope.ArtifactsDir` is upstream's `ArtifactSink.dir`. Not ported: the two download tests of `filled-secrets.test.ts` and the download case of `engine-secrets.test.ts` (no credentials config, runner fixtures, or store). Not ported: "a dialog handler assertion that no step follows" and "fails a %s handler assertion at the attempt end when no step follows it" of `web-platform.test.ts` (no `settleAttempt`), and nothing else. The frame tests of `dialogs-frames.e2e.ts` are `DialogsFrames/FramesTests`, and the download tests trigger the link with `browser.locator('a[download]')` as upstream does - With `ReconnectEndpoint`, as upstream, each attempt rides the default context of a fresh browser from `CdpEndpoint`, refuses a browser an earlier attempt of the same `WebEngine` rode, and reconnects once to the same browser and page after a transport drop. `Headers`, `BasicAuth`, `UserAgent`, `Locale`, and `TimezoneId` are `INVALID_CONFIG`, and `app.clearState()` is `UNSUPPORTED_CAPABILITY`. After a reconnect, `PerformAsync`, the agent's key press, and the viewport swipe fail with `NODE_STALE` until the next observation; the browser fixture's keyboard and mouse are direct input and keep working. As upstream, the reconnect is spent from the budget of the operation that found the transport dropped, both for the CDP dial and for the operation itself - `BasicAuth` takes a `Secret` password. As upstream, the attempt redacts the password and the `Authorization` credential the browser sends with it, padded and not, as text only: it does not withhold screenshot pixels, and the agent cannot fill it by name. The port has no engine secrets to declare, so the engine resolves the password through the attempt when it starts. A plain string password is not redacted - An `InitScripts` entry is a `WebInitScript`: source (a string converts to one), `WebInitScript.FromPath`, or `WebInitScript.FromFunction`. A C# lambda cannot run in the page, so a function is its JavaScript source. A relative path resolves against the project root (`EngineStartOptions.ProjectRoot`, which `E2ESession` passes from `E2ESessionOptions.ProjectRoot`). Each attempt reads the files when its session starts; upstream reads them once per process @@ -673,6 +673,8 @@ Upstream writes `report.json`, a `report-1` document of the whole run (`report/b ## App and browser fixtures +Text downloads are rescanned through the final secrecy ledger before their report records are read, including values filled after attachment. Fragment scans also recognize ASCII secrets written with JSON Unicode escapes, once or twice quoted. These safeguards extend upstream's attachment-time scan and ASCII spelling shortcut; stream redaction keeps its existing buffering. The .NET-only regressions are `DownloadRedactionTests.Rescans_downloads_with_values_registered_after_attachment`, `Reports_a_previously_complete_download_incomplete_when_the_final_scan_fails`, and `Redacts_JSON_Unicode_escaped_ASCII_secrets_before_reporting_complete`. + `App.RestartAsync` closes the tab and opens a new one in the same browser context, so cookies and storage survive. `App.ClearStateAsync` replaces the context with a clean one. Both then reopen the app at its base URL, as upstream, and keep a viewport set with `Browser.SetViewportAsync`. Without a base URL they leave a blank page. Differences from upstream: diff --git a/docs-site/src/content/docs/get-started/web.mdx b/docs-site/src/content/docs/get-started/web.mdx index 1d5eae84..2fd3c8d6 100644 --- a/docs-site/src/content/docs/get-started/web.mdx +++ b/docs-site/src/content/docs/get-started/web.mdx @@ -107,6 +107,9 @@ The first run asks the model to perform each `ActAsync`. A passing run records e ## Next +Text downloads are rescanned before reporting with all secrets known by then, +including later fills and ASCII secrets written as JSON Unicode escapes. + diff --git a/skills/e2e-dotnet/SKILL.md b/skills/e2e-dotnet/SKILL.md index 2d4419f6..56f4fd16 100644 --- a/skills/e2e-dotnet/SKILL.md +++ b/skills/e2e-dotnet/SKILL.md @@ -11,6 +11,9 @@ judge the screen. `Screen`, `App`, `Browser`, and `Expect` make exact interactions and checks without a model. A verified agent step is recorded and replayed on the next run with no model call; judgments still run live. +Text downloads are rescanned before reporting with all secrets known by then, +including later fills and JSON Unicode escapes of ASCII secrets. + ## Start here: pick the reference for the job Read the one reference that matches the request before doing anything else. @@ -38,6 +41,8 @@ trip. The installed tool prints the same text with `e2e guide ` Two sibling skills install with `npx skills add hardkoded/e2e-dotnet --skill `. `create-verification-skill` writes a project-local `verify-` skill. `migrate-to-e2e` moves an existing Playwright .NET, Selenium .NET, Appium, Cypress, Detox, or Maestro suite to e2e one slice at a time, with the old suite running beside the new one and a `MIGRATION.md` progress file. Hosted Limrun devices are covered in topic `mobile`. +Text downloads are redacted only when they are UTF-8 without NUL bytes; other files stay as served and `incomplete`. See `writing-tests` for download handling. + ## Quick start A config with one web target and a model, and a test that drives one goal diff --git a/skills/e2e-dotnet/references/writing-tests.md b/skills/e2e-dotnet/references/writing-tests.md index 5ffa8dcd..54e5c253 100644 --- a/skills/e2e-dotnet/references/writing-tests.md +++ b/skills/e2e-dotnet/references/writing-tests.md @@ -453,6 +453,9 @@ await Browser.OnDialogAsync(async dialog => ### Download a file +Before reporting, text downloads are scanned again with the final secrecy ledger, +including secrets filled later and ASCII secrets written as JSON Unicode escapes. + ```csharp var file = await Browser.WaitForDownloadAsync(() => Screen.GetByRole("link", "Export CSV").TapAsync()); @@ -463,7 +466,7 @@ The file is a `download` artifact of the step, saved under the attempt's `downloads` folder with a counted name (`downloads/001-report.csv`). Read it through `AbsolutePath`, which works with a hosted browser whose provider serves `Downloads` too; `Path` is relative to the attempt's artifact -directory, not to the project. `SuggestedFilename` is the name the page or +directory, not to the project. A text download (`text/*` or `application/json`) is rewritten in place when the session holds a secret value: every value, fragment, and encoded form of it becomes ``, a byte order mark stays, and the report records the artifact as `complete`; a binary file, a file that is not UTF-8 or contains NUL bytes (including BOM-less UTF-16), or a session with no secret leaves it as served and `incomplete`. `SuggestedFilename` is the name the page or server gave it. A trigger that throws fails the call with its own error. One that returns and starts no download within the timeout fails with `ACTION_FAILED`, naming how long it waited and how long the trigger took. Without `Downloads` on the provider, diff --git a/src/E2E/Agent/Agent.cs b/src/E2E/Agent/Agent.cs index d4ca36d0..c25a1b83 100644 --- a/src/E2E/Agent/Agent.cs +++ b/src/E2E/Agent/Agent.cs @@ -12,6 +12,7 @@ using E2E.Cache; using E2E.Engine; using E2E.Internal; +using E2E.Reports; namespace E2E; @@ -1298,18 +1299,36 @@ public Redactor Redactor public List Acts { get; } = []; - private readonly List<(string Id, string Kind, string Path)> _attached = []; + private readonly List<(string Id, string Kind, string Path, ArtifactRedaction? Redaction)> _attached = []; + + /// The attempt's artifact directory (upstream's ArtifactSink.dir); null when the scope has none, as in a unit test. + public string? ArtifactsDir { get; init; } /// - /// The files fixtures kept for the attempt, such as a download, as the id the step names, the kind, and the path under the attempt's - /// artifact directory (upstream's attachArtifact). + /// The files fixtures kept for the attempt, such as a download, as the id the step names, the kind, the path under the attempt's + /// artifact directory (upstream's attachArtifact), and the redaction the runner claims for it when it rewrote it (null when + /// the kind decides). /// - public IReadOnlyList<(string Id, string Kind, string Path)> Attached + public IReadOnlyList<(string Id, string Kind, string Path, ArtifactRedaction? Redaction)> Attached { get { lock (_attached) { + // Reports must use the final ledger, including values a later fill registered. + if (ArtifactsDir is { } directory && Secrecy.RedactsDownloads(Redactor)) + { + var redactor = Redactor; + for (var index = 0; index < _attached.Count; index++) + { + var entry = _attached[index]; + if (entry.Kind == "download" && ArtifactPaths.IsTextLike(ArtifactPaths.MediaTypeFor(entry.Path))) + { + _attached[index] = (entry.Id, entry.Kind, entry.Path, ArtifactPaths.RedactDownload(Path.Combine(directory, entry.Path), redactor)); + } + } + } + return [.. _attached]; } } @@ -1318,11 +1337,24 @@ public Redactor Redactor /// Keeps as an artifact of the running step, or of the last one when none runs. public void AttachArtifact(string kind, string relative) { + // A text-like download is rewritten through the session's ledger before it is measured, so the record claims what that did + // (upstream's `sink.register`, `run/artifacts.ts`). Any other download is bytes the app served: incomplete. + ArtifactRedaction? redaction = null; + if (kind == "download" && ArtifactsDir is { } directory) + { + redaction = ArtifactRedaction.Incomplete; + var redactor = Redactor; + if (Secrecy.RedactsDownloads(redactor) && ArtifactPaths.IsTextLike(ArtifactPaths.MediaTypeFor(relative))) + { + redaction = ArtifactPaths.RedactDownload(Path.Combine(directory, relative), redactor); + } + } + string id; lock (_attached) { id = kind + "-" + (_attached.Count(entry => entry.Kind == kind)).ToString(System.Globalization.CultureInfo.InvariantCulture); - _attached.Add((id, kind, relative)); + _attached.Add((id, kind, relative, redaction)); } Steps.AttachArtifact(id); diff --git a/src/E2E/Internal/Redactor.cs b/src/E2E/Internal/Redactor.cs index 31b7e2de..63a61a9a 100644 --- a/src/E2E/Internal/Redactor.cs +++ b/src/E2E/Internal/Redactor.cs @@ -36,6 +36,7 @@ internal sealed class Redactor private static readonly Regex EncodedRun = new(@"[A-Za-z0-9+/_-]{8,}={0,2}", RegexOptions.CultureInvariant, MatchTimeout); private readonly Regex? _pattern; + private readonly Lazy? _fragmentPattern; private readonly Regex? _known; private readonly string[] _markers = []; private readonly Dictionary _fragments = new(StringComparer.Ordinal); @@ -64,6 +65,9 @@ private Redactor(IEnumerable<(string Marker, string Value)> values) _maxMarkerLength = _markers.Max(marker => marker.Length); _spansLines = entries.Any(entry => entry.Value.Contains('\n') || Regex.IsMatch(entry.Value.Trim(), @"\s", RegexOptions.None, MatchTimeout)); _pattern = new Regex(source, RegexOptions.IgnoreCase | RegexOptions.CultureInvariant, MatchTimeout); + _fragmentPattern = new(() => new Regex( + string.Join('|', entries.Select(entry => "(" + ValuePattern(entry.Value, unicodeEscapes: true).Source + ")")), + RegexOptions.IgnoreCase | RegexOptions.CultureInvariant, MatchTimeout)); _leadingForms = new string[entries.Count][]; for (var index = 0; index < entries.Count; index++) { @@ -79,6 +83,9 @@ private Redactor(IEnumerable<(string Marker, string Value)> values) } } + /// Whether no value is registered (upstream's SecretLedger.isEmpty): every text passes through as it is. + public bool IsEmpty => _pattern is null; + /// A redactor with no values, which leaves every text as it is. public static Redactor None { get; } = new([]); @@ -258,7 +265,8 @@ private static int UnfinishedMarkerBefore(string text, int cut, int maxMarkerLen /// that also rewrites every run of at least /// consecutive characters of a value, in any /// case and with its whitespace runs collapsed, and every base64 run that - /// decodes to text holding a value or a fragment. For raw engine text, such + /// decodes to text holding a value or a fragment. Whole values also match + /// JSON Unicode escapes of ASCII letters and digits. For raw engine text, such /// as an error that quotes a value cut short. /// public string RedactFragments(string text) @@ -268,11 +276,15 @@ public string RedactFragments(string text) private string RedactPlainFragments(string text) { - var redacted = Redact(text); + // Fragment scans also cover ASCII letters and digits explicitly escaped by a JSON producer. + // Keep the stream redactor's ordinary pattern and buffering contract unchanged. + var redacted = text.Contains("\\u", StringComparison.OrdinalIgnoreCase) + ? MapPieces(text, piece => Rewrite(piece, _fragmentPattern!.Value)) + : Redact(text); return _fragments.Count == 0 ? redacted : MapPieces(redacted, RewriteFragments); } - private static Pattern ValuePattern(string value) + private static Pattern ValuePattern(string value, bool unicodeEscapes = false) { var trimmable = new StringInfo(value.Trim()).LengthInTextElements >= Secret.MinLength; var pieces = Regex.Split(value, @"(\s+)", RegexOptions.None, MatchTimeout); @@ -284,7 +296,7 @@ private static Pattern ValuePattern(string value) return WhitespaceRun(piece); } - var written = Concat(piece.EnumerateRunes().Select(rune => Character(rune.ToString()))); + var written = Concat(piece.EnumerateRunes().Select(rune => Character(rune.ToString(), unicodeEscapes))); return run && trimmable ? new Pattern("(?:" + written.Source + ")?", written.MaxLength) : written; })); } @@ -299,13 +311,13 @@ private static Pattern WhitespaceRun(string run) return new Pattern(one.Source + "{1," + run.Length.ToString(CultureInfo.InvariantCulture) + "}", one.MaxLength * run.Length); } - private static Pattern Character(string ch) + private static Pattern Character(string ch, bool unicodeEscapes) { var forms = CaseForms(ch); - var single = forms.Where(form => form.EnumerateRunes().Count() == 1).SelectMany(Spellings); + var single = forms.Where(form => form.EnumerateRunes().Count() == 1).SelectMany(form => Spellings(form, unicodeEscapes)); var expanded = forms .Where(form => form.EnumerateRunes().Count() > 1) - .Select(form => Concat(form.EnumerateRunes().Select(part => Alternation(Distinct(Spellings(part.ToString())).Select(Exact))))); + .Select(form => Concat(form.EnumerateRunes().Select(part => Alternation(Distinct(Spellings(part.ToString(), unicodeEscapes)).Select(Exact))))); return Alternation(Distinct(single).Select(Exact).Concat(expanded)); } @@ -371,11 +383,13 @@ private static IEnumerable Distinct(IEnumerable spellings) /// escapes it (once and inside another JSON string), as an HTML character /// reference, and percent-encoded. ASCII letters and digits have one. /// - private static List Spellings(string ch) + private static List Spellings(string ch) => Spellings(ch, unicodeEscapes: false); + + private static List Spellings(string ch, bool unicodeEscapes) { if (ch.Length == 1 && char.IsAsciiLetterOrDigit(ch[0])) { - return [ch]; + return unicodeEscapes ? [ch, UnicodeEscape(ch), JsonEscape(UnicodeEscape(ch))] : [ch]; } var inJson = new List { JsonEscape(ch), UnicodeEscape(ch) }; @@ -519,7 +533,12 @@ private string MapPieces(string text, Func rewrite) private string Rewrite(string text) { - return _pattern!.Replace(text, match => + return Rewrite(text, _pattern!); + } + + private string Rewrite(string text, Regex pattern) + { + return pattern.Replace(text, match => { for (var group = 1; group < match.Groups.Count && group <= _markers.Length; group++) { diff --git a/src/E2E/Report/ReportRecords.cs b/src/E2E/Report/ReportRecords.cs index da2027c6..b042ca39 100644 --- a/src/E2E/Report/ReportRecords.cs +++ b/src/E2E/Report/ReportRecords.cs @@ -3,6 +3,7 @@ // SPDX-License-Identifier: Apache-2.0 using E2E.Internal; +using ArtifactRedaction = E2E.Reports.ArtifactRedaction; using VcsInfo = E2E.Reports.VcsInfo; namespace E2E; @@ -105,7 +106,8 @@ internal sealed record ReportError(string Code, string Message, string? Phase = /// Where a hosted service keeps it. /// When a recording began; steps read their offset into it. /// The size of the file in bytes, when known; the run's usage.artifactBytes adds them up. -internal sealed record ReportArtifact(string Id, string Kind, string? Path = null, string? Url = null, DateTimeOffset? StartedAt = null, long? Size = null); +/// What the runner claims for the file when it rewrote it (a text-like download); null leaves the kind to decide. +internal sealed record ReportArtifact(string Id, string Kind, string? Path = null, string? Url = null, DateTimeOffset? StartedAt = null, long? Size = null, ArtifactRedaction? Redaction = null); /// What was seen when a failure landed (upstream's FailureEvidence). internal sealed partial record FailureEvidence diff --git a/src/E2E/Report/ReportWire.cs b/src/E2E/Report/ReportWire.cs index e21e46d5..51cadd51 100644 --- a/src/E2E/Report/ReportWire.cs +++ b/src/E2E/Report/ReportWire.cs @@ -8,6 +8,7 @@ using System.Text.Json.Nodes; using System.Text.RegularExpressions; using E2E.Internal; +using ArtifactRedaction = E2E.Reports.ArtifactRedaction; namespace E2E; @@ -333,7 +334,7 @@ private static JsonObject Failure(FailureEvidence evidence) /// /// An artifact as the report keeps it. The size and the digest are read from the file under , which is /// where starts; a file that cannot be read leaves the artifact incomplete without them. Pixels - /// cannot be proven free of a secret, so only a log reads complete. + /// cannot be proven free of a secret, so only a log reads complete, and a download the runner rewrote through the ledger (it carries the claim). /// public static JsonObject Artifact(ReportArtifact artifact, string resultsRoot, string? stepId) { @@ -356,7 +357,7 @@ public static JsonObject Artifact(ReportArtifact artifact, string resultsRoot, s } var kind = artifact.Kind is "screenshot" or "video" or "download" or "log" ? artifact.Kind : "other"; - var complete = kind == "log" && path is not null; + var complete = path is not null && (artifact.Redaction is { } claimed ? claimed == ArtifactRedaction.Complete : kind == "log"); return Compose( ("id", artifact.Id), ("kind", kind), @@ -380,18 +381,7 @@ private static string MediaType(string kind, string? name) "log" => "text/plain", // A download keeps the name the app suggested, so its extension says what it is (upstream's artifact store, mediaTypeFor). - "download" => extension switch - { - ".png" => "image/png", - ".zip" => "application/zip", - ".webm" => "video/webm", - ".mp4" => "video/mp4", - ".txt" => "text/plain", - ".csv" => "text/csv", - ".json" => "application/json", - ".html" or ".htm" => "text/html", - _ => "application/octet-stream", - }, + "download" => ArtifactPaths.MediaTypeFor(extension), _ => "application/octet-stream", }; } diff --git a/src/E2E/Run/ArtifactPaths.cs b/src/E2E/Run/ArtifactPaths.cs index 6bcfdb72..11fc85e5 100644 --- a/src/E2E/Run/ArtifactPaths.cs +++ b/src/E2E/Run/ArtifactPaths.cs @@ -5,6 +5,8 @@ using System.Globalization; using System.Text; using System.Text.RegularExpressions; +using E2E.Internal; +using E2E.Reports; namespace E2E; @@ -86,4 +88,86 @@ public static string ResultSegment(string id, string file, IReadOnlyList return string.Join('-', new[] { stem, title, id.Length > ResultDigestChars ? id[..ResultDigestChars] : id }.Where(part => part.Length > 0)); } + + /// The media type of a file by its extension (upstream's mediaTypeFor). + public static string MediaTypeFor(string relativePath) + { + if (relativePath.EndsWith(".png", StringComparison.OrdinalIgnoreCase)) + { + return "image/png"; + } + + if (relativePath.EndsWith(".zip", StringComparison.OrdinalIgnoreCase)) + { + return "application/zip"; + } + + if (relativePath.EndsWith(".webm", StringComparison.OrdinalIgnoreCase)) + { + return "video/webm"; + } + + if (relativePath.EndsWith(".mp4", StringComparison.OrdinalIgnoreCase)) + { + return "video/mp4"; + } + + if (relativePath.EndsWith(".txt", StringComparison.OrdinalIgnoreCase)) + { + return "text/plain"; + } + + if (relativePath.EndsWith(".csv", StringComparison.OrdinalIgnoreCase)) + { + return "text/csv"; + } + + if (relativePath.EndsWith(".json", StringComparison.OrdinalIgnoreCase)) + { + return "application/json"; + } + + if (relativePath.EndsWith(".html", StringComparison.OrdinalIgnoreCase) || relativePath.EndsWith(".htm", StringComparison.OrdinalIgnoreCase)) + { + return "text/html"; + } + + return "application/octet-stream"; + } + + /// Whether the bytes of a media type are text the secret redactor can rewrite (upstream's isTextLike). + public static bool IsTextLike(string mediaType) => mediaType.StartsWith("text/", StringComparison.Ordinal) || mediaType == "application/json"; + + /// + /// Rewrites a text-like download through the session's ledger, and returns the redaction the record can claim: Complete once + /// every registered value is gone from it, changed or not, and Incomplete when the file contains NUL bytes, is not UTF-8 text, + /// or cannot be read or written, in which case it is left as served. A leading byte order mark is kept as part of the text, so a + /// rewritten file starts the way it was served. Upstream's redactDownload. + /// + public static ArtifactRedaction RedactDownload(string absolute, Redactor ledger) + { + try + { + var bytes = File.ReadAllBytes(absolute); + // BOM-less UTF-16 can pass UTF-8 decoding while NULs keep its secrets from matching. + if (bytes.AsSpan().Contains((byte)0)) + { + return ArtifactRedaction.Incomplete; + } + + // GetString keeps a leading byte order mark as U+FEFF, as upstream's ignoreBOM does. + var text = new UTF8Encoding(false, true).GetString(bytes); + var redacted = ledger.RedactFragments(text); + if (!string.Equals(redacted, text, StringComparison.Ordinal)) + { + AtomicWrite.WriteFileAtomic(absolute, redacted); + } + + return ArtifactRedaction.Complete; + } + catch (Exception ex) when (ex is IOException or UnauthorizedAccessException or DecoderFallbackException or RegexMatchTimeoutException) + { + return ArtifactRedaction.Incomplete; + } + } } diff --git a/src/E2E/Run/AttemptReport.cs b/src/E2E/Run/AttemptReport.cs index 827abe5e..3ff33adf 100644 --- a/src/E2E/Run/AttemptReport.cs +++ b/src/E2E/Run/AttemptReport.cs @@ -104,7 +104,7 @@ public static (Report1Document Report, string ResultId) Build( ReportArtifact[] kept = [ .. videos.Select((video, index) => Artifact(video, index, directory, attemptSegment)), - .. scope.Attached.Select(attached => new ReportArtifact(attached.Id, attached.Kind, directory + "/" + attemptSegment + "/" + attached.Path)), + .. scope.Attached.Select(attached => new ReportArtifact(attached.Id, attached.Kind, directory + "/" + attemptSegment + "/" + attached.Path, Redaction: attached.Redaction)), .. failed && evidence?.Screen is { } screen ? new[] { new ReportArtifact(screen, "log", directory + "/" + attemptSegment + "/" + FailureEvidence.ScreenFile) } : [], .. failed && evidence?.Screenshot is { } shot ? new[] { new ReportArtifact(shot, "screenshot", directory + "/" + attemptSegment + "/" + FailureEvidence.ScreenshotFile) } : [], ]; diff --git a/src/E2E/Run/Browser.cs b/src/E2E/Run/Browser.cs index ba421a28..2f942c80 100644 --- a/src/E2E/Run/Browser.cs +++ b/src/E2E/Run/Browser.cs @@ -352,7 +352,9 @@ public Task> OnDialogAsync(Func handler, CancellationTo /// /// Runs and waits for the download it starts, within (the action timeout by - /// default). The file is kept under the attempt's downloads artifact folder, as an artifact of this step. A trigger that + /// default). The file is kept under the attempt's downloads artifact folder, as an artifact of this step. A text download + /// (text/* or application/json) is rewritten in place through the session's secrets when it holds any, and the report + /// labels it complete; any other download stays as the app served it, incomplete. A trigger that /// throws fails the call with its own error, and one that starts no download fails with ACTION_FAILED, naming the wait and /// how long the trigger took. /// diff --git a/src/E2E/Run/E2ESession.cs b/src/E2E/Run/E2ESession.cs index 46fb56c5..dc5be8a5 100644 --- a/src/E2E/Run/E2ESession.cs +++ b/src/E2E/Run/E2ESession.cs @@ -222,6 +222,7 @@ internal static async Task LaunchAsync(E2ESessionOptions options, IR var testFailed = options.TestFailed ?? (static () => false); var scope = new AttemptScope { + ArtifactsDir = attemptDir, Session = engine, Screens = screens, Environment = environment, diff --git a/src/E2E/Run/Secrecy.cs b/src/E2E/Run/Secrecy.cs index 4d0374ff..a105e031 100644 --- a/src/E2E/Run/Secrecy.cs +++ b/src/E2E/Run/Secrecy.cs @@ -2,11 +2,21 @@ // Modified by Dario Kondratiuk. // SPDX-License-Identifier: Apache-2.0 +using E2E.Internal; + namespace E2E; /// What one live session knows about secrets: the values to redact. internal static class Secrecy { + /// + /// Whether a text-like download of the session is rewritten through its ledger before it is kept: whenever the ledger holds a + /// value, filled or not. A value reaches the app in ways the runner never sees (a URL or a fill the test spelled it into, the + /// app's own config), so downloads are redacted like every report and observation, not only after a fill. Upstream's + /// redactsDownloads; the port has no SessionSecrecy, so it takes the ledger alone. + /// + public static bool RedactsDownloads(Redactor ledger) => !ledger.IsEmpty; + /// /// The plaintext of one secret an engine option carries. The value joins before it is /// returned, so it is redacted from the moment it exists, whoever it is handed to. diff --git a/tests/E2E.Tests/ArtifactStore/DownloadRedactionTests.cs b/tests/E2E.Tests/ArtifactStore/DownloadRedactionTests.cs new file mode 100644 index 00000000..7073f0e7 --- /dev/null +++ b/tests/E2E.Tests/ArtifactStore/DownloadRedactionTests.cs @@ -0,0 +1,222 @@ +// Copyright 2026 TesterArmy. +// Modified by Dario Kondratiuk. +// SPDX-License-Identifier: Apache-2.0 + +using System.Security.Cryptography; +using System.Text; +using System.Text.Json.Nodes; + +namespace E2E.Tests.ArtifactStore; + +/// +/// describe('download redaction') of upstream artifact-store.test.ts. The port has no createAttemptArtifacts or +/// ArtifactStore, so each test registers the download through and reads the record +/// writes; the assertions on what the store was handed check the file on disk and the record instead. +/// +public sealed class DownloadRedactionTests : IDisposable +{ + private const string Secret = "download-secret-Qx7-2718"; + private readonly string _root = Directory.CreateTempSubdirectory("e2e-artifact-store-").FullName; + + public void Dispose() => Directory.Delete(_root, recursive: true); + + private AttemptScope Downloads(params (string Name, string Value)[] filled) + { + Directory.CreateDirectory(Path.Combine(_root, "downloads")); + var scope = new AttemptScope + { + Session = null!, + Agents = new Dictionary(), + Cache = null, + CacheEnabled = false, + TestTitle = "downloads", + EnginePlatform = "web", + ArtifactsDir = _root, + }; + foreach (var (name, value) in filled) + { + scope.Register(name, value); + } + + return scope; + } + + private string Write(string name, byte[] bytes) + { + File.WriteAllBytes(Path.Combine(_root, "downloads", name), bytes); + return "downloads/" + name; + } + + private string Write(string name, string text) => Write(name, Encoding.UTF8.GetBytes(text)); + + private string Read(string name) => File.ReadAllText(Path.Combine(_root, "downloads", name), new UTF8Encoding(false)); + + private JsonObject Record(AttemptScope scope, int index) + { + var attached = scope.Attached[index]; + return ReportWire.Artifact(new ReportArtifact(attached.Id, attached.Kind, attached.Path, Redaction: attached.Redaction), _root, null); + } + + [Fact] + public void Labels_a_download_incomplete_by_default_bytes_the_app_served_not_rewritten_on_the_record_alone() + { + var scope = Downloads(); + var body = $"id,key\n1,{Secret}\n"; + scope.AttachArtifact("download", Write("export.csv", body)); + var record = Record(scope, 0); + Assert.Equal("download", (string?)record["kind"]); + Assert.Equal("text/csv", (string?)record["mediaType"]); + Assert.Equal("incomplete", (string?)record["redaction"]); + Assert.Equal(body, Read("export.csv")); + } + + [Fact] + public void Rewrites_a_text_download_with_nothing_filled_a_value_can_reach_the_app_without_a_fill() + { + var scope = Downloads(("api-key", Secret)); + scope.AttachArtifact("download", Write("headers.txt", $"authorization: {Secret}\n")); + Assert.Equal("complete", (string?)Record(scope, 0)["redaction"]); + Assert.Equal("authorization: \n", Read("headers.txt")); + } + + [Fact] + public void Rewrites_a_text_download_through_the_ledger_once_a_secret_was_filled_and_labels_it_complete_on_the_record_alone() + { + var scope = Downloads(("api-key", Secret)); + scope.AttachArtifact("download", Write("export.json", $"{{\"key\":\"{Secret}\"}}")); + var redacted = "{\"key\":\"\"}"; + var record = Record(scope, 0); + Assert.Equal("application/json", (string?)record["mediaType"]); + Assert.Equal("complete", (string?)record["redaction"]); + Assert.Equal(redacted, Read("export.json")); + Assert.Equal(Encoding.UTF8.GetByteCount(redacted), (long?)record["size"]); + Assert.Equal(Convert.ToHexStringLower(SHA256.HashData(Encoding.UTF8.GetBytes(redacted))), (string?)record["sha256"]); + } + + [Fact] + public void Rewrites_fragments_and_encoded_forms_in_a_download_the_way_trace_text_entries_are() + { + var scope = Downloads(("api-key", Secret)); + var cut = Secret.Substring(6, 14); + var encoded = Convert.ToBase64String(Encoding.UTF8.GetBytes(Secret)); + scope.AttachArtifact("download", Write("export.txt", $"cut={cut}\nencoded={encoded}\n")); + Assert.Equal("complete", (string?)Record(scope, 0)["redaction"]); + Assert.Equal("cut=\nencoded=\n", Read("export.txt")); + } + + [Fact] + public void Rewrites_a_value_CSV_quoted_its_double_quotes_doubled() + { + var scope = Downloads(("api-key", "pa\"ss,word")); + scope.AttachArtifact("download", Write("export.csv", "id,key\n1,\"pa\"\"ss,word\"\n")); + Assert.Equal("complete", (string?)Record(scope, 0)["redaction"]); + Assert.Equal("id,key\n1,\"\"\n", Read("export.csv")); + } + + [Fact] + public void Keeps_a_byte_order_mark_on_a_rewritten_download_on_the_record_alone() + { + var scope = Downloads(("api-key", Secret)); + byte[] bom = [0xef, 0xbb, 0xbf]; + scope.AttachArtifact("download", Write("export.csv", [.. bom, .. Encoding.UTF8.GetBytes($"key\n{Secret}\n")])); + var rewritten = File.ReadAllBytes(Path.Combine(_root, "downloads", "export.csv")); + var record = Record(scope, 0); + Assert.True(rewritten.AsSpan(0, 3).SequenceEqual(bom)); + Assert.Equal("key\n\n", Encoding.UTF8.GetString(rewritten, 3, rewritten.Length - 3)); + Assert.Equal("complete", (string?)record["redaction"]); + Assert.Equal(rewritten.Length, (long?)record["size"]); + Assert.Equal(Convert.ToHexStringLower(SHA256.HashData(rewritten)), (string?)record["sha256"]); + } + + [Fact] + public void Labels_a_scanned_text_download_complete_when_it_held_nothing_to_redact() + { + var scope = Downloads(("api-key", Secret)); + scope.AttachArtifact("download", Write("notes.txt", "nothing secret here")); + var record = Record(scope, 0); + Assert.Equal("complete", (string?)record["redaction"]); + Assert.Equal(19L, (long?)record["size"]); + } + + // .NET-only: the port snapshots the redactor at attachment and builds artifact records when the attempt ends. + [Theory] + [InlineData(false)] + [InlineData(true)] + public void Rescans_downloads_with_values_registered_after_attachment(bool alreadyHasSecret) + { + var scope = alreadyHasSecret ? Downloads(("api-key", Secret)) : Downloads(); + const string later = "later-credential-Jk8-5931"; + scope.AttachArtifact("download", Write("export.txt", $"key={later}\n")); + Assert.Equal(alreadyHasSecret ? "complete" : "incomplete", (string?)Record(scope, 0)["redaction"]); + Assert.Equal($"key={later}\n", Read("export.txt")); + + if (alreadyHasSecret) + { + // FillAsync remembers a Secret through this path; the earlier registered value remains known too. + scope.Remember(new Dictionary { ["api-key"] = global::E2E.Secret.Create("api-key", later) }); + } + else + { + scope.Register("api-key", later); + } + var record = Record(scope, 0); + const string redacted = "key=\n"; + Assert.Equal("complete", (string?)record["redaction"]); + Assert.Equal(redacted, Read("export.txt")); + Assert.Equal(Encoding.UTF8.GetByteCount(redacted), (long?)record["size"]); + Assert.Equal(Convert.ToHexStringLower(SHA256.HashData(Encoding.UTF8.GetBytes(redacted))), (string?)record["sha256"]); + } + + // .NET-only: a failed final scan must replace the earlier attachment's successful result. + [Fact] + public void Reports_a_previously_complete_download_incomplete_when_the_final_scan_fails() + { + var scope = Downloads(("api-key", Secret)); + scope.AttachArtifact("download", Write("export.txt", "nothing secret here")); + Assert.Equal("complete", (string?)Record(scope, 0)["redaction"]); + + Write("export.txt", [0xff, 0xfe]); + scope.Register("api-key", "later-credential-Jk8-5931"); + Assert.Equal("incomplete", (string?)Record(scope, 0)["redaction"]); + } + + // .NET-only regression: upstream's ASCII spelling shortcut also misses these valid JSON encodings. + [Theory] + [InlineData("""{"key":"\u0068\u0075\u006e\u0074\u0065\u0072\u0032"}""", """{"key":""}""")] + [InlineData("""{"key":"\u0068\u0075\u006E\u0074\u0065\u0072\u0032"}""", """{"key":""}""")] + [InlineData("""{"key":"hunter\u0032"}""", """{"key":""}""")] + [InlineData("""{"text":"{\"key\":\"\\u0068\\u0075\\u006e\\u0074\\u0065\\u0072\\u0032\"}"}""", """{"text":"{\"key\":\"\"}"}""")] + public void Redacts_JSON_Unicode_escaped_ASCII_secrets_before_reporting_complete(string body, string redacted) + { + var scope = Downloads(("api-key", "hunter2")); + scope.AttachArtifact("download", Write("export.json", body)); + var record = Record(scope, 0); + Assert.Equal("complete", (string?)record["redaction"]); + Assert.Equal(redacted, Read("export.json")); + Assert.Equal(Encoding.UTF8.GetByteCount(redacted), (long?)record["size"]); + Assert.Equal(Convert.ToHexStringLower(SHA256.HashData(Encoding.UTF8.GetBytes(redacted))), (string?)record["sha256"]); + } + + [Fact] + public void Leaves_a_download_as_served_with_no_secret_value_and_a_binary_one_with_a_fill_both_incomplete() + { + var untainted = Downloads(); + var body = $"key={Secret}"; + untainted.AttachArtifact("download", Write("env.txt", body)); + Assert.Equal("incomplete", (string?)Record(untainted, 0)["redaction"]); + Assert.Equal(body, Read("env.txt")); + + var tainted = Downloads(("api-key", Secret)); + byte[] binary = [0xff, 0xfe, 0x00, .. Encoding.UTF8.GetBytes(Secret)]; + tainted.AttachArtifact("download", Write("blob.bin", binary)); + tainted.AttachArtifact("download", Write("broken.txt", binary)); + var utf16 = Encoding.Unicode.GetBytes($"key={Secret}"); + tainted.AttachArtifact("download", Write("export.csv", utf16)); + Assert.Equal("incomplete", (string?)Record(tainted, 0)["redaction"]); + Assert.Equal("incomplete", (string?)Record(tainted, 1)["redaction"]); + Assert.Equal("incomplete", (string?)Record(tainted, 2)["redaction"]); + Assert.Equal(binary, File.ReadAllBytes(Path.Combine(_root, "downloads", "blob.bin"))); + Assert.Equal(binary, File.ReadAllBytes(Path.Combine(_root, "downloads", "broken.txt"))); + Assert.Equal(utf16, File.ReadAllBytes(Path.Combine(_root, "downloads", "export.csv"))); + } +}