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
3 changes: 3 additions & 0 deletions .github/workflows/release.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@ on:
push:
tags: ["v*"]

permissions:
contents: read

jobs:
build:
runs-on: ubuntu-latest
Expand Down
3 changes: 3 additions & 0 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@ on:
pull_request:
branches: [main]

permissions:
contents: read

jobs:
test:
runs-on: ubuntu-latest
Expand Down
9 changes: 8 additions & 1 deletion cmd/odek/browser_tool.go
Original file line number Diff line number Diff line change
Expand Up @@ -408,7 +408,14 @@ func parseHTML(ctx context.Context, html, pageURL string, status int) browserSna
}
href := strings.TrimSpace(m[1])
text := strings.TrimSpace(m[2])
if href == "" || text == "" || href == "#" || strings.HasPrefix(href, "javascript:") {
// Skip non-navigable schemes. The check is case-insensitive and
// covers javascript:, data:, and vbscript: — all of which can carry
// script/active content the agent must never be sent into.
lowerHref := strings.ToLower(href)
if href == "" || text == "" || href == "#" ||
strings.HasPrefix(lowerHref, "javascript:") ||
strings.HasPrefix(lowerHref, "data:") ||
strings.HasPrefix(lowerHref, "vbscript:") {
continue
}
// Skip duplicates
Expand Down
34 changes: 34 additions & 0 deletions cmd/odek/browser_tool_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -596,3 +596,37 @@ func TestBrowser_WrapsLinkURL(t *testing.T) {
t.Errorf("link URL should be wrapped as untrusted, got %q", r.Elements[0].URL)
}
}

// TestBrowser_SkipsDangerousLinkSchemes pins the incomplete-scheme-check fix:
// links with script/active-content schemes (javascript:, data:, vbscript:,
// any casing) are excluded from the extracted interactive elements.
func TestBrowser_SkipsDangerousLinkSchemes(t *testing.T) {
ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.Write([]byte(`<html><body>
<a href="javascript:alert(1)">js</a>
<a href="JavaScript:alert(1)">js-caps</a>
<a href="data:text/html,<script>alert(1)</script>">data</a>
<a href="vbscript:msgbox(1)">vbs</a>
<a href="/safe">safe link</a>
</body></html>`))
}))
defer ts.Close()

b := newTestBrowserTool()
callJSON(t, b, `{"action":"navigate","url":"`+ts.URL+`"}`)

result := callJSON(t, b, `{"action":"snapshot"}`)
var r struct {
Content string `json:"content"`
}
mustUnmarshal(t, result, &r)

