Skip to content

fix: render solid rule borders in disabled-SSL warning banner#2229

Open
anxkhn wants to merge 4 commits into
kagent-dev:mainfrom
anxkhn:fix/adk-ssl-warning-banner-border
Open

fix: render solid rule borders in disabled-SSL warning banner#2229
anxkhn wants to merge 4 commits into
kagent-dev:mainfrom
anxkhn:fix/adk-ssl-warning-banner-border

Conversation

@anxkhn

@anxkhn anxkhn commented Jul 13, 2026

Copy link
Copy Markdown
Contributor
## What

When SSL verification is disabled, `create_ssl_context` (in
`python/packages/kagent-adk/src/kagent/adk/models/_ssl.py`) logs a warning banner
that is meant to be framed by three 60-character rules of `=`. Because of an
operator-precedence issue those rules render as a jagged column of single `=`
signs instead of solid lines. This makes a security-relevant warning look
malformed in the logs. This PR fixes the borders and adds a regression test.

## Why it happens

Each rule is written as a string literal immediately followed by `"=" * 60`, for
example:

```python
logger.warning(
    "\n"
    "=" * 60 + "\n"
    ...
)

Python applies implicit string-literal concatenation before the * repetition,
so "\n" "=" * 60 parses as ("\n=") * 60, i.e. sixty \n= pairs, rather than a
newline followed by a 60-character rule. All three rules in the banner (top,
separator, and bottom) are affected, so the logged warning looks like this:

=
=
=
... (60 rows)
⚠️  SSL VERIFICATION DISABLED ⚠️
=
=
... (60 rows)

The fix

Add an explicit + before each "=" * 60 so the repetition applies to "="
alone. All three borders now render as solid 60-character rules:

============================================================
⚠️  SSL VERIFICATION DISABLED ⚠️
============================================================
SSL certificate verification is disabled.
This should ONLY be used in development/testing.
Production deployments MUST use proper certificates.
============================================================

The change is limited to the string framing; no logging behavior, return value,
or TLS logic changes.

Testing

Added test_ssl_context_disabled_warning_banner_borders, which captures the
warning via caplog and asserts the banner contains exactly three solid
60-character = rules and no stray single-= rows. It fails on the current code
and passes with the fix.

cd python
uv run pytest packages/kagent-adk/tests/unittests/models/test_ssl.py -q
# 7 passed

ruff check and ruff format --diff are clean on both changed files.

When SSL verification is disabled, create_ssl_context logs a warning
banner meant to be framed by three 60-character rules of '='. Each
rule was written as a string literal immediately followed by "=" * 60,
e.g. "\n" "=" * 60. Python applies implicit string-literal
concatenation before the "*" repetition, so this parsed as
("\n=") * 60 and produced a jagged column of single '=' signs on their
own lines instead of one solid rule.

Add an explicit "+" before each "=" * 60 so the repetition applies to
"=" alone. All three borders now render as solid 60-character rules.

Add a regression test asserting the disabled-verification warning
contains exactly three solid 60-character rules and no stray
single-'=' rows.

Signed-off-by: Anas Khan <83116240+anxkhn@users.noreply.github.com>
Copilot AI review requested due to automatic review settings July 13, 2026 22:40
@anxkhn
anxkhn requested a review from a team as a code owner July 13, 2026 22:40
@github-actions github-actions Bot added the bug Something isn't working label Jul 13, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Fixes the disabled-SSL-verification warning banner in create_ssl_context so its framing rules render as solid 60-character = lines (rather than a jagged column caused by implicit string-literal concatenation precedence), and adds a regression test to lock in the correct formatting.

Changes:

  • Adjust warning-banner string construction in create_ssl_context to ensure "=" * 60 repeats only "=" (not "\n=").
  • Add a regression test that captures the warning via caplog and asserts exactly three 60-character = runs are present (and no stray single-= rows).

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
python/packages/kagent-adk/src/kagent/adk/models/_ssl.py Fixes operator-precedence/implicit-concatenation issue so the warning banner borders render as solid rules.
python/packages/kagent-adk/tests/unittests/models/test_ssl.py Adds a caplog-based regression test to verify the banner contains exactly three solid 60-character rule borders.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@peterj

peterj commented Jul 14, 2026

Copy link
Copy Markdown
Collaborator

makes sense, but we probably don't really need a unit test for this

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

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants