SSRF-Schutz, Pfadpruefung und Retry-Logik
S5 — Die Domain-Whitelist wurde nur auf die Ausgangs-URL angewendet, HttpClient folgte Weiterleitungen aber selbst. Eine erlaubte Domain konnte damit auf beliebige interne Adressen weiterleiten: Router, NAS, der Git-Server im LAN, Cloud-Metadatendienste. Der Agent haette deren Inhalt zurueckgeliefert. UrlGuard prueft Schema, private und lokale Netzbereiche sowie die Whitelist. AllowAutoRedirect ist abgeschaltet; Weiterleitungen werden einzeln aufgeloest und JEDER Zwischenschritt erneut geprueft, begrenzt auf fuenf Spruenge. Nebenbei behoben: Die alte www-Behandlung ersetzte die Zeichenfolge ueber den ganzen Hostnamen, aus mywww.example.com wurde myexample.com. Und die Subdomain-Pruefung achtet jetzt auf den Punkt, sodass example.com.attacker.net nicht mehr als Treffer fuer example.com durchgeht. S6 — Die Pfadpruefung in FileRW verglich nur Zeichenketten-Praefixe. Ohne abschliessenden Verzeichnistrenner erlaubte ein Root wie Agent-X\Workspace damit auch Zugriffe auf Agent-X\Workspace-Backup. WorkspacePath vergleicht jetzt auf Verzeichnisgrenzen und lehnt zusaetzlich absolute Pfade, UNC-Freigaben und Alternate Data Streams ab. Das Dateisystem wird in den Tests bewusst nicht abstrahiert — sie sollen die echte Windows-Pfadsemantik pruefen. Eine Abstraktion wuerde genau die Fehlerklasse verstecken, um die es geht. B12 — Der Client warf bei jedem Nicht-2xx sofort; ein einzelnes HTTP 429 beendete damit einen kompletten geplanten Lauf, obwohl Rate-Limits und kurze 5xx bei OpenRouter Normalbetrieb sind. RetryPolicy wiederholt 408/425/429/5xx mit exponentiellem Backoff und Streuung, respektiert ein Retry-After des Servers und deckelt die Wartezeit. Dauerhafte Fehler wie 401 werden nicht wiederholt. Die Wartefunktion ist injizierbar, damit die Tests nicht wirklich warten. 264 Tests gruen (116 Core, 148 Tools). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.8
parent
e5067cae70
commit
cbb8ac22bc
@@ -167,6 +167,9 @@ public sealed class FileRWTool : IAgentTool
|
||||
var relativePath = Path.GetRelativePath(rootPath, fullPath)
|
||||
.Replace('\\', '/').TrimStart('/');
|
||||
|
||||
// GetProtectedPaths hängt an jeden Eintrag ein "/" an, deshalb greift
|
||||
// StartsWith hier auf einer echten Verzeichnisgrenze: "stocks/" trifft
|
||||
// "stocks/x.json", aber nicht "stocks-alt/x.json".
|
||||
return protectedPaths.Any(pp =>
|
||||
relativePath.StartsWith(pp, StringComparison.OrdinalIgnoreCase) ||
|
||||
relativePath.Equals(pp.TrimEnd('/'), StringComparison.OrdinalIgnoreCase));
|
||||
@@ -200,18 +203,9 @@ public sealed class FileRWTool : IAgentTool
|
||||
private string GetAndValidatePath(JsonElement input, string rootPath, string workspace, bool checkExtension, AgentToolContext context)
|
||||
{
|
||||
var relativePath = input.TryGetProperty("path", out var p) ? p.GetString() : "";
|
||||
if (string.IsNullOrWhiteSpace(relativePath))
|
||||
{
|
||||
relativePath = ".";
|
||||
}
|
||||
|
||||
var fullPath = Path.GetFullPath(Path.Combine(rootPath, relativePath));
|
||||
|
||||
// Path Traversal Check
|
||||
if (!fullPath.StartsWith(rootPath, StringComparison.OrdinalIgnoreCase))
|
||||
{
|
||||
throw new UnauthorizedAccessException($"Zugriff verweigert: Der Pfad liegt außerhalb des {workspace} Workspaces.");
|
||||
}
|
||||
// Prüft Ausbruch aus dem Workspace, absolute Pfade und Alternate Data Streams.
|
||||
var fullPath = WorkspacePath.Resolve(rootPath, relativePath, workspace);
|
||||
|
||||
// Extension Check
|
||||
if (checkExtension && !Directory.Exists(fullPath))
|
||||
|
||||
@@ -0,0 +1,83 @@
|
||||
namespace ClawdDotNet.Tools.FileRW;
|
||||
|
||||
/// <summary>
|
||||
/// Löst relative Pfade innerhalb eines Workspace auf und stellt sicher, dass sie
|
||||
/// diesen nicht verlassen.
|
||||
///
|
||||
/// Hintergrund (S6): Die Prüfung verglich vorher nur die Zeichenkette:
|
||||
///
|
||||
/// fullPath.StartsWith(rootPath, OrdinalIgnoreCase)
|
||||
///
|
||||
/// Ohne abschließenden Verzeichnistrenner erlaubte ein Root wie
|
||||
/// "…\Agent-X\Workspace" damit auch Zugriffe auf "…\Agent-X\Workspace-Backup\…" —
|
||||
/// ein fremdes Verzeichnis, das zufällig mit demselben Präfix beginnt.
|
||||
/// </summary>
|
||||
public static class WorkspacePath
|
||||
{
|
||||
/// <summary>
|
||||
/// Setzt <paramref name="relativePath"/> auf <paramref name="root"/> auf und prüft,
|
||||
/// dass das Ergebnis innerhalb des Workspace liegt.
|
||||
/// </summary>
|
||||
/// <exception cref="UnauthorizedAccessException">Wenn der Pfad ausbricht.</exception>
|
||||
public static string Resolve(string root, string? relativePath, string workspaceName)
|
||||
{
|
||||
var value = string.IsNullOrWhiteSpace(relativePath) ? "." : relativePath.Trim();
|
||||
|
||||
// Absolute Pfade und UNC-Freigaben würden Path.Combine den Root verwerfen lassen.
|
||||
if (Path.IsPathRooted(value) || value.StartsWith(@"\\") || value.StartsWith("//"))
|
||||
{
|
||||
throw new UnauthorizedAccessException(
|
||||
$"Zugriff verweigert: Absolute Pfade sind im {workspaceName} Workspace nicht erlaubt.");
|
||||
}
|
||||
|
||||
// Alternate Data Streams (datei.txt:versteckt) umgehen die Endungsprüfung.
|
||||
if (value.Contains(':'))
|
||||
{
|
||||
throw new UnauthorizedAccessException(
|
||||
$"Zugriff verweigert: Der Pfad enthält ein unzulässiges Zeichen (':').");
|
||||
}
|
||||
|
||||
var normalizedRoot = NormalizeDirectory(root);
|
||||
var fullPath = Path.GetFullPath(Path.Combine(normalizedRoot, value));
|
||||
|
||||
if (!IsInside(fullPath, normalizedRoot))
|
||||
{
|
||||
throw new UnauthorizedAccessException(
|
||||
$"Zugriff verweigert: Der Pfad liegt außerhalb des {workspaceName} Workspaces.");
|
||||
}
|
||||
|
||||
return fullPath;
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Prüft, ob <paramref name="candidate"/> im Verzeichnis <paramref name="root"/> liegt.
|
||||
/// Der Vergleich erfolgt auf Verzeichnisgrenzen, nicht auf Zeichenketten-Präfixen.
|
||||
/// </summary>
|
||||
public static bool IsInside(string candidate, string root)
|
||||
{
|
||||
var normalizedRoot = NormalizeDirectory(root);
|
||||
var normalizedCandidate = Path.GetFullPath(candidate);
|
||||
|
||||
// Der Root selbst gilt als innerhalb.
|
||||
if (string.Equals(
|
||||
normalizedCandidate.TrimEnd(Path.DirectorySeparatorChar),
|
||||
normalizedRoot.TrimEnd(Path.DirectorySeparatorChar),
|
||||
StringComparison.OrdinalIgnoreCase))
|
||||
{
|
||||
return true;
|
||||
}
|
||||
|
||||
// Der Trenner ist entscheidend: Ohne ihn gälte "…\Workspace-Backup" als
|
||||
// Teil von "…\Workspace".
|
||||
return normalizedCandidate.StartsWith(normalizedRoot, StringComparison.OrdinalIgnoreCase);
|
||||
}
|
||||
|
||||
/// <summary>Absoluter Pfad mit genau einem abschließenden Trenner.</summary>
|
||||
private static string NormalizeDirectory(string path)
|
||||
{
|
||||
var full = Path.GetFullPath(path);
|
||||
return full.EndsWith(Path.DirectorySeparatorChar)
|
||||
? full
|
||||
: full + Path.DirectorySeparatorChar;
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user