Reject reserved Hadoop keys in GCS fileSystemProperties

[akkaur]

Summary

The fileSystemProperties field of the GCS source (GCSFile) and GCS sink (GCS) is copied into the Hadoop configuration used by the GCS connector. The connector reads the storage endpoint, token server, proxy, credential source, impersonation target and several implementation classes from that configuration, so a user could point the connector at an arbitrary host or identity, including during pipeline validation where it runs with the identity of the CDAP system service.

Changes

• GCPUtils: RESERVED_FS_PROPERTY_KEYS / RESERVED_FS_PROPERTY_PREFIXES, isReservedFileSystemProperty() and validateFileSystemProperties(). The reserved set covers the GCS API endpoint and service path, token server, HTTP transport, proxy settings, all fs.gs.auth.* credential and impersonation keys, authorization handler and delegation token binding classes, the google.cloud.* fallback prefix the connector also reads, the deprecated service account aliases, the FileSystem implementation keys and the plugin-internal cdap.auth.* keys.
• GCSSourceConfig.validate() / GCSBatchSinkConfig.validate(): reserved keys are reported as a validation failure attributed to fileSystemProperties, one per offending key. In configurePipeline this runs before any file system is opened.
• GCPUtils.getFileSystemProperties() and the sink's user-property merge: reserved keys are dropped as a safeguard so values resolved from macros after validation cannot reach the Hadoop configuration either. Precedence of all other keys is unchanged.

Values are never inspected; a reserved key is rejected regardless of value. Tuning keys such as fs.gs.inputstream.*, fs.gs.outputstream.* and mapreduce.* keep working.

Tests

• GCPUtilsFileSystemPropertiesTest (new): reserved / allowed keys, null and whitespace handling, failure attribution, merge-point filtering with an auto-detect connection.
• GCSSourceConfigTest (new) and GCSBatchSinkTest: end-to-end validate() for endpoint, token server, fallback prefix, impersonation and plugin-internal keys; allowed keys still pass.

Note: testInvalidJsonFSProperties / testInvalidFSProperties fail locally on JDK 17+ because ValidationFailure.withStacktrace reflects on StackTraceElement; this reproduces on untouched develop and is unrelated to this change.

[gemini-code-assist]

Code Review

This pull request introduces validation and filtering of reserved Hadoop configuration keys in user-supplied file system properties to prevent unauthorized overrides of critical settings like API endpoints, proxies, and credentials. Key changes include defining reserved keys and prefixes in GCPUtils, implementing validation checks in GCSBatchSink and GCSSource, and adding extensive unit tests. The review feedback suggests copying the properties map in GCPUtils to avoid mutating the caller's map or throwing an UnsupportedOperationException on immutable maps, and replacing Mockito's internal FieldSetter in tests with standard Java reflection to ensure compatibility with newer Mockito versions.

[akkaur]

/gemini review

[gemini-code-assist]

Code Review

This pull request introduces a security mechanism to prevent users from overriding reserved Hadoop configuration keys through the 'fileSystemProperties' plugin property in GCS source and sink plugins. It defines lists of reserved keys and prefixes in GCPUtils, adds validation logic to reject them during pipeline validation, and filters them out during property retrieval to guard against macro-resolved values. Comprehensive unit tests are also added to verify this behavior. The review feedback points out a potential NullPointerException in GCPUtils.getFileSystemProperties if the input properties map is null, suggesting a defensive copy instead.

[akkaur]

/gemini review

[akkaur]

/gemini review

[gemini-code-assist]

Code Review

This pull request introduces a security mechanism to prevent users from overriding reserved Hadoop configuration keys (such as GCS API endpoints, token servers, proxies, and authentication properties) via the 'fileSystemProperties' plugin property in GCS Batch Sink and GCS Source. It defines lists of reserved keys and prefixes in GCPUtils, adds validation logic to reject these keys, and filters them out during property merging. Comprehensive unit tests are also added to verify this behavior. A review comment points out a potential NullPointerException in GCSBatchSink.java if config.getFileSystemProperties() returns null (e.g., when configured as blank or whitespace), suggesting a null check before iterating over its entry set.

[gemini-code-assist]

Code Review

This pull request introduces validation and filtering of user-supplied Hadoop configuration properties in GCS plugins to prevent overriding reserved keys (such as credentials, endpoints, and proxies). This is implemented by defining a set of reserved keys and prefixes in GCPUtils, validating them during configuration validation, and dropping them during runtime property resolution. A potential NullPointerException was identified in GCSBatchSink if config.getFileSystemProperties() returns null when merging properties.