feat(agent): use composer for vuln mgmt package info #962
Conversation
This file contains 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
|
lavarou
force-pushed
the
feat/vm/user-composer-for-package-info
branch
from
68dc921
to
6123bb4
Compare
zsistla
reviewed
Sep 12, 2024
zsistla
reviewed
Sep 12, 2024
|
What version(s) of composer will this work with? |
zsistla
reviewed
Sep 12, 2024
lavarou
commented
Sep 13, 2024
Comment on lines
+374
to
+403
| } else if 1 < len(splitCmd) && "composer-show.php" == splitCmd[1] { | ||
| lines := strings.Split(string(out), "\n") | ||
| version := "" | ||
| for _, line := range lines { | ||
| //fmt.Printf("line is |%s|\n", line) | ||
| splitLine := strings.Split(line, "=>") | ||
| if 2 == len(splitLine) { | ||
| name := strings.TrimSpace(splitLine[0]) | ||
| version = strings.TrimSpace(splitLine[1]) | ||
| pkgs.packages = append(pkgs.packages, PhpPackage{name, version}) | ||
| } | ||
| } |
There was a problem hiding this comment.
This is added so that composer mock can be used to test packages payload.
lavarou
commented
Sep 13, 2024
| } | ||
|
|
||
| nrl_verbosedebug(NRL_FRAMEWORK, "detected composer"); | ||
| NRPRG(txn)->composer_info.composer_detected = true; |
There was a problem hiding this comment.
This flag is not needed and the final version will probably have this removed.
So far it's been tested with 2.2 and 2.6. It should work with all versions >= 2.2. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## dev #962 +/- ##
==========================================
+ Coverage 78.35% 78.50% +0.14%
==========================================
Files 194 195 +1
Lines 26879 27057 +178
==========================================
+ Hits 21061 21241 +180
+ Misses 5818 5816 -2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
lavarou
commented
Sep 17, 2024
lavarou
commented
Sep 18, 2024
agent/fw_drupal8.c
Outdated
Comment on lines
693
to
694
| nr_fw_support_add_package_supportability_metric( | ||
| NRPRG(txn), PHP_PACKAGE_NAME, version, p); |
There was a problem hiding this comment.
This change (and similar changes in other fw_*.c modules) is required so that if composer api is enabled, the major version value, used in the metric name, will be from package info collected using composer api rather than from the package itself (from static VERSION constant or calling some version of get_version method in the package). The former method yields more useful and accurate results.
lavarou
commented
Sep 18, 2024
| typedef struct _nr_php_package_t { | ||
| char* package_name; | ||
| char* package_version; | ||
| nr_php_package_source_priority_t source_priority; |
There was a problem hiding this comment.
source_priority decides if package_version gets overwritten when a package with the same package_name is added to nr_php_packages_t.data again - see implementation of nr_php_packages_add_package.
lavarou
force-pushed
the
feat/vm/user-composer-for-package-info
branch
from
0654d0a
to
444a272
Compare
zsistla
reviewed
Sep 19, 2024
lavarou
force-pushed
the
feat/vm/user-composer-for-package-info
branch
from
444a272
to
ff6ef07
Compare
Use php_packages API rather than raw hashmap API to get the package from php_packages.
Co-authored-by: bduranleau-nr <[email protected]> Co-authored-by: Hitesh Ahuja <[email protected]>
Use correct value for failure when initializing a variable of zend_result type. Co-authored-by: Amber Sistla <[email protected]>
Co-authored-by: bduranleau-nr <[email protected]>
`vendor_path` needs to be freed when an error occurs.
ZNeumann
previously approved these changes
Sep 24, 2024
Ensure the code will not crash under unlikely conditions of pointers that are guaranteed to not be NULL being NULL.
Remove methods from the mock that are not relevant to composer instrumentation.
This reverts commit 80fdbdd. `zend_eval_string` expects `char *` not `const char *` so making `code` `const char *` does not make much sense because it would have to be casted back to `char *`.
Add more tests with composer API that throws Exception and Error.
Test autoloader detection when Composer api is enabled but package detection is disabled. Neither autoloader nor composer should be detected and used.
Test detection of packages when Composer is used but package metadata is bogus: - package_name is null, package_version is valid - package_name is valid, package_version is null - package_name is null, package_version is null.
This partially reverts commit 3a356ea. `NRSAFESTR` should still be used when accessing package_name and package_version for debug log message.
zsistla
previously approved these changes
Sep 25, 2024
hahuja2
previously approved these changes
Sep 25, 2024
Change how the package major number metrics are created. This now occurs in R_SHUTDOWN. This allows the use of package versions from ALL sources including the Composer API. Whenever instrumentation for a package detects a package, it can create a package suggestion. Initially if a version is not known it is fine to use PHP_PACKAGE_VERSION_UNKNOWN. If later the instrumentation determines a version from a class constant, etc, then the suggestion can be updated with the version. At the end of the transaction the suggestions are iterated over and the actual package data (which could include Composer API data) is referenced and a major number supportability metric is created with the best version available. --------- Co-authored-by: Michal Nowacki <[email protected]>
zsistla
approved these changes
Sep 26, 2024
hahuja2
approved these changes
Sep 26, 2024
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.
If possible, use Composer's runtime API to collect information about PHP packages used by the application for New Relic Vulnerability Management. This feature is disabled by default and can be enabled by setting
newrelic.vulnerability_management.composer_detection.enabledtotrue.