Skip to content

fix: add missing format placeholders in log.error and log.debug calls#60

Open
piyush140104 wants to merge 1 commit intoOWASP:masterfrom
piyush140104:fix/preprocess-log-format-strings
Open

fix: add missing format placeholders in log.error and log.debug calls#60
piyush140104 wants to merge 1 commit intoOWASP:masterfrom
piyush140104:fix/preprocess-log-format-strings

Conversation

@piyush140104
Copy link
Copy Markdown

What

Added missing %s placeholders in 5 logging calls in honeytraps/waf_modsec/preprocess-modsec-log.py.

Fix

log.debug("Processed file lines: %s", procIndex) # line 95
log.debug("Original file lines: %s", origIndex) # line 97
log.debug("Head: %s", index) # line 101
log.debug("Write Mode: %s", writeMode) # line 102
log.error("Error writing to file: %s", e) # line 117

Reviewers

@adrianwinckles

Test

import logging
logging.basicConfig(level=logging.ERROR)
log = logging.getLogger('test')
log.error("Error writing to file", Exception("disk full"))
#Before: triggers --- Logging error --- TypeError, actual error never logged
log.error("Error writing to file: %s", Exception("disk full"))
#After: ERROR:test:Error writing to file: disk full

Closes #59

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Fixes incorrect Python logging calls in the ModSecurity log preprocessing script that previously caused TypeError formatting failures and suppressed useful log output.

Changes:

  • Add missing %s placeholders to log.debug(...) calls in processLog.
  • Add missing %s placeholder to a log.error(...) call in the file-write exception handler.
Comments suppressed due to low confidence (1)

honeytraps/waf_modsec/preprocess-modsec-log.py:106

  • lines can be None when the original log file is missing (see getLines returning None on FileNotFoundError), but processLog still calls len(lines) in the loop. This will raise a TypeError and stop processing; consider treating missing files as an empty list or returning early with a log message when lines is None.
        log.debug("Write Mode: %s", writeMode)

        # Log line prasing/modification
        newLines = []
        for i in range(index, len(lines)):

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

Comment on lines 113 to +117
file.write(line + "\n")
#log.info(newLines)
file.close()
except Exception as e:
log.error("Error writing to file", e)
log.error("Error writing to file: %s", e)
Copy link

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

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

The file handle may be left open if an exception occurs during file.write(...) (since file.close() is only reached on the success path). Using a context manager (with open(...) as f:) avoids leaks and also simplifies the code; also consider log.exception(...) or log.error(..., exc_info=True) here so the traceback is captured for operational debugging.

Copilot uses AI. Check for mistakes.
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.

[Bug] log.error() and log.debug() calls in preprocess-modsec-log.py silently suppress errors due to missing format placeholders

2 participants