for _, bad := range []string{"javascript:", "JavaScript:", "data:", "vbscript:"} {
if strings.Contains(r.Content, bad) {
t.Errorf("snapshot contains %q link, want it filtered: %q", bad, r.Content)
}
}
if !strings.Contains(r.Content, "/safe") {
t.Errorf("snapshot missing the safe link: %q", r.Content)
}
}
7 changes: 7 additions & 0 deletions cmd/odek/serve.go
Original file line number Diff line number Diff line change
Expand Up @@ -1783,6 +1783,13 @@ func handleStatic(wsToken string) http.HandlerFunc {
Path: "/",
SameSite: http.SameSiteStrictMode,
HttpOnly: true,
// Secure is set even though the server usually runs on
// plain-http loopback: modern browsers treat localhost as a
// potentially trustworthy origin and accept Secure cookies
// there, and the UI always sends the token as a WebSocket
// subprotocol (and /api header), so a browser that drops
// the cookie loses nothing.
Secure: true,
// No explicit MaxAge/Expires so the cookie is a session cookie.
})
data = []byte(strings.Replace(string(data), "{{ODEK_WS_TOKEN}}", wsToken, 1))
Expand Down
51 changes: 37 additions & 14 deletions cmd/odek/ui/app.js
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,10 @@ function formatErrorMessage(msg) {

// ── Number formatting ──
function formatNum(n) {
// Coerce to a finite number so a crafted (non-numeric) value can never be
// reinterpreted as HTML when the result lands in an innerHTML assignment.
n = Number(n);
if (!isFinite(n)) n = 0;
if (n >= 1000) return (n / 1000).toFixed(n >= 10000 ? 0 : 1) + 'k';
return String(n);
}
Expand Down Expand Up @@ -305,8 +309,10 @@ function connect() {
if (lastAssistant) {
const stats = document.createElement('div');
stats.className = 'msg-stats';
const lat = Number(event.latency);
const latSafe = isFinite(lat) ? lat : 0;
const spans = [];
spans.push('<span title="Response time">⚡ ' + (event.latency < 1 ? (event.latency * 1000).toFixed(0) + 'ms' : event.latency.toFixed(1) + 's') + '</span>');
spans.push('<span title="Response time">⚡ ' + (latSafe < 1 ? (latSafe * 1000).toFixed(0) + 'ms' : latSafe.toFixed(1) + 's') + '</span>');
if (event.contextTokens != null) spans.push('<span title="Input tokens (prompt)">' + formatNum(event.contextTokens) + ' in</span>');
if (event.outputTokens != null) spans.push('<span title="Output tokens (completion)">' + formatNum(event.outputTokens) + ' out</span>');
// Cache metrics — show only when non-zero
Expand Down Expand Up @@ -1268,7 +1274,9 @@ function escapeHtml(s) {

function escapeAttr(s) {
if (!s) return '';
return s.replace(/"/g,'&quot;').replace(/'/g,'&#39;').replace(/&/g,'&amp;');
// & must be replaced first — doing it last double-escapes the entities
// introduced by the quote replacements (&quot; → &amp;quot;).
return s.replace(/&/g,'&amp;').replace(/"/g,'&quot;').replace(/'/g,'&#39;');
}

// ── Markdown to HTML (safe, no DOMPurify needed since we control input) ──
Expand Down Expand Up @@ -1460,18 +1468,33 @@ function clearAttachedFiles() {
}

function renderFileChips() {
if (attachedFiles.length === 0) {
fileChips.innerHTML = '';
return;
}
fileChips.innerHTML = attachedFiles.map((f, i) =>
'<span class="file-chip">' +
'<span class="chip-icon">📎</span>' +
'<span class="chip-name">' + escapeHtml(f.name) + '</span>' +
'<span class="chip-size">' + formatFileSize(f.size) + '</span>' +
'<span class="chip-remove" onclick="removeAttachedFile(' + i + ')">✕</span>' +
'</span>'
).join('');
// Build nodes with textContent rather than innerHTML so a file name can
// never be reinterpreted as markup.
fileChips.textContent = '';
attachedFiles.forEach((f, i) => {
const chip = document.createElement('span');
chip.className = 'file-chip';

const icon = document.createElement('span');
icon.className = 'chip-icon';
icon.textContent = '📎';

const name = document.createElement('span');
name.className = 'chip-name';
name.textContent = f.name;

const size = document.createElement('span');
size.className = 'chip-size';
size.textContent = formatFileSize(f.size);

const remove = document.createElement('span');
remove.className = 'chip-remove';
remove.textContent = '✕';
remove.addEventListener('click', () => removeAttachedFile(i));

chip.append(icon, name, size, remove);
fileChips.appendChild(chip);
});
}

function readFileAsText(file) {
Expand Down
15 changes: 15 additions & 0 deletions internal/danger/classifier.go
Original file line number Diff line number Diff line change
Expand Up @@ -463,6 +463,21 @@ func parseBrowserIP(host string) net.IP {
nums = append(nums, uint32(val))
}

// inet_aton requires every leading part to fit in one byte and the final
// part to fit in the remaining bytes (a.b → b ≤ 0xFFFFFF, a.b.c →
// c ≤ 0xFFFF, a.b.c.d → d ≤ 0xFF). Reject out-of-range parts instead of
// silently truncating them — "300.1.1.1" must not parse as 44.1.1.1 and
// mislead the internal-IP checks downstream.
for i, n := range nums {
max := uint64(0xFF)
if i == len(nums)-1 {
max = (uint64(1) << (8 * uint(5-len(nums)))) - 1
}
if uint64(n) > max {
return nil
}
}

switch len(nums) {
case 1:
// Single number: full 32-bit address
Expand Down
6 changes: 6 additions & 0 deletions internal/danger/whitebox_coverage_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -314,6 +314,12 @@ func TestParseBrowserIP(t *testing.T) {
{"::1", false, "::1"}, // IPv6
{"1.2.3.4.5", true, ""}, // too many parts
{"99999999999.1", true, ""}, // part exceeds 32 bits → nil
{"300.1.1.1", true, ""}, // leading part > 0xFF → nil, not 44.1.1.1
{"256.1", true, ""}, // leading part > 0xFF in short form
{"1.16777216", true, ""}, // final part > 0xFFFFFF in a.b form
{"1.2.65536", true, ""}, // final part > 0xFFFF in a.b.c form
{"1.2.3.256", true, ""}, // final part > 0xFF in a.b.c.d form
{"255.255.255.255", false, "255.255.255.255"}, // boundary: all parts at max
{"0xZZ.0.0.1", true, ""}, // bad hex
{"not.an.ip.addr", true, ""}, // non-numeric
{"", true, ""}, // empty
Expand Down
13 changes: 13 additions & 0 deletions internal/skills/importer.go
Original file line number Diff line number Diff line change
Expand Up @@ -412,6 +412,19 @@ func parsePrivateIP(host string) net.IP {
}
nums = append(nums, uint32(val))
}
// inet_aton requires every leading part to fit in one byte and the final
// part to fit in the remaining bytes (a.b → b ≤ 0xFFFFFF, a.b.c →
// c ≤ 0xFFFF, a.b.c.d → d ≤ 0xFF). Reject out-of-range parts instead of
// silently truncating them (e.g. "300.1.1.1" → 44.1.1.1).
for i, n := range nums {
max := uint64(0xFF)
if i == len(nums)-1 {
max = (uint64(1) << (8 * uint(5-len(nums)))) - 1
}
if uint64(n) > max {
return nil
}
}
switch len(nums) {
case 1:
return net.IPv4(byte(nums[0]>>24), byte(nums[0]>>16), byte(nums[0]>>8), byte(nums[0]))
Expand Down
5 changes: 5 additions & 0 deletions internal/skills/importer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -424,6 +424,11 @@ func TestIsPrivateHost(t *testing.T) {
{"0x7f000001", true}, // hex 127.0.0.1
{"127.1", true}, // short form 127.0.0.1
{"0x0.0x0.0x0.0x0", true}, // hex-dotted
// Out-of-range parts are rejected (browser semantics: invalid IP →
// hostname), not silently truncated — 10.300.1.1 must not parse as
// 10.44.1.1 and slip into the private range.
{"10.300.1.1", false},
{"300.1.1.1", false},
// RFC 6598 carrier-grade NAT
{"100.64.0.1", true},
{"100.127.255.254", true},
Expand Down
Loading