Conversation
rsenden
requested changes
Sep 23, 2026
| } | ||
|
|
||
| /** Neutralize CSV formula injection by prefixing affected text values with a single quote. */ | ||
| private static ObjectNode escapeFormulas(ObjectNode formattedRecord) { |
Contributor
There was a problem hiding this comment.
- Whether we should escape formula-like characters depends on the consumer. For example, if user loads CSV file into Excel, we should escape formulas, but we shouldn't escape if CSV file is consumed by some automation that consumes the records as-is without interpreting formulas. Probably best way to handle this is through the
--styleoption, i.e., add new styles to control whether output should be escaped. Need to think whether these styles should be CSV-specific, like[no-]csv-escapeor[no-]formula-escape, or more generic like[no-]escape. For safety, escaping enabled should be the default. - Current implementation operates at textual nodes only, not the final output string values. For example, it doesn't handle POJONodes for which the contents eventually get rendered as a string that could potentially represent a formula
- Current implementation modifies the existing
ObjectNode, which could cause issues if it's also used elsewhere (like if output is simultaneously being written to an fcli variable) - Current implementation causes an extra iteration over the fields, which may have a small performance impact
For the latter three, probably better to use a custom ObjectMapper/Serializer/SerializerModifier to escape the output as it's being written by Jackson.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes three issues reported by a Fortify scan:
2 Path Manipulation and 1 Privacy Violation
Report directory deletion followed symbolic links, so a symlink inside the target directory could cause files outside it to be
deleted. Now uses the existing FileUtils.deleteRecursive(), which does not follow links.
CSV output wrote values starting with =, +, -, @, tab, CR or LF as-is, which spreadsheet apps treat as formulas. These values are
now prefixed with a single quote. Numbers are unaffected.
The FVDL description parser recursed without any depth limit, so a deeply nested description in an FPR could cause a StackOverflowError. Nesting is now capped, with remaining content rendered as plain text.