Merge pull request #1008 from sechkova/rework_path_checks

Adopt a consistent behavior when adding targets and paths

Author: lukpueh

docs/TUTORIAL.md

@@ -352,15 +352,8 @@ the target filepaths to metadata.
 # in targets metadata.
 >>> repository = load_repository('repository')
 
-# get_filepaths_in_directory() returns a list of file paths in a directory. It
-# can also return files in sub-directories if 'recursive_walk' is True.
->>> list_of_targets = repository.get_filepaths_in_directory(
-...     "repository/targets/", recursive_walk=False, followlinks=True)
-
-# Note: Since we set the 'recursive_walk' argument to false, the 'myproject'
-# sub-directory is excluded from 'list_of_targets'.
->>> list_of_targets
-['/path/to/repository/targets/file2.txt', '/path/to/repository/targets/file1.txt', '/path/to/repository/targets/file3.txt']
+# Create a list of all targets in the directory.
+>>> list_of_targets = ['file1.txt', 'file2.txt', 'file3.txt']
 
 # Add the list of target paths to the metadata of the top-level Targets role.
 # Any target file paths that might already exist are NOT replaced, and
@@ -376,8 +369,11 @@ the target filepaths to metadata.
 # (octal number specifying file access for owner, group, others e.g., 0755) is
 # added alongside the default fileinfo.  All target objects in metadata include
 # the target's filepath, hash, and length.
->>> target4_filepath = os.path.abspath("repository/targets/myproject/file4.txt")
->>> octal_file_permissions = oct(os.stat(target4_filepath).st_mode)[4:]
+# Note: target path passed to add_target() method has to be relative
+# to the targets directory or an exception is raised.
+>>> target4_filepath = 'myproject/file4.txt'
+>>> target4_abspath = os.path.abspath(os.path.join('repository', 'targets', target4_filepath))
+>>> octal_file_permissions = oct(os.stat(target4_abspath).st_mode)[4:]
 >>> custom_file_permissions = {'file_permissions': octal_file_permissions}
 >>> repository.targets.add_target(target4_filepath, custom_file_permissions)
 ```
@@ -498,7 +494,6 @@ targets and generate signed metadata.
 
 # Make a delegation (delegate trust of 'myproject/*.txt' files) from "targets"
 # to "unclaimed", where "unclaimed" initially contains zero targets.
-# NOTE: Please ignore the warning about the path pattern's location (see #963)
 >>> repository.targets.delegate('unclaimed', [public_unclaimed_key], ['myproject/*.txt'])
 
 # Thereafter, we can access the delegated role by its name to e.g. add target
@@ -635,8 +630,7 @@ to some role.
 >>> repository.targets('unclaimed').remove_target("myproject/file4.txt")
 
 # Get a list of target paths for the hashed bins.
->>> targets = repository.get_filepaths_in_directory(
-...     'repository/targets/myproject', recursive_walk=True)
+>>> targets = ['myproject/file4.txt']
 
 # Delegate trust to 32 hashed bin roles. Each role is responsible for the set
 # of target files, determined by the path hash prefix. TUF evenly distributes

tests/test_mix_and_match_attack.py

@@ -218,7 +218,7 @@ def test_with_tuf(self):
     # new version is generated.
     with open(file3_path, 'wt') as file_object:
       file_object.write('This is role2\'s target file.')
-    repository.targets('role1').add_target(file3_path)
+    repository.targets('role1').add_target(os.path.basename(file3_path))
 
     repository.writeall()
 

tests/test_repository_tool.py

@@ -165,15 +165,15 @@ def test_root_role_versioning(self):
     repository.timestamp.load_signing_key(timestamp_privkey)
 
     # (4) Add target files.
-    target1 = os.path.join(targets_directory, 'file1.txt')
-    target2 = os.path.join(targets_directory, 'file2.txt')
-    target3 = os.path.join(targets_directory, 'file3.txt')
+    target1 = 'file1.txt'
+    target2 = 'file2.txt'
+    target3 = 'file3.txt'
     repository.targets.add_target(target1)
     repository.targets.add_target(target2)
 
 
     # (5) Perform delegation.
-    repository.targets.delegate('role1', [role1_pubkey], [os.path.basename(target3)])
+    repository.targets.delegate('role1', [role1_pubkey], [target3])
     repository.targets('role1').load_signing_key(role1_privkey)
 
     # (6) Write repository.

tests/test_root_versioning_integration.py

@@ -223,31 +223,27 @@ def test_tutorial(self):
 
 
     repository = load_repository('repository')
-    list_of_targets = repository.get_filepaths_in_directory(
-        os.path.join('repository', 'targets'), recursive_walk=False, followlinks=True)
-
-    self.assertListEqual(sorted(list_of_targets), [
-        os.path.abspath(os.path.join('repository', 'targets', 'file1.txt')),
-        os.path.abspath(os.path.join('repository', 'targets', 'file2.txt')),
-        os.path.abspath(os.path.join('repository', 'targets', 'file3.txt'))])
 
+    # TODO: replace the hard-coded list of targets with a helper
+    # method that returns a list of normalized relative target paths
+    list_of_targets = ['file1.txt', 'file2.txt', 'file3.txt']
 
     repository.targets.add_targets(list_of_targets)
 
     self.assertTrue('file1.txt' in repository.targets.target_files)
     self.assertTrue('file2.txt' in repository.targets.target_files)
     self.assertTrue('file3.txt' in repository.targets.target_files)
 
-
-    target4_filepath = os.path.abspath(os.path.join(
-        'repository', 'targets', 'myproject', 'file4.txt'))
-    octal_file_permissions = oct(os.stat(target4_filepath).st_mode)[4:]
+    target4_filepath = 'myproject/file4.txt'
+    target4_abspath = os.path.abspath(os.path.join(
+        'repository', 'targets', target4_filepath))
+    octal_file_permissions = oct(os.stat(target4_abspath).st_mode)[4:]
     custom_file_permissions = {'file_permissions': octal_file_permissions}
     repository.targets.add_target(target4_filepath, custom_file_permissions)
     # Note that target filepaths specified in the repo use '/' even on Windows.
     # (This is important to make metadata platform-independent.)
     self.assertTrue(
-        os.path.join('myproject/file4.txt') in repository.targets.target_files)
+        os.path.join(target4_filepath) in repository.targets.target_files)
 
 
     # Skipping user entry of password
@@ -346,8 +342,7 @@ def test_tutorial(self):
     # ----- Tutorial Section: Delegate to Hashed Bins
     repository.targets('unclaimed').remove_target("myproject/file4.txt")
 
-    targets = repository.get_filepaths_in_directory(
-        os.path.join('repository', 'targets', 'myproject'), recursive_walk=True)
+    targets = ['myproject/file4.txt']
 
     # Patch logger to assert that it accurately logs the output of hashed bin
     # delegation. The logger is called multiple times, first with info level

tests/test_tutorial.py

@@ -863,7 +863,7 @@ def test_3__update_metadata_if_changed(self):
 
     # Modify one target file on the remote repository.
     repository = repo_tool.load_repository(self.repository_directory)
-    target3 = os.path.join(self.repository_directory, 'targets', 'file3.txt')
+    target3 = 'file3.txt'
 
     repository.targets.add_target(target3)
     repository.root.version = repository.root.version + 1
@@ -936,7 +936,7 @@ def test_4_refresh(self):
                       unsafely_update_root_if_necessary=False)
 
     repository = repo_tool.load_repository(self.repository_directory)
-    target3 = os.path.join(self.repository_directory, 'targets', 'file3.txt')
+    target3 = 'file3.txt'
 
     repository.targets.add_target(target3)
     repository.targets.load_signing_key(self.role_keys['targets']['private'])
@@ -969,8 +969,6 @@ def test_4_refresh(self):
 
     # Verify that the client's metadata was updated.
     targets_metadata = self.repository_updater.metadata['current']['targets']
-    targets_directory = os.path.join(self.repository_directory, 'targets')
-    target3 = target3[len(targets_directory) + 1:]
     self.assertTrue(target3 in targets_metadata['targets'])
 
     # Verify the expected version numbers of the updated roles.
@@ -1142,12 +1140,13 @@ def test_6_get_one_valid_targetinfo(self):
     # Test updater.get_one_valid_targetinfo() backtracking behavior (enabled by
     # default.)
     targets_directory = os.path.join(self.repository_directory, 'targets')
-    foo_directory = os.path.join(targets_directory, 'foo')
+    os.makedirs(os.path.join(targets_directory, 'foo'))
+
+    foo_package = 'foo/foo1.1.tar.gz'
     foo_pattern = 'foo/foo*.tar.gz'
-    os.makedirs(foo_directory)
 
-    foo_package = os.path.join(foo_directory, 'foo1.1.tar.gz')
-    with open(foo_package, 'wb') as file_object:
+    foo_fullpath = os.path.join(targets_directory, foo_package)
+    with open(foo_fullpath, 'wb') as file_object:
       file_object.write(b'new release')
 
     # Modify delegations on the remote repository to test backtracking behavior.
@@ -1452,7 +1451,7 @@ def test_7_updated_targets(self):
 
     length, hashes = securesystemslib.util.get_file_details(target1)
 
-    repository.targets.add_target(target1)
+    repository.targets.add_target(os.path.basename(target1))
     repository.targets.load_signing_key(self.role_keys['targets']['private'])
     repository.snapshot.load_signing_key(self.role_keys['snapshot']['private'])
 
@@ -1461,7 +1460,7 @@ def test_7_updated_targets(self):
 
     length, hashes = securesystemslib.util.get_file_details(target1)
 
-    repository.targets.add_target(target1)
+    repository.targets.add_target(os.path.basename(target1))
     repository.targets.load_signing_key(self.role_keys['targets']['private'])
     repository.snapshot.load_signing_key(self.role_keys['snapshot']['private'])
     repository.timestamp.load_signing_key(self.role_keys['timestamp']['private'])
@@ -2065,7 +2064,7 @@ def test_get_valid_targetinfo(self):
     repository.targets.remove_target(os.path.basename(target1))
 
     custom_field = {"custom": "my_custom_data"}
-    repository.targets.add_target(target1, custom_field)
+    repository.targets.add_target(os.path.basename(target1), custom_field)
     repository.targets.load_signing_key(self.role_keys['targets']['private'])
     repository.snapshot.load_signing_key(self.role_keys['snapshot']['private'])
     repository.timestamp.load_signing_key(self.role_keys['timestamp']['private'])
@@ -2091,7 +2090,7 @@ def test_get_valid_targetinfo(self):
 
     repository.targets.remove_target(os.path.basename(target1))
 
-    repository.targets.add_target(target1)
+    repository.targets.add_target(os.path.basename(target1))
     repository.targets.load_signing_key(self.role_keys['targets']['private'])
     repository.snapshot.load_signing_key(self.role_keys['snapshot']['private'])
     repository.timestamp.load_signing_key(self.role_keys['timestamp']['private'])

tests/test_updater.py

@@ -1319,7 +1319,7 @@ def generate_targets_metadata(targets_directory, target_files, version,
         raise securesystemslib.exceptions.Error('use_existing_hashes option set'
             ' but fileinfo\'s length is not set')
 
-      filedict[target.replace('\\', '/').lstrip('/')] = fileinfo
+      filedict[target] = fileinfo
 
   else:
     filedict = _generate_targets_fileinfo(target_files, targets_directory,
@@ -1393,7 +1393,7 @@ def _generate_targets_fileinfo(target_files, targets_directory,
     # the target's fileinfo dictionary) if specified here.
     custom_data = fileinfo.get('custom', None)
 
-    filedict[relative_targetpath.replace('\\', '/').lstrip('/')] = \
+    filedict[relative_targetpath] = \
         get_metadata_fileinfo(target_path, custom_data)
 
     # Copy 'target_path' to 'digest_target' if consistent hashing is enabled.