Skip to content

fix(owlwatch): remediate issues 620-622 + Plan 111 reconciliation - #626

Open
d-oit wants to merge 15 commits into
mainfrom
feat/remediate-owlwatch-620-622
Open

fix(owlwatch): remediate issues 620-622 + Plan 111 reconciliation#626
d-oit wants to merge 15 commits into
mainfrom
feat/remediate-owlwatch-620-622

Conversation

@d-oit

@d-oit d-oit commented Aug 9, 2026

Copy link
Copy Markdown
Owner

Summary

Remediates open OwlWatch issues and reconciles plans/ per Plan 111 (GOAP swarm).

Changes

Docs

  • plans/111-owlwatch-issues-pr624-625-2026-08-09.md

Validation

  • ruff clean on all changed Python (baseline pre-existing E501s untouched; .agents/ excluded from DeepSource)
  • pytest: 187 passed, 8 deselected (×2 stable)
  • pnpm lint / typecheck / test / build green (pre-commit quality gate)

Closes #621, #622. Documents #620 (upstream-blocked; closes). #623 tracks PR #624 and closes when that merges.

@vercel

vercel Bot commented Aug 9, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
do-knowledge-studio Ready Ready Preview, v0 Aug 10, 2026 5:11pm

@deepsource-io

deepsource-io Bot commented Aug 9, 2026

Copy link
Copy Markdown

DeepSource Code Review

We reviewed changes in 63558b9...854f0e2 on this pull request. Below is the summary for the review, and you can see the individual issues we found as inline review comments.

See full review on DeepSource ↗

Important

Some issues found as part of this review are outside of the diff in this pull request and aren't shown in the inline review comments due to GitHub's API limitations. You can see those issues on the DeepSource dashboard.

PR Report Card

Overall Grade   Security  

Reliability  

Complexity  

Hygiene  

Code Review Summary

Analyzer Status Updated (UTC) Details
JavaScript Aug 10, 2026 5:10p.m. Review ↗
Python Aug 10, 2026 5:10p.m. Review ↗
Shell Aug 10, 2026 5:10p.m. Review ↗
SQL Aug 10, 2026 5:10p.m. Review ↗

Important

AI Review is run only on demand for your team. We're only showing results of static analysis review right now. To trigger AI Review, comment @deepsourcebot review on this thread.

@codacy-production

Copy link
Copy Markdown
Contributor

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.

@owl-watch owl-watch Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🦉 OwlWatch reviewed this change — 2 new finding(s).

Comment thread .agents/skills/do-web-doc-resolver/scripts/resolve.py
Comment thread .agents/skills/do-web-doc-resolver/scripts/resolve.py
Comment thread .agents/skills/do-web-doc-resolver/scripts/resolve.py
Comment thread .agents/skills/do-web-doc-resolver/scripts/resolve.py
Comment thread .agents/skills/do-web-doc-resolver/scripts/resolve.py Outdated

@owl-watch owl-watch Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🦉 OwlWatch reviewed this change — 1 new finding(s).

Comment thread .agents/skills/do-web-doc-resolver/scripts/resolve.py Outdated

@owl-watch owl-watch Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🦉 OwlWatch reviewed this change — 1 new finding(s).

Comment thread .agents/skills/do-web-doc-resolver/scripts/resolve.py

@owl-watch owl-watch Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🦉 OwlWatch reviewed this change — 1 new finding(s).

Comment thread .agents/skills/do-web-doc-resolver/scripts/resolve.py Outdated

@owl-watch owl-watch Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🦉 OwlWatch reviewed this change — 1 new finding(s).

Additional findings:

  • [HIGH · tracker] SSRF protection bypass in is_safe_url due to swallowed exceptions (.agents/skills/do-web-doc-resolver/scripts/utils.py:114)

Comment thread .agents/skills/do-web-doc-resolver/scripts/resolve.py Outdated

@owl-watch owl-watch Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🦉 OwlWatch reviewed this change — 2 new finding(s).

Comment thread .agents/skills/do-web-doc-resolver/scripts/providers_impl.py
Comment thread .agents/skills/do-web-doc-resolver/scripts/resolve.py Outdated

@owl-watch owl-watch Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🦉 OwlWatch reviewed this change — 1 new finding(s).

Additional findings:

  • [HIGH · tracker] Incorrect return type in LLM fallback path (.agents/skills/do-web-doc-resolver/scripts/resolve.py:135)

@owl-watch owl-watch Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🦉 OwlWatch reviewed this change — 1 new finding(s).

Additional findings:

  • [MEDIUM · hunter] Potential API key exposure in synthesize_results (.agents/skills/do-web-doc-resolver/scripts/resolve.py:143)

@owl-watch owl-watch Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🦉 OwlWatch reviewed this change — 1 new finding(s).

Additional findings:

  • [MEDIUM · hunter] Potential insecure handling of Mistral API key in synthesize_results (.agents/skills/do-web-doc-resolver/scripts/resolve.py:157)

@owl-watch owl-watch Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🦉 OwlWatch reviewed this change — 1 new finding(s).

Comment thread .agents/skills/do-web-doc-resolver/scripts/resolve.py

@owl-watch owl-watch Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🦉 OwlWatch reviewed this change — 2 new finding(s).

Additional findings:

  • [MEDIUM · tracker] Unmanaged network request in LLM synthesis bypasses application session policy (.agents/skills/do-web-doc-resolver/scripts/resolve.py:157)

Comment thread .agents/skills/do-web-doc-resolver/scripts/resolve.py
@d-oit d-oit closed this Aug 9, 2026
auto-merge was automatically disabled August 9, 2026 19:09

Pull request was closed

@d-oit d-oit reopened this Aug 9, 2026
@d-oit
d-oit enabled auto-merge (squash) August 9, 2026 19:09
class TestIsSafeUrl:
"""SSRF safety checks."""

def test_blocks_non_http_schemes(self):

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Method doesn't use the class instance and could be converted into a static method


The method doesn't use its bound instance. Decorate this method with @staticmethod decorator, so that Python does not have to instantiate a bound method for every instance of this class thereby saving memory and computation. Read more about staticmethods here.

assert is_safe_url("javascript:alert(1)") is False
assert is_safe_url("data:text/plain;base64,AA==") is False

def test_blocks_localhost_aliases(self):

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Method doesn't use the class instance and could be converted into a static method


The method doesn't use its bound instance. Decorate this method with @staticmethod decorator, so that Python does not have to instantiate a bound method for every instance of this class thereby saving memory and computation. Read more about staticmethods here.

assert is_safe_url("http://127.0.0.1/foo") is False
assert is_safe_url("http://0.0.0.0/foo") is False

def test_blocks_private_ipv4(self):

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Method doesn't use the class instance and could be converted into a static method


The method doesn't use its bound instance. Decorate this method with @staticmethod decorator, so that Python does not have to instantiate a bound method for every instance of this class thereby saving memory and computation. Read more about staticmethods here.

assert is_safe_url("http://192.168.1.1/foo") is False
assert is_safe_url("http://169.254.169.254/latest/meta-data/") is False

def test_blocks_private_ipv6(self):

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Method doesn't use the class instance and could be converted into a static method


The method doesn't use its bound instance. Decorate this method with @staticmethod decorator, so that Python does not have to instantiate a bound method for every instance of this class thereby saving memory and computation. Read more about staticmethods here.

assert is_safe_url("http://[::1]/foo") is False
assert is_safe_url("http://[fc00::1]/foo") is False

def test_blocks_public_hostname_resolving_to_private_ip(self):

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Method doesn't use the class instance and could be converted into a static method


The method doesn't use its bound instance. Decorate this method with @staticmethod decorator, so that Python does not have to instantiate a bound method for every instance of this class thereby saving memory and computation. Read more about staticmethods here.

with patch("socket.getaddrinfo", return_value=[addr]):
assert is_safe_url("http://example.internal/foo") is False

def test_accepts_public_https_url(self):

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Method doesn't use the class instance and could be converted into a static method


The method doesn't use its bound instance. Decorate this method with @staticmethod decorator, so that Python does not have to instantiate a bound method for every instance of this class thereby saving memory and computation. Read more about staticmethods here.

class TestSocketTimeoutRestore:
"""The DNS-resolution path must restore the previous default timeout."""

def test_restores_previous_timeout_after_resolution(self):

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Method doesn't use the class instance and could be converted into a static method


The method doesn't use its bound instance. Decorate this method with @staticmethod decorator, so that Python does not have to instantiate a bound method for every instance of this class thereby saving memory and computation. Read more about staticmethods here.

finally:
socket.setdefaulttimeout(original)

def test_restores_previous_timeout_when_blocked(self):

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Method doesn't use the class instance and could be converted into a static method


The method doesn't use its bound instance. Decorate this method with @staticmethod decorator, so that Python does not have to instantiate a bound method for every instance of this class thereby saving memory and computation. Read more about staticmethods here.

finally:
socket.setdefaulttimeout(original)

def test_restores_when_resolution_raises(self):

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Method doesn't use the class instance and could be converted into a static method


The method doesn't use its bound instance. Decorate this method with @staticmethod decorator, so that Python does not have to instantiate a bound method for every instance of this class thereby saving memory and computation. Read more about staticmethods here.

@owl-watch owl-watch Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🦉 OwlWatch reviewed this change — 2 new finding(s).

Additional findings:

  • [LOW · tracker] Potential global state mutation in is_safe_url (.agents/skills/do-web-doc-resolver/scripts/utils.py:128)



def _detect_error_type(error: Exception) -> ErrorType:
"""Classify an exception message into the matching ErrorType category."""

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🦉 OwlWatch [MEDIUM · mason] High cyclomatic complexity in _detect_error_type

Replace long chain of if-any conditions with a lookup table or registry for cleaner mapping of error messages to ErrorType.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

config documentation Documentation improvements skills

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Owlwatch] Long function resolve_url_stream (230 lines)

1 participant