Skip to content

fix(config): allow unsetting int/float config keys - #1320

Merged
ocervell merged 1 commit into
freelabz:mainfrom
Sanjays2402:fix/config-unset-int-typeerror
Aug 17, 2026
Merged

fix(config): allow unsetting int/float config keys#1320
ocervell merged 1 commit into
freelabz:mainfrom
Sanjays2402:fix/config-unset-int-typeerror

Conversation

@Sanjays2402

@Sanjays2402 Sanjays2402 commented Jul 25, 2026

Copy link
Copy Markdown
Contributor

Closes #1052

secator config unset <int-key> (e.g. runners.progress_update_frequency) raised TypeError: int() argument must be ... not 'NoneType'. Config.unset() calls set(key, None), but the type-coercion branch ran int(None) before the set_partial handler could interpret None as "reset to default". Coercion is now skipped when the value is None, so the existing reset path runs — same for float/Path keys.

New test_unset_numeric_key fails with that exact TypeError without the one-line source change and passes with it.

Summary by CodeRabbit

  • Bug Fixes

    • Fixed configuration handling when removing numeric settings.
    • Unset numeric values are now removed cleanly without conversion errors.
    • Default values are restored when removed settings are loaded again.
  • Tests

    • Added coverage for saving, unsetting, and reloading numeric configuration values.

secator config unset <int-key> raised TypeError. Config.unset() calls
set(key, None), and the type-coercion branch ran int(None)/float(None)
before the set_partial handler could interpret None as "reset to default".

Skip coercion when the value is None so the existing reset path runs.

Closes freelabz#1052
@coderabbitai

coderabbitai Bot commented Jul 25, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Walkthrough

Config.set() now avoids coercing None, allowing numeric configuration keys to be unset. A unit test verifies YAML removal and restoration of the default value after re-parsing.

Changes

Configuration unsetting

Layer / File(s) Summary
Skip coercion when unsetting numeric keys
secator/config.py, tests/unit/test_config.py
Config.set() bypasses type conversion for None; the test verifies numeric YAML persistence, key removal, and restoration of the default value.

Estimated code review effort: 2 (Simple) | ~10 minutes

Suggested reviewers: ocervell

Poem

A rabbit found a number key,
And cleared it cleanly, hop hooray!
No integer spell was cast on None,
The YAML shed its value then,
Defaults returned at break of day.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly describes the main change: fixing config unset for numeric keys.
Linked Issues check ✅ Passed The fix addresses issue #1052 by skipping coercion on None and adding a test for unsetting a numeric config key.
Out of Scope Changes check ✅ Passed The changes stay focused on config unsetting behavior and the related regression test.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

@coderabbitai coderabbitai Bot 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.

🧹 Nitpick comments (1)
tests/unit/test_config.py (1)

87-98: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Cover the float and Path reset paths too.

The PR objective includes integer, float, and Path keys, but this test only exercises an integer. Add equivalent cases for a float and a Path field so the shared None bypass is protected across all affected coercion branches.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/unit/test_config.py` around lines 87 - 98, Extend
test_unset_numeric_key to also unset and verify a float field and a Path field,
covering their set, save, unset, and save cycles. Assert each key is removed
from the persisted partial configuration and that Config.parse restores its
corresponding default, preserving coverage of the shared None bypass across all
coercion branches.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@tests/unit/test_config.py`:
- Around line 87-98: Extend test_unset_numeric_key to also unset and verify a
float field and a Path field, covering their set, save, unset, and save cycles.
Assert each key is removed from the persisted partial configuration and that
Config.parse restores its corresponding default, preserving coverage of the
shared None bypass across all coercion branches.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: f7debedf-6bde-40b8-bab9-3ba2dd034d2b

📥 Commits

Reviewing files that changed from the base of the PR and between 8a97c79 and 1554f95.

📒 Files selected for processing (2)
  • secator/config.py
  • tests/unit/test_config.py

@ocervell

Copy link
Copy Markdown
Contributor

Thanks for this ! I just came back from vacations, will test this shorty and if all good add it to the next release ;)

@ocervell
ocervell merged commit 4f380c0 into freelabz:main Aug 17, 2026
7 of 8 checks passed
ocervell added a commit that referenced this pull request Aug 18, 2026
🤖 I have created a release *beep* *boop*
---


##
[0.42.0](v0.41.0...v0.42.0)
(2026-08-18)


### Features

* **kev:** bundle a local CISA KEV mirror as an offline fallback
([#1331](#1331))
([4f1ff5c](4f1ff5c))
* **output-types:** tag known-exploited vulnerabilities with `kev`
([#1319](#1319))
([#1321](#1321))
([fc1dcaf](fc1dcaf))
* **security:** non-interactive sudo password for headless workers
([#1335](#1335))
([8aa0031](8aa0031))


### Bug Fixes

* **command:** don't crash on sudo prompt when TTY detection is wrong
([#1332](#1332))
([#1333](#1333))
([1ac6207](1ac6207))
* **command:** tty issue dumb terminals
([#1324](#1324))
([f5af590](f5af590))
* **config:** allow unsetting int/float config keys
([#1320](#1320))
([4f380c0](4f380c0))
* **docker:** bump alpine runtime to 3.23 (Go 1.25.10) for tool installs
([#1330](#1330))
([fa6c647](fa6c647))
* **lint,test:** cli.py lint + deterministic empty-arg query test
([#1327](#1327))
([7d79d91](7d79d91))
* **query:** allow empty ARG when a filter option is provided
([#1211](#1211))
([6206b14](6206b14))
* **runners:** nested target filter dropping task inputs since
[#1312](#1312)
([#1329](#1329))
([9929ac3](9929ac3))
* **tasks:** force system OpenSSL in testssl to fix missing
libproviders.so in Docker
([#1134](#1134))
([b528cfb](b528cfb))
* **wpscan:** don't leak wpscan's version status into the finding status
([#1326](#1326))
([428557b](428557b))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).
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.

fix(cli): secator c unset broken for int values

2 participants