Skip to content

Conversation

@BeArchiTek
Copy link
Contributor

@BeArchiTek BeArchiTek commented Sep 15, 2025

Fixed issue with improperly escaped special characters in hfid fields and other string values in GraphQL mutations by implementing proper JSON-style string escaping

Summary by CodeRabbit

  • Bug Fixes
    • Correctly escape special characters in string values (including hfid fields) within GraphQL mutations, preventing malformed requests and related errors.
  • Documentation
    • Added a changelog entry noting the string-escaping fix.

@BeArchiTek BeArchiTek requested a review from a team September 15, 2025 14:15
@BeArchiTek BeArchiTek self-assigned this Sep 15, 2025
@coderabbitai
Copy link

coderabbitai bot commented Sep 15, 2025

Walkthrough

This change adds a changelog entry documenting a fix for escaping special characters in hfid fields and other strings in GraphQL mutations. In infrahub_sdk/graphql.py, json is imported and convert_to_graphql_as_string now uses json.dumps(value) for JSON-compatible string escaping instead of manual quoting (f""{value}""). Handling for null, Enum, booleans, lists, and BaseModel remains unchanged. No exported/public API signatures were modified.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The PR title "fix improperly escaped special characters in HFID" accurately and concisely describes the primary change: fixing string-escaping for HFID values (the implementation shows JSON-style escaping in infrahub_sdk/graphql.py). It is specific to the bug being fixed and avoids vague phrasing or file lists, so a reviewer scanning history will understand the main intent.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch bkr-fix-hfid-double-quotes

Tip

👮 Agentic pre-merge checks are now available in preview!

Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.

  • Built-in checks – Quickly apply ready-made checks to enforce title conventions, require pull request descriptions that follow templates, validate linked issues for compliance, and more.
  • Custom agentic checks – Define your own rules using CodeRabbit’s advanced agentic capabilities to enforce organization-specific policies and workflows. For example, you can instruct CodeRabbit’s agent to verify that API documentation is updated whenever API schema files are modified in a PR. Note: Upto 5 custom checks are currently allowed during the preview period. Pricing for this feature will be announced in a few weeks.

Please see the documentation for more information.

Example:

reviews:
  pre_merge_checks:
    custom_checks:
      - name: "Undocumented Breaking Changes"
        mode: "warning"
        instructions: |
          Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).

Please share your feedback with us on this Discord post.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@cloudflare-workers-and-pages
Copy link

Deploying infrahub-sdk-python with  Cloudflare Pages  Cloudflare Pages

Latest commit: 4ec8102
Status: ✅  Deploy successful!
Preview URL: https://319a0365.infrahub-sdk-python.pages.dev
Branch Preview URL: https://bkr-fix-hfid-double-quotes.infrahub-sdk-python.pages.dev

View logs

@codecov
Copy link

codecov bot commented Sep 15, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.

@@           Coverage Diff           @@
##           stable     #547   +/-   ##
=======================================
  Coverage   75.59%   75.59%           
=======================================
  Files         100      100           
  Lines        8891     8892    +1     
  Branches     1750     1750           
=======================================
+ Hits         6721     6722    +1     
  Misses       1688     1688           
  Partials      482      482           
Flag Coverage Δ
integration-tests 34.69% <50.00%> (+<0.01%) ⬆️
python-3.10 48.11% <50.00%> (-0.03%) ⬇️
python-3.11 48.11% <50.00%> (-0.03%) ⬇️
python-3.12 48.11% <50.00%> (-0.01%) ⬇️
python-3.13 48.11% <50.00%> (-0.01%) ⬇️
python-3.9 46.75% <50.00%> (+0.01%) ⬆️
python-filler-3.12 25.00% <50.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
infrahub_sdk/graphql.py 99.11% <100.00%> (+<0.01%) ⬆️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (3)
changelog/+escape-hfid.fixed.md (1)

1-1: Tighten wording and style (capitalization + period).

Minor polish for consistency with the PR title and house style.

-- Fixed issue with improperly escaped special characters in `hfid` fields and other string values in GraphQL mutations by implementing proper JSON-style string escaping
+- Fixed improperly escaped special characters in `HFID` fields and other string values in GraphQL mutations by implementing JSON‑style string escaping.
infrahub_sdk/graphql.py (2)

22-24: Avoid unnecessary ASCII-escaping of Unicode.

json.dumps defaults to ensure_ascii=True, which turns non‑ASCII into \uXXXX. GraphQL accepts raw Unicode; keeping it improves readability and reduces diff noise.

-        return json.dumps(value)
+        return json.dumps(value, ensure_ascii=False)

15-16: Make variable references explicit; avoid magic “$”-prefix heuristic.

Treating any user string starting with “$” as a variable reference is ambiguous and can misrender legitimate values. Prefer an explicit sentinel type.

+class VarRef(str):
+    """Explicit GraphQL variable reference wrapper."""
+    pass
...
-    if isinstance(value, str) and value.startswith("$"):
-        return value
+    if isinstance(value, VarRef):
+        return str(value)

If you want, I can add a minimal VarRef usage example and tests for both VarRef("$id") and a literal "$acct-001".

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e358da2 and 4ec8102.

📒 Files selected for processing (2)
  • changelog/+escape-hfid.fixed.md (1 hunks)
  • infrahub_sdk/graphql.py (2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

When implementing Infrahub checks, subclass InfrahubCheck and override validate(data); do not implement or rely on a check() method

Files:

  • infrahub_sdk/graphql.py
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
  • GitHub Check: unit-tests (3.10)
  • GitHub Check: unit-tests (3.11)
  • GitHub Check: unit-tests (3.12)
  • GitHub Check: unit-tests (3.13)
  • GitHub Check: integration-tests-latest-infrahub
🔇 Additional comments (1)
infrahub_sdk/graphql.py (1)

22-24: Correct fix: use json.dumps for GraphQL string escaping.

This removes ad‑hoc quoting and correctly handles quotes, backslashes, control chars, and Unicode.

@BeArchiTek BeArchiTek merged commit 10618ee into stable Sep 15, 2025
20 checks passed
@BeArchiTek BeArchiTek deleted the bkr-fix-hfid-double-quotes branch September 15, 2025 15:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants