Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions NEWS
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ TBD
* Prevent writing duplicate groups to metadata file
* Disable all filesystem access in flatpak-builder --run sandbox
* Add ability to set custom fusermount path
* Support installing files from file sources

Changes in 1.4.6
================
Expand Down
12 changes: 11 additions & 1 deletion data/flatpak-manifest.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -1019,6 +1019,15 @@
"dest-filename": {
"description": "Filename to for the downloaded file, defaults to the basename of url.",
"type": "string"
},
"install-dir": {
"description": "Directory relative to FLATPAK_DEST where the file will be installed. The directory is created if absent and the install is skipped with a warning if the file exists in the target path.",
"type": "string"
},
"install-mode": {
"description": "Optional octal permissions applied to the installed file, e.g. '755' or '0755'. Requires install-dir. If absent, the original permissions of the file are preserved.",
"type": "string",
"pattern": "^[0-7]{3,4}$"
}
},
"patternProperties": {
Expand All @@ -1037,7 +1046,8 @@
{ "required": [ "sha1" ] },
{ "required": [ "md5" ] }
]
}
},
"install-mode": { "required": [ "install-dir" ] }
},
"additionalProperties": false
},
Expand Down
22 changes: 22 additions & 0 deletions doc/flatpak-manifest.xml
Original file line number Diff line number Diff line change
Expand Up @@ -729,6 +729,28 @@
<term><option>dest-filename</option> (string)</term>
<listitem><para>Filename to for the downloaded file, defaults to the basename of url.</para></listitem>
</varlistentry>
<varlistentry>
<term><option>install-dir</option> (string)</term>
<listitem>
<para>
Install the file into this directory relative to <literal>FLATPAK_DEST</literal>.
The directory will be created if it doesn't exist and the install will be
skipped with a warning if the file already exists in the target path.
</para>
</listitem>
</varlistentry>
<varlistentry>
<term><option>install-mode</option> (string)</term>
<listitem>
<para>
Optional octal permissions to apply to the installed file,
such as <literal>0755</literal> or <literal>755</literal>.
This option requires <option>install-dir</option> to be set. If
this is not set the original permissions on the file will be
preserved.
</para>
</listitem>
</varlistentry>
</variablelist>
</refsect3>
<refsect3>
Expand Down
14 changes: 14 additions & 0 deletions src/builder-module.c
Original file line number Diff line number Diff line change
Expand Up @@ -2211,6 +2211,20 @@ builder_module_build_helper (BuilderModule *self,
}
}

for (GList *l = self->sources; l != NULL; l = l->next)
{
BuilderSource *source = l->data;

if (!builder_source_is_enabled (source, context))
continue;

if (!builder_source_install (source, source_dir, context, error))
{
g_prefix_error (error, "module %s: ", self->name);
return FALSE;
}
}

/* Run unit tests */

if (self->run_tests && builder_context_get_run_tests (context))
Expand Down
136 changes: 136 additions & 0 deletions src/builder-source-file.c
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,8 @@ struct BuilderSourceFile
char *dest_filename;
char *http_referer;
gboolean disable_http_decompression;
char *install_dir;
char *install_mode;
};

typedef struct
Expand All @@ -67,6 +69,8 @@ enum {
PROP_MIRROR_URLS,
PROP_HTTP_REFERER,
PROP_DISABLE_HTTP_DECOMPRESSION,
PROP_INSTALL_DIR,
PROP_INSTALL_MODE,
LAST_PROP
};

Expand All @@ -84,6 +88,8 @@ builder_source_file_finalize (GObject *object)
g_free (self->dest_filename);
g_free (self->http_referer);
g_strfreev (self->mirror_urls);
g_free (self->install_dir);
g_free (self->install_mode);

G_OBJECT_CLASS (builder_source_file_parent_class)->finalize (object);
}
Expand Down Expand Up @@ -138,6 +144,14 @@ builder_source_file_get_property (GObject *object,
g_value_set_boolean (value, self->disable_http_decompression);
break;

case PROP_INSTALL_DIR:
g_value_set_string (value, self->install_dir);
break;

case PROP_INSTALL_MODE:
g_value_set_string (value, self->install_mode);
break;

default:
G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec);
}
Expand Down Expand Up @@ -216,6 +230,16 @@ builder_source_file_set_property (GObject *object,
self->disable_http_decompression = g_value_get_boolean (value);
break;

case PROP_INSTALL_DIR:
g_free (self->install_dir);
self->install_dir = g_value_dup_string (value);
break;

case PROP_INSTALL_MODE:
g_free (self->install_mode);
self->install_mode = g_value_dup_string (value);
break;

default:
G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec);
}
Expand All @@ -231,6 +255,20 @@ builder_source_file_validate (BuilderSource *source,
strchr (self->dest_filename, '/') != NULL)
return flatpak_fail (error, "No slashes allowed in dest-filename, use dest property for directory");

if (self->install_dir != NULL && g_path_is_absolute (self->install_dir))
return flatpak_fail (error, "install-dir must be relative to FLATPAK_DEST");

if (self->install_mode != NULL)
{
char *end;
guint64 mode = g_ascii_strtoull (self->install_mode, &end, 8);
if (*end != '\0' || mode > 07777)
return flatpak_fail (error, "install-mode must be a valid octal permission string");
}

if (self->install_mode != NULL && self->install_dir == NULL)
return flatpak_fail (error, "install-mode requires install-dir to be set");

return TRUE;
}

Expand Down Expand Up @@ -552,6 +590,87 @@ builder_source_file_extract (BuilderSource *source,
return TRUE;
}

static gboolean
builder_source_file_install (BuilderSource *source,
GFile *build_dir,
BuilderContext *context,
GError **error)
{
BuilderSourceFile *self = BUILDER_SOURCE_FILE (source);
const char *filename;
g_autofree char *basename = NULL;
g_autofree char *dst_path = NULL;
g_autoptr(GFile) src = NULL;
g_autoptr(GFile) install_dir = NULL;
g_autoptr(GFile) dst = NULL;
g_autoptr(GFile) dest_dir = NULL;
GFile *app_dir = NULL;

if (self->install_dir == NULL || self->install_dir[0] == '\0')
return TRUE;

if (self->dest_filename)
filename = self->dest_filename;
else
{
gboolean is_local, is_inline;
g_autoptr(GFile) source_file = get_source_file (self, context, &is_local, &is_inline, error);
if (source_file == NULL)
return FALSE;
basename = g_file_get_basename (source_file);
filename = basename;
}

app_dir = builder_context_get_app_dir (context);
dest_dir = builder_context_get_build_runtime (context)
? g_file_get_child (app_dir, "usr")
: g_file_get_child (app_dir, "files");
install_dir = g_file_resolve_relative_path (dest_dir, self->install_dir);

if (!g_file_has_prefix (install_dir, dest_dir))
return flatpak_fail (error, "install-dir cannot escape the install prefix");

src = g_file_get_child (build_dir, filename);
dst = g_file_get_child (install_dir, filename);
dst_path = g_file_get_path (dst);

if (!flatpak_file_query_exists_nofollow (src))
return flatpak_fail (error, "Source file '%s' not found", filename);

if (!flatpak_mkdir_p (install_dir, NULL, error))
return FALSE;

if (flatpak_file_query_exists_nofollow (dst))
g_printerr ("Warning: %s already exists, skipping install\n", dst_path);
else
{
if (!g_file_copy (src, dst, G_FILE_COPY_NOFOLLOW_SYMLINKS,
Copy link
Contributor

Choose a reason for hiding this comment

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

AFAIU this is executed in the host context and dst can point to arbitrary locations on the filesystem. You really have to make sure that dst points somewhere inside dest_dir. This is harder than it sounds because even if you ensure there are no ".." in self->install_dir, there might be symlinks anywhere in the build-controlled path (and they do get followed even if you use G_FILE_COPY_NOFOLLOW_SYMLINKS; they only care about the last element in the path).

I think we have a very similar problem with the LICENSE file code (i.e. you can copy certain files from the host system to the build output).

The solution is the use something like openat2/chase (https://gitlab.gnome.org/GNOME/libglnx/-/merge_requests/64).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yea this is indeed executed in host context.

Copy link
Contributor

Choose a reason for hiding this comment

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

Urgh, builder_migrate_locale_dirs also got this wrong already. I'm wondering if people assume that flatpak-builder is secure to run on untrusted content, because it certainly doesn't look like it is.

NULL, NULL, NULL, error))
return FALSE;

if (self->install_mode)
{
guint32 mode = (guint32) g_ascii_strtoull (self->install_mode, NULL, 8);

if (!g_file_set_attribute (dst,
G_FILE_ATTRIBUTE_UNIX_MODE,
G_FILE_ATTRIBUTE_TYPE_UINT32,
&mode,
G_FILE_QUERY_INFO_NOFOLLOW_SYMLINKS,
NULL,
error))
return FALSE;
}

if (self->install_mode)
g_print ("Installed %s to %s with permissions %s\n", filename, dst_path, self->install_mode);
else
g_print ("Installed %s to %s with default permissions\n", filename, dst_path);
}

return TRUE;
}

static gboolean
builder_source_file_bundle (BuilderSource *source,
BuilderContext *context,
Expand Down Expand Up @@ -655,6 +774,8 @@ builder_source_file_checksum (BuilderSource *source,
builder_cache_checksum_compat_str (cache, self->sha512);
builder_cache_checksum_str (cache, self->dest_filename);
builder_cache_checksum_compat_strv (cache, self->mirror_urls);
builder_cache_checksum_compat_str (cache, self->install_dir);
builder_cache_checksum_compat_str (cache, self->install_mode);
}

static void
Expand All @@ -670,6 +791,7 @@ builder_source_file_class_init (BuilderSourceFileClass *klass)
source_class->show_deps = builder_source_file_show_deps;
source_class->download = builder_source_file_download;
source_class->extract = builder_source_file_extract;
source_class->install = builder_source_file_install;
source_class->bundle = builder_source_file_bundle;
source_class->update = builder_source_file_update;
source_class->checksum = builder_source_file_checksum;
Expand Down Expand Up @@ -747,6 +869,20 @@ builder_source_file_class_init (BuilderSourceFileClass *klass)
"",
FALSE,
G_PARAM_READWRITE));
g_object_class_install_property (object_class,
PROP_INSTALL_DIR,
g_param_spec_string ("install-dir",
"",
"",
NULL,
G_PARAM_READWRITE));
g_object_class_install_property (object_class,
PROP_INSTALL_MODE,
g_param_spec_string ("install-mode",
"",
"",
NULL,
G_PARAM_READWRITE));
}

static void
Expand Down
23 changes: 23 additions & 0 deletions src/builder-source.c
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,15 @@ builder_source_real_extract (BuilderSource *self,
return FALSE;
}

static gboolean
builder_source_real_install (BuilderSource *self,
GFile *build_dir,
BuilderContext *context,
GError **error)
{
return TRUE;
}

static gboolean
builder_source_real_bundle (BuilderSource *self,
BuilderContext *context,
Expand Down Expand Up @@ -194,6 +203,7 @@ builder_source_class_init (BuilderSourceClass *klass)
klass->show_deps = builder_source_real_show_deps;
klass->download = builder_source_real_download;
klass->extract = builder_source_real_extract;
klass->install = builder_source_real_install;
klass->bundle = builder_source_real_bundle;
klass->update = builder_source_real_update;

Expand Down Expand Up @@ -415,6 +425,19 @@ builder_source_extract (BuilderSource *self,
return class->extract (self, real_dest, source_dir, build_options, context, error);
}

gboolean
builder_source_install (BuilderSource *self,
GFile *build_dir,
BuilderContext *context,
GError **error)
{
BuilderSourceClass *class;

class = BUILDER_SOURCE_GET_CLASS (self);

return class->install (self, build_dir, context, error);
}

gboolean
builder_source_bundle (BuilderSource *self,
BuilderContext *context,
Expand Down
8 changes: 8 additions & 0 deletions src/builder-source.h
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,10 @@ typedef struct
BuilderOptions *build_options,
BuilderContext *context,
GError **error);
gboolean (* install)(BuilderSource *self,
GFile *build_dir,
BuilderContext *context,
GError **error);
gboolean (* bundle)(BuilderSource *self,
BuilderContext *context,
GError **error);
Expand Down Expand Up @@ -99,6 +103,10 @@ gboolean builder_source_extract (BuilderSource *self,
BuilderOptions *build_options,
BuilderContext *context,
GError **error);
gboolean builder_source_install (BuilderSource *self,
GFile *build_dir,
BuilderContext *context,
GError **error);
gboolean builder_source_bundle (BuilderSource *self,
BuilderContext *context,
GError **error);
Expand Down
8 changes: 8 additions & 0 deletions tests/test-builder.sh
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ cp $(dirname $0)/source2.json include1/include2/
cp $(dirname $0)/data2 include1/include2/
cp $(dirname $0)/data2.patch include1/include2/
echo "MY LICENSE" > ./LICENSE
touch ./foobar

for MANIFEST in test.json test.yaml test-rename.json test-rename-appdata.json ; do
echo "building manifest $MANIFEST" >&2
Expand Down Expand Up @@ -106,6 +107,13 @@ for MANIFEST in test.json test.yaml test-rename.json test-rename-appdata.json ;

assert_file_has_content appdir/files/share/licenses/org.test.Hello2/test/LICENSE '^MY LICENSE$'

assert_has_file appdir/files/share/icons/hicolor/64x64/apps/foobar
PERMS=$(stat -c "%a" appdir/files/share/icons/hicolor/64x64/apps/foobar)
if [ "$PERMS" != "755" ]; then
echo "not ok install-dir+install-mode: expected 755, got $PERMS"
exit 1
fi

echo "ok build"
done

Expand Down
6 changes: 6 additions & 0 deletions tests/test-rename-appdata.json
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,12 @@
"dest-filename": "hello2.sh",
"commands": [ "echo \"Hello world2, from a sandbox\"" ]
},
{
"type": "file",
"path": "foobar",
"install-dir": "share/icons/hicolor/64x64/apps",
"install-mode": "0755"
},
{
"type": "shell",
"commands": [
Expand Down
Loading